[Lldb-commits] [PATCH] D107213: Disassemble AArch64 pc-relative address calculations & symbolicate

Wed Aug 4 00:43:00 PDT 2021

jasonmolenda added a comment.

Thanks so much for taking the time to read the patch over and make suggestions David. I cribbed the bit masking / manip from MachODump.cpp's SymbolizerSymbolLookUp but you're right about it being better with at least a little bit of comments about what is going on there.

Yeah, this is a very aarch64 specific thing in the generic symbolizer, but it's not an especially large function so I didn't feel too bad about it.  If we started needing to handle multi-instruction sequences like this in the symbolizer (where we save state from previous instructions to determine a result), then this definitely would not scale well.

I've written it so that I only recognize the pattern with ADRP is immediately followed by an ADD, that was maybe a choice I could have gone either way on.  I clear the saved ADRP instruction when it's not followed by that ADD in the same instruction.  I didn't want to mis-calculate an address if the instruction pair using the same register were in two different basic blocks or something.

================
Comment at: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp:1325
+        // the same register, this is pc-relative address calculation.
+        if (*type_ptr == LLVMDisassembler_ReferenceType_In_ARM64_ADDXri &&
+            m_adrp_address == pc - 4 &&
----------------
DavidSpickett wrote:
> There is a 32 bit variant of ADD with W registers, I assume `LLVMDisassembler_ReferenceType_In_ARM64_ADDXri` is specific to the X version though, correct?
> 
> Which saves you checking the "sf" field later. (plus I don't know that any compiler would bother using the W version for this sort of thing anyway)
Ah, good point.  I did a quick check and clang currently generates the same adrp+add with an x register, and then ANDs the result with 0xffffffff for our "arm64_32" target (ILP32 codegen that executes in the lower 4GB of virtual address space but uses the aarch64 ISA), so it works out.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107213