[Lldb-commits] [PATCH] D99944: [LLDB] AArch64 PAC elf-core stack unwinder support

Tue Apr 13 08:30:16 PDT 2021

omjavaid added a comment.

In D99944#2684280 <https://reviews.llvm.org/D99944#2684280>, @jasonmolenda wrote:

> Hi Omair, sorry for the delay in looking at this.  It seems like we have two overlapping patches here, @justincohen  patch in https://reviews.llvm.org/D98886 and this one.
>
> I'm not convinced that we'll see a target where we have noncontiguous bits used for addressing (the best reason for using a mask), but Justin points out that Linux ARM's NT_ARM_PAC_MASK (gettable by ptrace PTRACE_GETREGSET) is expressed as a mask (and googling around, it looks like you can have a different mask for code pointers and data pointers, so Process would really need to store both, but it may be that for crashpad only the code mask needs to be tracked in Process).
>
> lldb needs to allow for users to set the number of bits used for addressing in terms of a number -- 52 in your example -- but given that Linux wants to give us a mask via the ptrace call, I think we should preserve and use that mask.  Darwin's API also provide the value in terms of the number of bits used for addressing, and as a user setting I think that's the most natural way of handling it and we should allow it.  But internal to Process, we should convert this to a CodeAddress mask and down in the ABI's where we support AArch v8.3 ptrauth, we use bit 55 to clear/set the non-addressing high bits.
>
> Omair, Justin, what do you think here?  I don't think it's especially hard to accept this in terms of # of bits OR a mask, and we should use the more general internal rep in lldb.  Another alternative would be "the mask should be converted to the # of bits in addressing and stored in Process in those terms".
>
> (Honestly the idea of an address mask makes me nervous all over - if the rule is "take bit 55 and set all non-addressing bits to 0 or 1 depending on that", then we're implicitly saying "the mask only masks off high bits".  If it ever masked off bit 0, for instance, on a code address, then we're going to set that to 0 or 1 when we set/clear non-addressing bits?  Obviously not.  So it's not REALLY a mask, is it, it's just the # of bits used for addressing in another more flexible format that you can't actually flex or it all breaks. :)

Frankly I am also not too sure about keeping single mask for both code and data masks. For now Linux kernel ensures both are the same but they can differ in future. For user-space Linux apps top-byte is used for tags and bit 55 only gives us a hint on whether we mask the top byte only or also need to mask off bits (54:VA_Bits). For unwinder code mask is relevant but we would also like to use the mask for clearing top-bits off watchpoint addresses in-case of a tagged data pointer and if we have a separate data_mask we ll essentially need to store that information separately.

On linux masks are readable through code_mask and data_mask registers. We dont really need to keep this information in process as @labath pointed out but that is only fine for user-space apps. In case we intend to debug linux kernel or any bare-metal app utilizing the top byte(s) which i am not aware of any user of LLDB right now using it to debug the Linux kernel but I dont wanna introduce anything here which makes LLDB user-app specific so thats why one of the option is to send the information over gdb-remote protocol qHostInfo but that also restricts LLDB to lldb-server only.

In any case we ll need atleast one mask for sure and I dont mind @justincohen writing that patch and I ll rebase this patch on top of it once its merged.

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

https://reviews.llvm.org/D99944