[PATCH] D72973: [llvm-objdump] Use symbol index+symbol name + storage mapping class as label for -D

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:63
+void llvm::objdump::printXCOFFSymbolDescription(const SymbolInfoTy &SymbolInfo,
+                                                StringRef SymbolName) {
+  // Dummy symbols have no symbol index.
----------------
Add a way to assert `SymbolInfo.XCOFFSymInfo.isXCOFF`. Maybe a `getXCOFFSymInfo` function that asserts `isXCOFF`.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1210
     StringRef Name = unwrapOrError(Symbol.getName(), FileName);
     if (Name.empty())
       continue;
----------------
DiggerLin wrote:
> hubert.reinterpretcast wrote:
> > With descriptive printing, we can print at least the index for a symbol with an empty name?
> I am not sure there is a  symbol with an empty name in symbol table ?
```
extern const char *const str = "Hello, world!";
```

Compiled with:
```
IBM XL C/C++ for AIX, V16.1.0  (5725-C72, 5765-J12)
Version: 16.01.0000.0004
```

Has this:
```
                        ***Relocation Information***
             Vaddr      Symndx  Sign  Fixup     Len      Type  Name
.data:
        0x00000000  0x0000000e     0      0  0x001f   Pos_Rel  str
        0x00000004  0x0000000c     0      0  0x001f   Pos_Rel  **No Symbol**
        0x00000008  0x0000000c     0      0  0x001f   Pos_Rel  **No Symbol**
```
[ ... ]
```
[12]    m   0x0000000c     .data     1  unamex                    **No Symbol**
[13]    a4  0x0000000e       0    0     SD       RO    0    0
```


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1168
+    Optional<XCOFF::StorageMappingClass> Smc = getXCOFFSymbolCsectSMC(
+        static_cast<const XCOFFObjectFile *>(Obj), Symbol);
+    return SymbolInfoTy(Addr, Name, Smc, SymbolIndex,
----------------
We can use `XCOFFObj` instead of casting `Obj` again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72973