[AArch64InstPrinter][llvm-objdump] Print ADR PC-relative label as a target address hexadecimal form

[krisb]

Summary

This is similar to ADRP and matches GNU objdump:

GNU objdump:

0000000000200100 <_start>:
  200100:    adr    x0, 201000 <_start+0xf00>

llvm-objdump (before patch):

0000000000200100 <_start>:
  200100:    adr    x0, #3840

llvm-objdump (after patch):

0000000000200100 <_start>:
  200100:    adr    x0, 0x201000 <_start+0xf00>

[simon_tatham]

A much needed improvement – thanks!

llvm/test/tools/llvm-objdump/ELF/AArch64/pcrel-address.yaml

6

Not that it matters for testing purposes, but did you mean to point that at 0x201000? Your other new adr matches the adrp that was there already.

[peter.smith]

No objections from me. I guess that --no-print-imm-hex is not honoured as we are not printing the immediate anymore?

My one caveat is that this can give some strange answers for some code that I doubt anyone would write in practice like

.text
adr x0, -4096

Which when disassembling an object will wrap around the address space. With gnu objdump:

0000000000000000 <.text>:
   0:   10ff8000        adr     x0, fffffffffffff000 <.text+0xfffffffffffff000>

which is less helpful than the current llvm-objdump (I'm not using --no-print-imm-hex)

0000000000000000 <$x.0>:
       0: 10ff8000      adr     x0, #-0x1000

However that should only happen for code that does a relative offset outside the section which is unlikely to work in practice.

I suggest updating the description with a slightly different example as it looks like it is just changing from decimal to hex. It is doing a bit more than that. It is changing a decimal offset from the PC to the absolute address of PC + offset.

[simon_tatham]

I think that most ideally, if you don't know the true address of the instruction being disassembled, the best way to disassemble ADR is to indicate explicitly that the address you're going to end up with depends on that unknown, and not make it look as if it's a constant.

So if you were disassembling a single encoding in the complete absence of any context, you'd write something like adr x0, .+0x1234 (adjusted as necessary for the representation of 'here' in the assembler syntax you're outputting). And if you're in a context like an object file, where your ADR instruction is at a known offset from the start of the section but the section in turn might end up anywhere in memory after linking, you could say something relative to the section name or to one of the symbols in it, like adr x0, .text+0x12 or adr x0, .L.stringliteral.

But that's a much bigger piece of work, and shouldn't block this change, which is a strict improvement on the previous representation!

[krisb]

Make test checks more readable.

[krisb]

@peter.smith thank you for your comments!

In D144079#4128441, @peter.smith wrote:

No objections from me. I guess that --no-print-imm-hex is not honoured as we are not printing the immediate anymore?

Right.

My one caveat is that this can give some strange answers for some code that I doubt anyone would write in practice like

.text
adr x0, -4096

Which when disassembling an object will wrap around the address space. With gnu objdump:

0000000000000000 <.text>:
   0:   10ff8000        adr     x0, fffffffffffff000 <.text+0xfffffffffffff000>

which is less helpful than the current llvm-objdump (I'm not using --no-print-imm-hex)

0000000000000000 <$x.0>:
       0: 10ff8000      adr     x0, #-0x1000

However that should only happen for code that does a relative offset outside the section which is unlikely to work in practice.

I agree that llvm-objdump output w/o the patch looks a bit better, but the example seems to be rather artificial. So, I'm not sure we need to make this more complex, but we can, for example, check that a calculated address is still in +/-1MB range and opt out if it's not.

I suggest updating the description with a slightly different example as it looks like it is just changing from decimal to hex. It is doing a bit more than that. It is changing a decimal offset from the PC to the absolute address of PC + offset.

Done.

llvm/test/tools/llvm-objdump/ELF/AArch64/pcrel-address.yaml

6

I didn't mean to fully match addresses from adrp example, just reused the offsets w/o any specific intent.
It seems to be more readable if to remove --no-leading-addr and check for instruction address as well to demonstrate that the printed address is PC + offset.

[krisb]

@simon_tatham thank you for looking at this!

In D144079#4128592, @simon_tatham wrote:

I think that most ideally, if you don't know the true address of the instruction being disassembled, the best way to disassemble ADR is to indicate explicitly that the address you're going to end up with depends on that unknown, and not make it look as if it's a constant.
So if you were disassembling a single encoding in the complete absence of any context, you'd write something like adr x0, .+0x1234 (adjusted as necessary for the representation of 'here' in the assembler syntax you're outputting). And if you're in a context like an object file, where your ADR instruction is at a known offset from the start of the section but the section in turn might end up anywhere in memory after linking, you could say something relative to the section name or to one of the symbols in it, like adr x0, .text+0x12 or adr x0, .L.stringliteral.

This looks like how a target address label works, which is mostly what I want to achieve by this patch. In llvm/test/tools/llvm-objdump/ELF/AArch64/pcrel-address.yaml the following example

adr x0, 0x201004 <_start+0xf04>

demonstrates this. Label in '<>' (target address label, not sure about the notion, just using the wording from llvm-objdump code) printed relatively to a known symbol _start.
But llvm-objdump doesn't output any label if it isn't able to resolve an address against any known symbol. May be we can improve this.

[peter.smith]

Thanks for the updates. I agree with SImon that this is an improvement worth having.

lld/test/COFF/arm64-relocs-imports.test

 # BEFORE:       70:       00000008        udf #8
 # BEFORE:       74:       00000000        udf #0
 # BEFORE:       78:       00000001        udf #1
 # BEFORE:       7c:       00000001        udf #1
 # BEFORE:       80:       91000000        add     x0, x0, #0
 # BEFORE:       84:       91400000        add     x0, x0, #0, lsl #12
 # BEFORE:       88:       f9400000        ldr     x0, [x0]
 # BEFORE:       8c:       00000001        udf #1
-# BEFORE:       90:       30091a20        adr x0, #74565
+# BEFORE:       90:       30091a20        adr x0, 0x123d5
 # BEFORE:       94:       54000001        b.ne    0x94
 # BEFORE:       98:       36000000        tbz     w0, #0, 0x98
 # BEFORE:       9c:       00000001        udf #1
 # BEFORE:       a0:       90800002        adrp    x2, 0xffffffff00000000
 
 # AFTER: Disassembly of section .text:
 # AFTER-EMPTY:
 # AFTER:  140001000:      f81f0ffe        str     x30, [sp, #-16]!

 # AFTER:  140001070:      40002010        <unknown>
 # AFTER:  140001074:      00000001        udf #1
 # AFTER:  140001078:      00002009        udf #8201
 # AFTER:  14000107c:      00000009        udf #9
 # AFTER:  140001080:      910e2000        add     x0, x0, #904
 # AFTER:  140001084:      91400400        add     x0, x0, #1, lsl #12
 # AFTER:  140001088:      f941c400        ldr     x0, [x0, #904]
 # AFTER:  14000108c:      00000003        udf #3
-# AFTER:  140001090:      300995e0        adr     x0, #78525
+# AFTER:  140001090:      300995e0        adr     x0, 0x14001434d
 # AFTER:  140001094:      54000081        b.ne    0x1400010a4
 # AFTER:  140001098:      36000060        tbz     w0, #0, 0x1400010a4
 # AFTER:  14000109c:      ffffff61        <unknown>
 # AFTER:  1400010a0:      b0fff802        adrp    x2,  0x13ff02000
 # AFTER:  1400010a4:      b0000010        adrp    x16, 0x140002000
 # AFTER:  1400010a8:      f9402a10        ldr     x16, [x16, #80]
 # AFTER:  1400010ac:      d61f0200        br      x16
 

lld/test/ELF/aarch64-copy.s

 // CHECK-NEXT:     Other:
 // CHECK-NEXT:     Section: .bss
 // CHECK: ]
 
 // CODE: Disassembly of section .text:
 // CODE-EMPTY:
 // CODE-NEXT: <_start>:
 // S + A - P = 0x2303f0 + 0 - 0x21031c = 131284
-// CODE-NEXT:  21031c: adr  x1, #131284
+// CODE-NEXT:  21031c: adr  x1, 0x2303f0
 // Page(S + A) - Page(P) = Page(0x230400) - Page(0x210320) = 131072
 // CODE-NEXT:  210320: adrp x2, 0x230000
 // (S + A) & 0xFFF = (0x230400 + 0) & 0xFFF = 1024
 // CODE-NEXT:  210324: add  x2, x2, #1024
 
 // RODATA: Contents of section .rodata:
 // S(z) = 0x230404
 // RODATA-NEXT:  200318 04042300

lld/test/ELF/aarch64-relocs.s

 # REQUIRES: aarch64
 # RUN: llvm-mc -filetype=obj -triple=aarch64-unknown-freebsd %s -o %t
 # RUN: echo '.globl zero; zero = 0' | llvm-mc -filetype=obj -triple=aarch64-unknown-freebsd -o %t2.o
 # RUN: ld.lld %t %t2.o -o %t2
 # RUN: llvm-objdump --no-print-imm-hex -d %t2 | FileCheck %s
 
 .section .R_AARCH64_ADR_PREL_LO21,"ax",@progbits
 .globl _start
 _start:
   adr x1,msg
 msg:  .asciz  "Hello, world\n"
 msgend:
 
 # CHECK: Disassembly of section .R_AARCH64_ADR_PREL_LO21:
 # CHECK-EMPTY:
 # CHECK: <_start>:
-# CHECK:        0:       10000021        adr     x1, #4
+# CHECK:        0:       10000021        adr     x1, 0x210124
 # CHECK: <msg>:
 # CHECK:        4:
 # #4 is the adr immediate value.
 
 .section .R_AARCH64_ADR_PREL_PG_HI21,"ax",@progbits
   adrp x1,mystr
 mystr:
   .asciz "blah"

lld/test/ELF/aarch64-undefined-weak.s

 
 // CHECK: Disassembly of section .text:
 // CHECK-EMPTY:
 // CHECK-NEXT: 0000000010010120 <_start>:
 // CHECK-NEXT: 10010120: b       0x10010124
 // CHECK-NEXT: 10010124: bl      0x10010128
 // CHECK-NEXT: 10010128: b.eq    0x1001012c
 // CHECK-NEXT: 1001012c: cbz     x1, 0x10010130
-// CHECK-NEXT: 10010130: adr     x0, #0
+// CHECK-NEXT: 10010130: adr     x0, 0x10010130
 // CHECK-NEXT: 10010134: adrp    x0, 0x10010000
 // CHECK-NEXT: 10010138: ldr     x8, 0x10010138
 // CHECK:      1001013c: 00 00 00 00     .word   0x00000000
 // CHECK-NEXT: 10010140: 00 00 00 00     .word   0x00000000
 // CHECK-NEXT: 10010144: 00 00 00 00     .word   0x00000000
 // CHECK-NEXT: 10010148: 00 00 00 00     .word   0x00000000
 // CHECK-NEXT: 1001014c: 00 00           .short  0x0000
 // CHECK-LABEL: <bl_undefweak2>:
 // CHECK-NEXT:    bl {{.*}} <bl_undefweak2+0x4>

llvm/lib/Target/AArch64/AArch64InstrFormats.td

 // ADR[P] instruction labels.
 def AdrpOperand : AsmOperandClass {
   let Name = "AdrpLabel";
   let ParserMethod = "tryParseAdrpLabel";
   let DiagnosticType = "InvalidLabel";
 }
 def adrplabel : Operand<i64> {
   let EncoderMethod = "getAdrLabelOpValue";
-  let PrintMethod = "printAdrpLabel";
+  let PrintMethod = "printAdrAdrpLabel";
   let ParserMatchClass = AdrpOperand;
   let OperandType = "OPERAND_PCREL";
 }
 
 def AdrOperand : AsmOperandClass {
   let Name = "AdrLabel";
   let ParserMethod = "tryParseAdrLabel";
   let DiagnosticType = "InvalidLabel";
 }
 def adrlabel : Operand<i64> {
   let EncoderMethod = "getAdrLabelOpValue";
+  let PrintMethod = "printAdrAdrpLabel";
   let ParserMatchClass = AdrOperand;
+  let OperandType = "OPERAND_PCREL";
 }
 
 class SImmOperand<int width> : AsmOperandClass {
   let Name = "SImm" # width;
   let DiagnosticType = "InvalidMemoryIndexedSImm" # width;
   let RenderMethod = "addImmOperands";
   let PredicateMethod = "isSImm<" # width # ">";
 }

llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.h

protected:
   void printTypedVectorList(const MCInst *MI, unsigned OpNum,
                             const MCSubtargetInfo &STI, raw_ostream &O);
 
   template <unsigned Scale = 1>
   void printVectorIndex(const MCInst *MI, unsigned OpNum,
                         const MCSubtargetInfo &STI, raw_ostream &O);
   void printMatrixIndex(const MCInst *MI, unsigned OpNum,
                         const MCSubtargetInfo &STI, raw_ostream &O);
-  void printAdrpLabel(const MCInst *MI, uint64_t Address, unsigned OpNum,
+  void printAdrAdrpLabel(const MCInst *MI, uint64_t Address, unsigned OpNum,
                          const MCSubtargetInfo &STI, raw_ostream &O);
   void printBarrierOption(const MCInst *MI, unsigned OpNum,
                           const MCSubtargetInfo &STI, raw_ostream &O);
   void printBarriernXSOption(const MCInst *MI, unsigned OpNum,
                              const MCSubtargetInfo &STI, raw_ostream &O);
   void printMSRSystemRegister(const MCInst *MI, unsigned OpNum,
                               const MCSubtargetInfo &STI, raw_ostream &O);
   void printMRSSystemRegister(const MCInst *MI, unsigned OpNum,
                               const MCSubtargetInfo &STI, raw_ostream &O);

llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp

void AArch64InstPrinter::printAlignedLabel(const MCInst *MI, uint64_t Address,
   if (BranchTarget && BranchTarget->evaluateAsAbsolute(TargetAddress)) {
     O << formatHex((uint64_t)TargetAddress);
   } else {
     // Otherwise, just print the expression.
     MI->getOperand(OpNum).getExpr()->print(O, &MAI);
   }
 }
 
-void AArch64InstPrinter::printAdrpLabel(const MCInst *MI, uint64_t Address,
+void AArch64InstPrinter::printAdrAdrpLabel(const MCInst *MI, uint64_t Address,
                                            unsigned OpNum,
                                            const MCSubtargetInfo &STI,
                                            raw_ostream &O) {
   const MCOperand &Op = MI->getOperand(OpNum);
 
   // If the label has already been resolved to an immediate offset (say, when
   // we're running the disassembler), just print the immediate.
   if (Op.isImm()) {
-    const int64_t Offset = Op.getImm() * 4096;
+    int64_t Offset = Op.getImm();
+    if (MI->getOpcode() == AArch64::ADRP) {
+      Offset = Offset * 4096;
+      Address = Address & -4096;
+    }
     O << markup("<imm:");
     if (PrintBranchImmAsAddress)
-      O << formatHex((Address & -4096) + Offset);
+      O << formatHex(Address + Offset);
     else
       O << "#" << Offset;
     O << markup(">");
     return;
   }
 
   // Otherwise, just print the expression.
   MI->getOperand(OpNum).getExpr()->print(O, &MAI);

llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCTargetDesc.cpp

bool evaluateBranch(const MCInst &Inst, uint64_t Addr, uint64_t Size,
                       uint64_t &Target) const override {
     // Search for a PC-relative argument.
     // This will handle instructions like bcc (where the first argument is the
     // condition code) and cbz (where it is a register).
     const auto &Desc = Info->get(Inst.getOpcode());
     for (unsigned i = 0, e = Inst.getNumOperands(); i != e; i++) {
       if (Desc.operands()[i].OperandType == MCOI::OPERAND_PCREL) {
         int64_t Imm = Inst.getOperand(i).getImm();
-        if (Inst.getOpcode() == AArch64::ADRP)
+        if (Inst.getOpcode() == AArch64::ADR)
+          Target = Addr + Imm;
+        else if (Inst.getOpcode() == AArch64::ADRP)
           Target = (Addr & -4096) + Imm * 4096;
         else
           Target = Addr + Imm * 4;
         return true;
       }
     }
     return false;
   }

llvm/test/MC/AArch64/adr.s

 // RUN: llvm-mc -triple aarch64-elf -filetype=obj %s -o - | llvm-objdump --no-print-imm-hex -d -r - | FileCheck %s
 
-// CHECK: adr x0, #100
-// CHECK-NEXT: adr x2, #0
+// CHECK: adr x0, 0x64
+// CHECK-NEXT: adr x2, 0x4
 // CHECK-NEXT: R_AARCH64_ADR_PREL_LO21  Symbol
-// CHECK-NEXT: adr x3, #0
+// CHECK-NEXT: adr x3, 0x8
 // CHECK-NEXT: R_AARCH64_ADR_PREL_LO21  Symbol
-// CHECK-NEXT: adr x4, #0
+// CHECK-NEXT: adr x4, 0xc
 // CHECK-NEXT: R_AARCH64_ADR_PREL_LO21  Symbol+0xf1000
-// CHECK-NEXT: adr x5, #0
+// CHECK-NEXT: adr x5, 0x10
 // CHECK-NEXT: R_AARCH64_ADR_PREL_LO21  Symbol+0xf1000
-// CHECK-NEXT: adr x6, #0
+// CHECK-NEXT: adr x6, 0x14
 // CHECK-NEXT: R_AARCH64_ADR_PREL_LO21  Symbol+0xf1000
 
   adr x0, 100
   adr x2, Symbol
   adr x3, Symbol + 0
   adr x4, Symbol + 987136
   adr x5, (0xffffffff000f1000 - 0xffffffff00000000 + Symbol)
   adr x6, Symbol + (0xffffffff000f1000 - 0xffffffff00000000)

llvm/test/MC/AArch64/coff-relocations.s

 
 // DISASM: 30:       b0091a20     adrp    x0, 0x12345000
 // DISASM: 34:       910d1400     add     x0, x0, #837
 // DISASM: 38:       394d1400     ldrb    w0, [x0, #837]
 // DISASM: 3c:       f941a400     ldr     x0, [x0, #840]
 // DISASM: 40:       91000000     add     x0, x0, #0
 // DISASM: 44:       91400000     add     x0, x0, #0, lsl #12
 // DISASM: 48:       f9400000     ldr     x0, [x0]
-// DISASM: 4c:       30091a20     adr     x0, #74565
+// DISASM: 4c:       30091a20     adr     x0, 0x12391
 
 // DATA: Contents of section .rdata:
 // DATA-NEXT:  0000 30000000 08000000

llvm/test/tools/llvm-objdump/ELF/AArch64/elf-aarch64-mapping-symbols.test

.section .myothersection,"ax",@progbits
   adrp x1,mystr
 mystr:
   .ascii "blah"
   .size mystr, 4
   .byte 0x9a
 
 # CHECK: Disassembly of section .mysection:
 # CHECK: <_start>:
-# CHECK:        0:       10000021        adr     x1, #4
+# CHECK:        0:       10000021        adr     x1, 0x4
 # CHECK: <msg>:
 # CHECK:        4:       48 65 6c 6c     .word
 # CHECK:        8:       6f 2c 20 77     .word
 # CHECK:        c:       6f 72 6c 64     .word
 # CHECK:       10:       0a 00           .short  0x000a
 # CHECK: Disassembly of section .myothersection:
 # CHECK: <$x.2>:
 # CHECK:        0:       90000001        adrp    x1, 0x0
 # CHECK: <mystr>:
 # CHECK:        4:       62 6c 61 68     .word
 # CHECK:        8:       9a              .byte   0x9a

llvm/test/tools/llvm-objdump/ELF/AArch64/pcrel-address.yaml

 # RUN: yaml2obj %s -o %t
 # RUN: llvm-objdump %t -d --no-show-raw-insn --no-leading-addr | FileCheck %s
 
 # CHECK-LABEL: <_start>:
+# CHECK-NEXT:    adr x0, 0x220000
+# CHECK-NEXT:    adr x0, 0x201004 <_start+0xf04>
 # CHECK-NEXT:    adrp x2, 0x220000
 # CHECK-NEXT:    adrp x2, 0x201000 <_start+0xf00>
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_AARCH64
 Sections:
   - Name:    .text
     Type:    SHT_PROGBITS
     Address: 0x200100
     Flags:   [SHF_ALLOC, SHF_EXECINSTR]
-    Content: '02010090020000B0'
+    Content: '00F80F100078001002010090020000B0'
   - Name:    .data
     Type:    SHT_PROGBITS
     Flags:   [SHF_ALLOC, SHF_WRITE]
     Address: 0x220000
 Symbols:
   - Name:    _start
     Section: .text
     Value:   0x200100