[Lldb-commits] [PATCH] D112824: [lldb][AArch64] Add MakeTaggedRanges to MemoryTagManager

Wed Jan 12 03:00:22 PST 2022

DavidSpickett marked 2 inline comments as done.
DavidSpickett added a comment.

ping!

================
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);
----------------
DavidSpickett wrote:
> DavidSpickett wrote:
> > omjavaid wrote:
> > > 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.
> > The end goal here is "memory read <...> --show-tags" and that command will have to use the ABI plugin in any case so the addresses this gets will be PAC/TBI/MTE free already.
> > 
> > So for now this should be addressed by https://reviews.llvm.org/D103626. (once I convince myself which address should be show in the output but that's not for this issue)
> > 
> > I agree that there is a bit of a conflict here with the ABI plugin removing everything vs the MemoryTagManager removing only the tag. However I think that it will work fine in this case.
> Speaking of conflict, `MemoryTagManagerAArch64MTE::RemoveNonAddressBits` should probably be renamed to `RemoveTagBits` but I'll do that as another change.
On second thought this will be better done in a general effort to remove the layering weirdness we currently have. Where we've got a tag manager that is removing the tags and the rest of the top byte. Then the ABI removing all of that and more.

Ideally the tag manager should only know about tags but some places assume it can also remove non-address bits in general. So this is more than just a rename and will take a bit more time.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112824/new/

https://reviews.llvm.org/D112824