[XCOFF][llvm-objdump] change the priority of symbols with the same address by symbol types.

[Esme]

Summary

In XCOFF, each section comes with a default symbol with the same name as the section. It doesn't bind to code locations and it may cause incorrect display of symbol names under llvm-objdump -d.
For example, here is a scenario where the text section has three symbols, i.e.

00000000 .text // .text is a default symbol for text section.
00000000 .foo
0000001c .foo2

The symbols in the same section are sorted in ascending order. ie.

00000000 .foo
00000000 .text // .text has higher priority than .foo because we compare them by their names.
0000001c .foo2

llvm-objdump -d will choose the highest priority symbol to display when there are symbols with the same address.

00000000 <.text>: // incorrect here; <.foo> is expected.
...
0000001c <.foo2>:
...

This patch will change the priority of symbols with the same address by symbol type.

[shchenz]

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

1144

Only handle .text is not enough I think. We can explicitly declare the section names in sthe ource code. For example:

int __attribute__((section ("explicit_sec"))) foo(int a)
{
          return a;
}
int __attribute__((section ("explicit_sec2")))  foo1(int a)
{
          return a;
}

int  __attribute__((section ("explicit_sec"))) foo2(int a)
{
          return a;
}

int __attribute__((section ("explicit_sec2"))) foo3(int a)
{
          return a;
}
int   foo4(int a)
{
          return a;
}
int   foo5(int a)
{
          return a;
}

Your patch can fix the first function in .text section, but can not fix the first function in explicit_sec2 or explicit_sec?

[jhenderson]

The problem is, as I understand it, that the XCOFF symbol type enum isn't ordered in a convenient symbol-priority order, right?

[Esme]

In D117642#3286951, @jhenderson wrote:

The problem is, as I understand it, that the XCOFF symbol type enum isn't ordered in a convenient symbol-priority order, right?

Hmm, I think the problem is the symbol type should have a higher priority than the symbol name size during sorting (at least for XCOFF).

[Esme]

Make the SymbolRef::Type in a convenient symbol-priority order.

[jhenderson]

Looks reasonable to me. Might be worth getting someone with XCOFF familiarity to review too though.

[shchenz]

LGTM too. Thanks for fixing this.

llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h

 private:
   bool IsXCOFF;
 
 public:
   SymbolInfoTy(uint64_t Addr, StringRef Name,
                Optional<XCOFF::StorageMappingClass> Smc, Optional<uint32_t> Idx,
                bool Label)
       : Addr(Addr), Name(Name), XCOFFSymInfo(Smc, Idx, Label), IsXCOFF(true) {}
-  SymbolInfoTy(uint64_t Addr, StringRef Name, uint8_t Type)
-      : Addr(Addr), Name(Name), Type(Type), IsXCOFF(false) {}
+  SymbolInfoTy(uint64_t Addr, StringRef Name, uint8_t Type,
+               bool IsXCOFF = false)
+      : Addr(Addr), Name(Name), Type(Type), IsXCOFF(IsXCOFF) {}
   bool isXCOFF() const { return IsXCOFF; }
 
 private:
   friend bool operator<(const SymbolInfoTy &P1, const SymbolInfoTy &P2) {
     assert(P1.IsXCOFF == P2.IsXCOFF &&
            "P1.IsXCOFF should be equal to P2.IsXCOFF.");
     if (P1.IsXCOFF)
-      return std::tie(P1.Addr, P1.XCOFFSymInfo, P1.Name) <
-             std::tie(P2.Addr, P2.XCOFFSymInfo, P2.Name);
+      return std::tie(P1.Addr, P1.XCOFFSymInfo, P1.Type, P1.Name) <
+             std::tie(P2.Addr, P2.XCOFFSymInfo, P2.Type, P2.Name);
 
     return std::tie(P1.Addr, P1.Name, P1.Type) <
-             std::tie(P2.Addr, P2.Name, P2.Type);
   }
 };
 
 using SectionSymbolsTy = std::vector<SymbolInfoTy>;
 
 template <typename T> class ArrayRef;
 class MCContext;
 class MCInst;

llvm/include/llvm/Object/ObjectFile.h

 /// This is a value type class that represents a single symbol in the list of
 /// symbols in the object file.
 class SymbolRef : public BasicSymbolRef {
   friend class SectionRef;
 
 public:
   enum Type {
     ST_Unknown, // Type not specified
+    ST_Other,
     ST_Data,
     ST_Debug,
     ST_File,
     ST_Function,
-    ST_Other
   };
 
   SymbolRef() = default;
   SymbolRef(DataRefImpl SymbolP, const ObjectFile *Owner);
   SymbolRef(const BasicSymbolRef &B) : BasicSymbolRef(B) {
     assert(isa<ObjectFile>(BasicSymbolRef::getObject()));
   }
 

llvm/test/CodeGen/PowerPC/aix-prefixed-instruction-boundary.mir

bb.0.entry:
     renamable $x3 = PADDI8 $x3, 13
     renamable $x3 = LI8 2
 
     renamable $x3 = PADDI8 $x3, 13
     BLR8 implicit $lr8, implicit $rm, implicit killed $x3
 ...
 
 # DIS:      Disassembly of section .text:
-# DIS:      00000000 <.text>:
+# DIS:      00000000 <.aix-prefixed-instruction-boundary>:
 # DIS-NEXT:   0: 38 60 00 02              li 3, 2
 # DIS-NEXT:   4: 06 00 00 00 38 63 00 0d  paddi 3, 3, 13, 0
 # DIS-NEXT:   c: 06 00 00 00 38 63 00 0d  paddi 3, 3, 13, 0
 # DIS-NEXT:  14: 06 00 00 00 38 63 00 0d  paddi 3, 3, 13, 0
 # DIS-NEXT:  1c: 06 00 00 00 38 63 00 0d  paddi 3, 3, 13, 0
 # DIS-NEXT:  24: 06 00 00 00 38 63 00 0d  paddi 3, 3, 13, 0
 # DIS-NEXT:  2c: 06 00 00 00 38 63 00 0d  paddi 3, 3, 13, 0
 # DIS-NEXT:  34: 06 00 00 00 38 63 00 0d  paddi 3, 3, 13, 0

llvm/test/CodeGen/PowerPC/aix-return55.ll

 define dso_local signext i32 @foo() {
 entry:
   ret i32 55
 ; CHECK-LABEL: .foo:
 ; CHECK: li 3, 55
 ; CHECK: blr
 }
 
-;CHECKOBJ:      00000000 <.text>:
+;CHECKOBJ:      00000000 <.foo>:
 ;CHECKOBJ-NEXT:       0: 38 60 00 37                    li 3, 55
 ;CHECKOBJ-NEXT:       4: 4e 80 00 20                    blr{{[[:space:]] *}}
 ;CHECKOBJ-NEXT: 00000008 <.rodata.str1.1>:
 ;CHECKOBJ-NEXT:       8: 68 65 6c 6c                   xori 5, 3, 27756
 ;CHECKOBJ-NEXT:       c: 6f 77 6f 72 xoris 23, 27, 28530
 ;CHECKOBJ-NEXT:      10: 0a 00 00 00 tdlti 0, 0{{[[:space:]] *}}
 ;CHECKOBJ-NEXT: Disassembly of section .data:{{[[:space:]] *}}
 ;CHECKOBJ-NEXT: 00000018 <a>:

llvm/test/CodeGen/PowerPC/aix-user-defined-memcpy.ll

 ; 32-REL-NEXT:    Type: R_POS (0x0)
 ; 32-REL-NEXT:  }
 ; 32-REL-NEXT:  }
 ; 32-REL-NEXT: ]
 
 ; 32-REL-NOT:  Type: R_RBR (0x1A)
 
 ; 32-DIS:      Disassembly of section .text:
-; 32-DIS:      00000000 <.text>:
+; 32-DIS:      00000000 <.memcpy>:
 ; 32-DIS-NEXT:        0: 38 60 00 03                   li 3, 3
 ; 32-DIS-NEXT:        4: 4e 80 00 20                   blr
 ; 32-DIS-NEXT:        8: 60 00 00 00                   nop
 ; 32-DIS-NEXT:        c: 60 00 00 00                   nop
 ; 32-DIS:      00000010 <.call_memcpy>:
 ; 32-DIS-NEXT:       10: 7c 08 02 a6                   mflr 0
 ; 32-DIS-NEXT:       14: 90 01 00 08                   stw 0, 8(1)
 ; 32-DIS-NEXT:       18: 94 21 ff c0                   stwu 1, -64(1)
 ; 32-DIS-NEXT:       1c: 4b ff ff e5                   bl 0x0
 ; 32-DIS-NEXT:       20: 60 00 00 00                   nop
 ; 32-DIS-NEXT:       24: 38 21 00 40                   addi 1, 1, 64
 ; 32-DIS-NEXT:       28: 80 01 00 08                   lwz 0, 8(1)
 ; 32-DIS-NEXT:       2c: 7c 08 03 a6                   mtlr 0
 ; 32-DIS-NEXT:       30: 4e 80 00 20                   blr
 
 ; 64-CHECK: LLVM ERROR: 64-bit XCOFF object files are not supported yet.

llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll

 
 ;CHECK-NEXT:         .align  3
 ;CHECK-NEXT: L..__const.main.cnst4:
 ;CHECK-NEXT:         .vbyte 2, 16392                   # 0x4008
 ;CHECK-NEXT:         .byte   0                       # 0x0
 ;CHECK-NEXT:         .space  1
 
 
-;CHECKOBJ:      00000000 <.text>:
+;CHECKOBJ:      00000000 <.main>:
 ;CHECKOBJ-NEXT:        0: 38 60 00 00                    li 3, 0
 ;CHECKOBJ-NEXT:        4: 4e 80 00 20                    blr
 ;CHECKOBJ-NEXT:          ...{{[[:space:]] *}}
 ;CHECKOBJ-NEXT: 00000010 <.rodata>:
 ;CHECKOBJ-NEXT:        10: 40 00 00 00
 ;CHECKOBJ-NEXT:        14: 00 00 00 32
 ;CHECKOBJ-NEXT:          ...{{[[:space:]] *}}
 ;CHECKOBJ-SAME:       30: 40 00 00 00

llvm/test/CodeGen/PowerPC/aix-xcoff-reloc-symb.mir

 # SYM-NEXT:      StorageMappingClass: XMC_RO (0x1)
 # SYM-NEXT:      StabInfoIndex: 0x0
 # SYM-NEXT:      StabSectNum: 0x0
 # SYM-NEXT:    }
 # SYM-NEXT:  }
 
 # DIS:      Disassembly of section .text:
 # DIS-EMPTY:
-# DIS-NEXT: 00000000 <.text>:
+# DIS-NEXT: 00000000 <.foo>:
 # DIS-NEXT:        0: 80 62 00 00                   lwz 3, 0(2)
 # DIS-NEXT:        4: 4e 80 00 20                   blr
 # DIS-EMPTY:
 # DIS-NEXT: 00000008 <.rodata>:
 # DIS-NEXT:        8: ff ff ff f8                   fmsub 31, 31, 31, 31
 # DIS-EMPTY:

llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll

 ; SYM-NEXT:       StabSectNum: 0x0
 ; SYM-NEXT:     }
 ; SYM-NEXT:   }
 ; SYM-NEXT: ]
 
 
 ; DIS:      {{.*}}aix-xcoff-reloc.ll.tmp.o:   file format aixcoff-rs6000
 ; DIS:      Disassembly of section .text:
-; DIS:      00000000 <.text>:
+; DIS:      00000000 <.foo>:
 ; DIS-NEXT:        0: 7c 08 02 a6                   mflr 0
 ; DIS-NEXT:        4: 90 01 00 08                   stw 0, 8(1)
 ; DIS-NEXT:        8: 94 21 ff c0                   stwu 1, -64(1)
 ; DIS-NEXT:        c: 38 60 00 01                   li 3, 1
 ; DIS-NEXT:       10: 4b ff ff f1                   bl 0x0
 ; DIS-NEXT:       14: 60 00 00 00                   nop
 ; DIS-NEXT:       18: 80 82 00 00                   lwz 4, 0(2)
 ; DIS-NEXT:       1c: 80 a2 00 04                   lwz 5, 4(2)

llvm/test/CodeGen/PowerPC/aix-xcoff-textdisassembly.ll

 ; RUN: llc -verify-machineinstrs -mcpu=pwr9 -mattr=-altivec -mtriple powerpc-ibm-aix-xcoff \
 ; RUN:     -xcoff-traceback-table=false -filetype=obj -o %t.o  < %s
 ; RUN: llvm-objdump -D %t.o | FileCheck --check-prefix=CHECK %s
 
 define i32 @foo() #0 {
 entry:
   ret i32 0
 }
 
 define i32 @foo1() #0 {
 entry:
   ret i32 1
 }
 
 ; CHECK:     Disassembly of section .text:{{[[:space:]] *}}
-; CHECK-NEXT:     00000000 <.text>:
+; CHECK-NEXT:     00000000 <.foo>:
 ; CHECK-NEXT:        0: 38 60 00 00                   li 3, 0
 ; CHECK-NEXT:        4: 4e 80 00 20                   blr
 ; CHECK-NEXT:        8: 60 00 00 00                   nop
 ; CHECK-NEXT:        c: 60 00 00 00                   nop
 ; CHECK:     00000010 <.foo1>:
 ; CHECK-NEXT:       10: 38 60 00 01                   li 3, 1
 ; CHECK-NEXT:       14: 4e 80 00 20                   blr

llvm/test/tools/llvm-objdump/XCOFF/disassemble-all.test

 ## __thread double d = 3.14159;
 ##
 ## int func(void)  {
 ##   return a;
 ## }
 
 CHECK:        Inputs/xcoff-section-headers.o: file format aixcoff-rs6000
 CHECK:        Disassembly of section .text:
-CHECK:        00000000 <.text>:
+CHECK:        00000000 <.func>:
 CHECK-NEXT:        0: 80 62 00 04                   lwz 3, 4(2)
 WITH-R-NEXT:                         00000002:  R_TOC        a
 CHECK-NEXT:        4: 80 63 00 00                   lwz 3, 0(3)
 CHECK-NEXT:        8: 4e 80 00 20                   blr
 CHECK-NEXT:        c: 00 00 00 00                   <unknown>
 CHECK-NEXT:       10: 00 00 20 40                   <unknown>
 CHECK-NEXT:       14: 00 00 00 01                   <unknown>
 CHECK-NEXT:       18: 00 00 00 0c                   <unknown>

llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-description.test

 ## __thread double d = 3.14159;
 ##
 ## int func(void)  {
 ##   return a;
 ## }
 
 COMMON: Inputs/xcoff-section-headers.o: file format aixcoff-rs6000
 COMMON: Disassembly of section .text:
-PLAIN:      00000000 <.text>:
+PLAIN:      00000000 <.func>:
 DESC:       00000000 (idx: 16) .func: 
 COMMON-NEXT:        0: 80 62 00 04                    lwz 3, 4(2)
 RELOC:                              00000002:  R_TOC        (idx: 26) a[TC]
 COMMON-NEXT:        4: 80 63 00 00                    lwz 3, 0(3)
 COMMON-NEXT:        8: 4e 80 00 20                    blr
 COMMON-NEXT:        c: 00 00 00 00                    <unknown>
 COMMON-NEXT:       10: 00 00 20 40                    <unknown>
 COMMON-NEXT:       14: 00 00 00 01                    <unknown>

llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-priority.ll

+; RUN: llc -mtriple=powerpc-ibm-aix-xcoff %s -filetype=obj -o %t
+; RUN: llvm-objdump %t -d --no-show-raw-insn | FileCheck %s
+
+; CHECK: Disassembly of section .text:
+; CHECK: 00000000 <.foo3>:
+; CHECK: 00000020 <.foo4>:
+; CHECK: 00000040 <.foo>:
+; CHECK: 00000060 <.foo2>:
+
+define dso_local signext i32 @foo(i32 noundef signext %a) #0 section "explicit_sec" {
+entry:
+  ret i32 %a
+}
+
+define dso_local signext i32 @foo2(i32 noundef signext %a) #0 section "explicit_sec" {
+entry:
+  ret i32 %a
+}
+
+define dso_local signext i32 @foo3(i32 noundef signext %a) #0 {
+entry:
+  ret i32 %a
+}
+
+define dso_local signext i32 @foo4(i32 noundef signext %a) #0 {
+entry:
+  ret i32 %a
+}

llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbolize-operands.ll

 ; RUN: llc -mtriple=powerpc-ibm-aix-xcoff %s -filetype=obj -o %t
 ; RUN: llvm-objdump %t -d --symbolize-operands --no-show-raw-insn \
 ; RUN:   | FileCheck %s
 
 ;; Expect to find the branch labels.
-; CHECK-LABEL: <.text>:
-;; TODO: <.internal> should be printed instead of <.text>.
+; CHECK-LABEL: <.internal>:
 ; CHECK-NEXT:         0:      mr 4, 3
 ; CHECK-NEXT:         4:      li 3, 0
 ; CHECK-NEXT:         8:      mtctr 4
 ; CHECK-NEXT:  <L0>:
 ; CHECK-NEXT:         c:      addi 3, 3, 1
 ; CHECK-NEXT:        10:      bdnz 0xc <L0>
 ; CHECK-NEXT:        14:      blr
 
 ; CHECK-LABEL: <.foo>:
 ; CHECK:             5c:        b 0x64 <L0>
 ; CHECK-NEXT:  <L2>:
 ; CHECK-NEXT:        60:        bf  8, 0x84 <L1>
 ; CHECK-NEXT:  <L0>:
 ; CHECK-NEXT:        64:        mr  3, 31
-; CHECK-NEXT:        68:        bl 0x0 <.text>
+; CHECK-NEXT:        68:        bl 0x0 <.internal>
 ; CHECK-NEXT:        6c:        mr  31, 3
 ; CHECK-NEXT:        70:        cmplwi  3, 11
 ; CHECK-NEXT:        74:        bt  0, 0x60 <L2>
-; CHECK-NEXT:        78:        bl 0x0 <.text>
+; CHECK-NEXT:        78:        bl 0x0 <.internal>
 ; CHECK-NEXT:        7c:        nop
 ; CHECK-NEXT:        80:        b 0x60 <L2>
 ; CHECK-NEXT:  <L1>:
 ; CHECK-NEXT:        84:        lwz 31, 60(1)
 
 define internal i32 @internal(i32 %a) {
 entry:
   br label %for.body

llvm/test/tools/llvm-objdump/XCOFF/print-linenumber.test

 ## basic.c:
 ## int main(void)
 ## {
 ##   return 0;
 ## }
 
 # LINES32:       Inputs/basic32.o:  file format aixcoff-rs6000
 # LINES32:       Disassembly of section .text:
-# LINES32:       00000000 <.text>:
+# LINES32:       00000000 <.main>:
 # LINES32:       ; .main():
 # LINES32-NEXT:  ; /basic.c:1
 # LINES32-NEXT:         0: 38 60 00 00    li 3, 0
 # LINES32-NEXT:  ; /basic.c:3
 # LINES32-NEXT:         4: 48 00 00 04    b 0x8
 # LINES32-NEXT:  ; /basic.c:4
 # LINES32-NEXT:         8: 4e 80 00 20    blr
 # LINES32-NEXT:         c: 00 00 00 00    <unknown>

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

if (Obj->isXCOFF() && SymbolDescription) {
     const auto *XCOFFObj = cast<XCOFFObjectFile>(Obj);
     DataRefImpl SymbolDRI = Symbol.getRawDataRefImpl();
 
     const uint32_t SymbolIndex = XCOFFObj->getSymbolIndex(SymbolDRI.p);
     Optional<XCOFF::StorageMappingClass> Smc =
         getXCOFFSymbolCsectSMC(XCOFFObj, Symbol);
     return SymbolInfoTy(Addr, Name, Smc, SymbolIndex,
                         isLabel(XCOFFObj, Symbol));
+  } else if (Obj->isXCOFF()) {
+    const SymbolRef::Type SymType = unwrapOrError(Symbol.getType(), FileName);
+    return SymbolInfoTy(Addr, Name, SymType, true);
   } else
     return SymbolInfoTy(Addr, Name,
                         Obj->isELF() ? getElfSymbolType(Obj, Symbol)
                                      : (uint8_t)ELF::STT_NOTYPE);
 }
 
 static SymbolInfoTy createDummySymbolInfo(const ObjectFile *Obj,
                                           const uint64_t Addr, StringRef &Name,
if (!NameOrErr) {
       continue;
     }
     if (NameOrErr->empty() && !(Obj->isXCOFF() && SymbolDescription))
       continue;
 
     if (Obj->isELF() && getElfSymbolType(Obj, Symbol) == ELF::STT_SECTION)
       continue;
 
     if (MachO) {
       // __mh_(execute|dylib|dylinker|bundle|preload|object)_header are special
       // symbols that support MachO header introspection. They do not bind to
       // code locations and are irrelevant for disassembly.
       if (NameOrErr->startswith("__mh_") && NameOrErr->endswith("_header"))
         continue;
       // Don't ask a Mach-O STAB symbol for its section unless you know that
       // STAB symbol's section field refers to a valid section index. Otherwise
       // the symbol may error trying to load a section that does not exist.