[Lldb-commits] [PATCH] D97281: [lldb][AArch64] Add class for managing memory tags

Tue Mar 2 05:43:28 PST 2021

DavidSpickett added inline comments.

================
Comment at: lldb/source/Plugins/Process/Utility/MemoryTagManagerAArch64MTE.cpp:43
+
+ptrdiff_t MemoryTagManagerAArch64MTE::AddressDiff(lldb::addr_t addr1,
+                                                  lldb::addr_t addr2) const {
----------------
omjavaid wrote:
> DavidSpickett wrote:
> > omjavaid wrote:
> > > I am a little apprehensive about this AddressDiff function. AArch64 virtual address is either 48 bits or 52 bits. MTE start from 56th bit and I believe bits (48 - 55) will be either set zero (for user address) and set to one for kernel addresses. 
> > > I am not sure if there is an interface available for us to know if VA is 48 or 52 bits. What do you think how should we manage this here.
> > I looked at this a while back and there is no user space way to tell what size virtual address you have. You can assume at least 48 but then the rest I think is handled by some backwards compatibility stuff that means you don't have to care.
> > 
> > What is an issue is this only ignores the tag bits, not the whole top byte. This is the instruction the ptrdiff intrinsic emits:
> > ```
> > C6.2.317 SUBPS
> > Subtract Pointer, setting Flags subtracts the 56-bit address held in the second source register from the 56-bit address
> > held in the first source register, sign-extends the result to 64-bits, and writes the result to the destination register. It
> > updates the condition flags based on the result of the subtraction.
> > ```
> > 
> > That assumes top byte ignore, so I should be doing the same here along with the sign extension to handle the 0/1 fill.
> > (this might apply to a lot more than just this diff call too)
> hmm .. yes GDB also assumes 52 bit address for AArch64. Did you try reading kenel space address during testing and see what you get in that case may be stepping into a syscall (never tried it with LLDB) not sure if this is a must have for LLDB as its currently a user application debugger as far as AArch64 is concerned. 
> 
> Even if this is 52 bits VA this function assumes bit 52-55 set to zero? This can also mean that by the time we get this address TBI interface would have cleaned it up for us? 
> 
> I think we should have a way to figure out significant address bits and clean up the rest. For now this needs no change but I guess I ll have to manage this in TBI support and provided and interface for significant address bit for a process.
> 
> 
I wasn't aware stepping into a syscall was a thing for lldb but I'll see what I can find.

General TBI handling would solve some of this, still need to sign extend based on bit 55 I think.

I'm going to change this to ignore the top byte always, so it'll work on a PAC+MTE system. Then later we can make it use the "proper" TBI hooks.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97281