omjavaid added inline comments.
================ Comment at: lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp:70 +static llvm::Error make_invalid_range_err(lldb::addr_t addr, + lldb::addr_t end_addr) { ---------------- I think this function name should be camel-case although there are examples of similar functions not following the camel case. I am bit unsure do you a reference for/against? ================ Comment at: lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp:148 + // For the logic to work regions must be in ascending order. + // We're going to assume that there are no overlapping regions + // and that each region is granule aligned already. ---------------- so what would be the behavior if regions do overlap. Should we return an error rather than keep going? ================ Comment at: lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp:160 + // we must use an untagged address. + MemoryRegionInfo::RangeType range(RemoveNonAddressBits(addr), len); + range = ExpandToGranule(range); ---------------- RemoveNonAddressBits hard-coded but symbols may resolve to higher order bits containing PACs. For now I only came across code pointers with PACs. But if you suspect code regions can be inputs to this function then may be make accommodating changes probably in separate patch. ================ Comment at: lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp:167 + // While there are regions to check and the range has non zero length + for (; region != end_region && range.IsValid(); ++region) { + // If the region doesn't overlap the range at all, ignore it. ---------------- this loop looks like a candidate for using range based for instead of declaring iterate. We can always break if range in invalid. for (auto region : memory_regions) may be ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112824/new/ https://reviews.llvm.org/D112824 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits