[llvm-objdump] Avoid using mapping symbols as branch target labels

[krisb]

Summary

The main motivation for this change is to avoid ambiguity because
mapping symbol names may not be unique across a binary and do not allow uniquely
identifying target address. So that mapping symbols used as branch target
labels make llvm-objdump output less readable.

Another point is that mapping symbols sometimes appear in
non-allocatable sections, like debug info sections which make objdump
output even more confusing.

For example, a small AArch64 executable would contain plenty of $d[.*]
symbols and none of them would be useful as a label (base) for resolving
a branch or a memory operand target address:

0000000000000254 l       .note.ABI-tag	0000000000000000 $d
00000000000008d4 l       .eh_frame            0000000000000000 $d
0000000000000868 l       .rodata              0000000000000000 $d
0000000000011028 l       .data                0000000000000000 $d
0000000000010db8 l       .fini_array          0000000000000000 $d
0000000000010db0 l       .init_array          0000000000000000 $d
00000000000008e8 l       .eh_frame            0000000000000000 $d
0000000000011034 l       .bss                 0000000000000000 $d
0000000000000165 l       .debug_abbrev	0000000000000000 $d.1
0000000000000553 l       .debug_info    	0000000000000000 $d.2
0000000000000000 l       .debug_str_offsets	0000000000000000 $d.3
000000000000039c l       .debug_str           0000000000000000 $d.4
0000000000000000 l       .debug_addr          0000000000000000 $d.5
00000000000000c1 l       .comment             0000000000000000 $d.6
0000000000000948 l       .eh_frame            0000000000000000 $d.7
00000000000001d4 l       .debug_line          0000000000000000 $d.8
0000000000000000 l       .debug_line_str	0000000000000000 $d.9
0000000000011030 l       .data        	0000000000000000 $d.1
00000000000001ac l       .debug_abbrev	0000000000000000 $d.2
00000000000005a1 l       .debug_info    	0000000000000000 $d.3
0000000000000024 l       .debug_str_offsets	0000000000000000 $d.4
000000000000039c l       .debug_str           0000000000000000 $d.5
0000000000000010 l       .debug_addr          0000000000000000 $d.6
00000000000000c1 l       .comment             0000000000000000 $d.7
0000000000000948 l       .eh_frame            0000000000000000 $d.8
0000000000000258 l       .debug_line          0000000000000000 $d.9
00000000000009a0 l       .eh_frame            0000000000000000 $d

Note that GNU objdump doesn't use mapping symbols as branch target
labels for all targets that support such symbols (ARM, AArch64, CSKY).

[krisb]

Fix lld test

[ikudrin]

LGTM. Please give some time for other reviewers to react.

[simon_tatham]

Yes, I agree too – definitely a good change.

I especially like the part where a mapping symbol isn't used even if it's the only symbol at that address at all, as seen in one of the modified tests. functionname+0x34 really is an improvement on $x.12!

lld/test/ELF/aarch64-range-thunk-extension-plt32.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t.o
 // RUN: echo 'SECTIONS { \
 // RUN:         .text.1 0x10000 : { *(.text.1) } \
 // RUN:         .text.2 0x200000000 : AT(0x20000) { *(.text.2) } \
 // RUN:       } ' > %t.script
 // RUN: ld.lld --script %t.script %t.o -o %t
 // RUN: llvm-objdump -d --no-show-raw-insn --print-imm-hex %t | FileCheck %s
 
 // The word should be an offset to the range extension thunk.
 // CHECK-LABEL: <_start>:
 // CHECK-NEXT:    10000:       04 00 00 00     .word   0x00000004
 
 // The thunk redirects to the address of callee.
 // CHECK-LABEL: <__AArch64AbsLongThunk_callee>:
-// CHECK-NEXT:    10004:       ldr     x16, 0x1000c <$d>
+// CHECK-NEXT:    10004:       ldr     x16, 0x1000c <__AArch64AbsLongThunk_callee+0x8>
 // CHECK-NEXT:    10008:       br      x16
 
 // CHECK-LABEL: <$d>:
 // CHECK-NEXT:    1000c:       00 00 00 00     .word   0x00000000
 // CHECK-NEXT:    10010:       02 00 00 00     .word   0x00000002
 
 // CHECK-LABEL: <callee>:
 // CHECK-NEXT:    200000000:      ret

llvm/test/CodeGen/AArch64/callbr-asm-obj-file.ll

 ; RUN: llc < %s -mtriple=aarch64-unknown-linux-gnu -filetype=obj -o - \
 ; RUN:  | llvm-objdump --no-print-imm-hex --triple=aarch64-unknown-linux-gnu -d - \
 ; RUN:  | FileCheck %s
 
 %struct.c = type { i1 (...)* }
 
 @l = common hidden local_unnamed_addr global i32 0, align 4
 
 ; CHECK-LABEL: <test1>:
 ; CHECK-LABEL: <$d.1>:
 ; CHECK-LABEL: <$x.2>:
-; CHECK-NEXT:    b 0x2c <$x.4>
+; CHECK-NEXT:    b 0x2c <test1+0x2c>
 ; CHECK-LABEL: <$x.4>:
 ; CHECK-NEXT:    mov w0, wzr
 ; CHECK-NEXT:    ldr x30, [sp], #16
 ; CHECK-NEXT:    ret
 define hidden i32 @test1() {
   %1 = tail call i32 bitcast (i32 (...)* @g to i32 ()*)()
   %2 = icmp eq i32 %1, 0
   br i1 %2, label %3, label %5

 
 10:                                               ; preds = %7, %0, %6, %9
   ret i32 undef
 }
 
 ; CHECK-LABEL: <test3>:
 ; CHECK-LABEL: <$d.9>:
 ; CHECK-LABEL: <$x.10>:
-; CHECK-NEXT:    b {{.*}} <$x.12>
+; CHECK-NEXT:    b {{.*}} <test3+0x34>
 ; CHECK-LABEL: <$x.12>:
 ; CHECK-NEXT:    ldr x30, [sp], #16
 ; CHECK-NEXT:    ret
 define internal i1 @test3() {
   %1 = tail call i32 bitcast (i32 (...)* @g to i32 ()*)()
   %2 = icmp eq i32 %1, 0
   br i1 %2, label %3, label %5
 

llvm/test/MC/ARM/branch-disassemble.s

 @ RUN: llvm-mc -mcpu=cortex-a9 -triple armv7-arm-none-eabi -filetype obj -o - %s \
 @ RUN:   | llvm-objdump --no-print-imm-hex --mcpu=cortex-a9 --triple=armv7-arm-none-eabi -d - \
 @ RUN:   | FileCheck %s -check-prefix CHECK-ARM
 
 @ RUN: llvm-mc -mcpu=cortex-m3 -triple thumbv7m-arm-none-eabi -filetype obj -o - %s \
 @ RUN:   | llvm-objdump --no-print-imm-hex --mcpu=cortex-m3 --triple=thumbv7m-arm-none-eabi -d - \
 @ RUN:   | FileCheck %s -check-prefix CHECK-THUMB
 
+foo:
 b.w .Lbranch
-@ CHECK-ARM: b 0xc <$a.0+0xc> @ imm = #4
-@ CHECK-THUMB: b.w 0xc <$t.0+0xc> @ imm = #8
+@ CHECK-ARM: b 0xc <foo+0xc> @ imm = #4
+@ CHECK-THUMB: b.w 0xc <foo+0xc> @ imm = #8
 adds r0, r1, #42
 adds r1, r2, #42
 .Lbranch:
 movs r2, r3

llvm/test/MC/Disassembler/ARM/mve-lol.txt

 # RUN: llvm-mc -assemble -triple=thumbv8.1m.main -mattr=+mve -filetype=obj %s -o - 2>&1 | \
 # RUN:   llvm-objdump --no-print-imm-hex -d -r --triple=thumbv8.1m.main --mattr=+mve - | FileCheck %s
 
 # Test to check that we can evaluate branches and their targets, i.e.
-# checking that we see branch targets annotations like <$t.0+0xc> in the
+# checking that we see branch targets annotations like <foo+0xc> in the
 # disassembly.
 
-# CHECK:  wls lr, r3, 0xc <$t.0+0xc>          @ imm = #8
+foo:
+# CHECK:  wls lr, r3, 0xc <foo+0xc>          @ imm = #8
 # CHECK:  vmov  q0, q1
-# CHECK:  le  lr, 0x4 <$t.0+0x4>              @ imm = #-8
+# CHECK:  le  lr, 0x4 <foo+0x4>              @ imm = #-8
 
           wls lr, r3, #8
           vmov  q0, q1
           le  lr, #-8
 
 
-# CHECK:  wlstp.8 lr, r3, 0x18 <$t.0+0x18>    @ imm = #8
+# CHECK:  wlstp.8 lr, r3, 0x18 <foo+0x18>    @ imm = #8
 # CHECK:  vmov  q0, q1
-# CHECK:  letp  lr, 0x10 <$t.0+0x10>          @ imm = #-8
+# CHECK:  letp  lr, 0x10 <foo+0x10>          @ imm = #-8
 
           wlstp.8  lr, r3, #8
           vmov  q0, q1
           letp  lr, #-8
 
 
-# CHECK:  wlstp.16  lr, r3, 0x24 <$t.0+0x24>  @ imm = #8
+# CHECK:  wlstp.16  lr, r3, 0x24 <foo+0x24>  @ imm = #8
 # CHECK:  vmov  q0, q1
-# CHECK:  letp  lr, 0x1c <$t.0+0x1c>          @ imm = #-8
+# CHECK:  letp  lr, 0x1c <foo+0x1c>          @ imm = #-8
 
           wlstp.16  lr, r3, #8
           vmov  q0, q1
           letp  lr, #-8
 
 
-# CHECK:  wlstp.32  lr, r3, 0x30 <$t.0+0x30>  @ imm = #8
+# CHECK:  wlstp.32  lr, r3, 0x30 <foo+0x30>  @ imm = #8
 # CHECK:  vmov  q0, q1
-# CHECK:  letp  lr, 0x28 <$t.0+0x28>          @ imm = #-8
+# CHECK:  letp  lr, 0x28 <foo+0x28>          @ imm = #-8
 
           wlstp.32  lr, r3, #8
           vmov  q0, q1
           letp  lr, #-8
 
 
-# CHECK:  wlstp.64  lr, r3, 0x3c <$t.0+0x3c>  @ imm = #8
+# CHECK:  wlstp.64  lr, r3, 0x3c <foo+0x3c>  @ imm = #8
 # CHECK:  vmov  q0, q1
-# CHECK:  letp  lr, 0x34 <$t.0+0x34>          @ imm = #-8
+# CHECK:  letp  lr, 0x34 <foo+0x34>          @ imm = #-8
 
           wlstp.64  lr, r3, #8
           vmov  q0, q1
           letp  lr, #-8

llvm/tools/llvm-objdump/llvm-objdump.cpp

static bool isCSKYElf(const ObjectFile &Obj) {
   const auto *Elf = dyn_cast<ELFObjectFileBase>(&Obj);
   return Elf && Elf->getEMachine() == ELF::EM_CSKY;
 }
 
 static bool hasMappingSymbols(const ObjectFile &Obj) {
   return isArmElf(Obj) || isAArch64Elf(Obj) || isCSKYElf(Obj) ;
 }
 
+static bool isMappingSymbol(const SymbolInfoTy &Sym) {
+  return Sym.Name.startswith("$d") || Sym.Name.startswith("$x") ||
+         Sym.Name.startswith("$a") || Sym.Name.startswith("$t");
+}
+
 static void printRelocation(formatted_raw_ostream &OS, StringRef FileName,
                             const RelocationRef &Rel, uint64_t Address,
                             bool Is64Bits) {
   StringRef Fmt = Is64Bits ? "%016" PRIx64 ":  " : "%08" PRIx64 ":  ";
   SmallString<16> Name;
   SmallString<32> Val;
   Rel.getTypeName(Name);
   if (Error E = getRelocationValueString(Rel, Val))
for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
               // using the nearest preceding absolute symbol (if any), if there
               // are no other valid symbols.
               const SymbolInfoTy *TargetSym = nullptr;
               for (const SectionSymbolsTy *TargetSymbols :
                    TargetSectionSymbols) {
                 auto It = llvm::partition_point(
                     *TargetSymbols,
                     [=](const SymbolInfoTy &O) { return O.Addr <= Target; });
-                if (It != TargetSymbols->begin()) {
-                  TargetSym = &*(It - 1);
+                while (It != TargetSymbols->begin()) {
+                  --It;
+                  // Skip mapping symbols to avoid possible ambiguity as they
+                  // do not allow uniquely identifying the target address.
+                  if (!hasMappingSymbols(Obj) || !isMappingSymbol(*It)) {
+                    TargetSym = &*It;
                     break;
                   }
                 }
+                if (TargetSym)
+                  break;
+              }
 
               // Print the labels corresponding to the target if there's any.
               bool BBAddrMapLabelAvailable = BBAddrMapLabels.count(Target);
               bool LabelAvailable = AllLabels.count(Target);
               if (TargetSym != nullptr) {
                 uint64_t TargetAddress = TargetSym->Addr;
                 uint64_t Disp = Target - TargetAddress;
                 std::string TargetName = TargetSym->Name.str();