[XCOFF][llvm-readobj] Print symbol value kind when dumping symbols

[diggerlin]

llvm-readobj print out symbol value name for xcoff symbol table.

reference doc: https://www.ibm.com/docs/en/aix/7.2?topic=formats-xcoff-object-file-format#XCOFF__yaa3i18fjbau

[llvmbot]

@llvm/pr-subscribers-llvm-binary-utilities

Author: zhijian lin (diggerlin)

Changes

llvm-readobj print out symbol value name for xcoff symbol table

---

Full diff: https://github.com/llvm/llvm-project/pull/125861.diff

2 Files Affected:

• (modified) llvm/test/tools/llvm-readobj/XCOFF/symbols.test (+105)
• (modified) llvm/tools/llvm-readobj/XCOFFDumper.cpp (+7-3)

diff --git a/llvm/test/tools/llvm-readobj/XCOFF/symbols.test b/llvm/test/tools/llvm-readobj/XCOFF/symbols.test
index 71347a85f1ba55..f298210e9611fc 100644
--- a/llvm/test/tools/llvm-readobj/XCOFF/symbols.test
+++ b/llvm/test/tools/llvm-readobj/XCOFF/symbols.test
@@ -162,6 +162,48 @@ Symbols:
         SectionOrLength:        256
         StabInfoIndex:          44
         StabSectNum:            55
+  - Name:               .fun3
+    Value:              0x0
+    Section:            N_DEBUG
+    Type:               0x20
+    StorageClass:       C_WEAKEXT
+
+  - Name:              stsym
+    Value:              0x05
+    Section:            .text
+    Type:               0x00
+    StorageClass:       C_STSYM
+
+  - Name:              bincl
+    Value:              0x06
+    Section:            .text
+    Type:               0x00
+    StorageClass:       C_BINCL
+
+  - Name:              lsym
+    Value:              0x07
+    Section:            .text
+    Type:               0x00
+    StorageClass:       C_LSYM
+
+  - Name:              rsym
+    Value:              0x08
+    Section:            .text
+    Type:               0x00
+    StorageClass:       C_RSYM
+
+  - Name:              ecoml
+    Value:              0x09
+    Section:            .text
+    Type:               0x00
+    StorageClass:       C_ECOML
+
+  - Name:              cinfo
+    Value:              0x02
+    Section:            .text
+    Type:               0x00
+    StorageClass:       C_INFO
+
 
 # SYMBOL32:      Symbols [
 # SYMBOL32-NEXT:   Symbol {
@@ -368,4 +410,67 @@ Symbols:
 # SYMBOL32-NEXT:       StabSectNum: 0x37
 # SYMBOL32-NEXT:     }
 # SYMBOL32-NEXT:   }
+# SYMBOL32-NEXT:   Symbol {
+# SYMBOL32-NEXT:     Index: 25
+# SYMBOL32-NEXT:     Name: .fun3
+# SYMBOL32-NEXT:     Value (RelocatableAddress): 0x0
+# SYMBOL32-NEXT:     Section: N_DEBUG
+# SYMBOL32-NEXT:     Type: 0x20
+# SYMBOL32-NEXT:     StorageClass: C_WEAKEXT (0x6F)
+# SYMBOL32-NEXT:     NumberOfAuxEntries: 0
+# SYMBOL32-NEXT:   }
+# SYMBOL32-NEXT:   Symbol {
+# SYMBOL32-NEXT:     Index: 26
+# SYMBOL32-NEXT:     Name: Unimplemented Debug Name
+# SYMBOL32-NEXT:     Value (OffsetInCSect): 0x5
+# SYMBOL32-NEXT:     Section: .text
+# SYMBOL32-NEXT:     Type: 0x0
+# SYMBOL32-NEXT:     StorageClass: C_STSYM (0x85)
+# SYMBOL32-NEXT:     NumberOfAuxEntries: 0
+# SYMBOL32-NEXT:   }
+# SYMBOL32-NEXT:   Symbol {
+# SYMBOL32-NEXT:     Index: 27
+# SYMBOL32-NEXT:     Name: bincl
+# SYMBOL32-NEXT:     Value (OffsetInFile): 0x6
+# SYMBOL32-NEXT:     Section: .text
+# SYMBOL32-NEXT:     Type: 0x0
+# SYMBOL32-NEXT:     StorageClass: C_BINCL (0x6C)
+# SYMBOL32-NEXT:     NumberOfAuxEntries: 0
+# SYMBOL32-NEXT:   }
+# SYMBOL32-NEXT:   Symbol {
+# SYMBOL32-NEXT:     Index: 28
+# SYMBOL32-NEXT:     Name: Unimplemented Debug Name
+# SYMBOL32-NEXT:     Value (OffsetRelToStackFrame): 0x7
+# SYMBOL32-NEXT:     Section: .text
+# SYMBOL32-NEXT:     Type: 0x0
+# SYMBOL32-NEXT:     StorageClass: C_LSYM (0x81)
+# SYMBOL32-NEXT:     NumberOfAuxEntries: 0
+# SYMBOL32-NEXT:   }
+# SYMBOL32-NEXT:   Symbol {
+# SYMBOL32-NEXT:     Index: 29
+# SYMBOL32-NEXT:     Name: Unimplemented Debug Name
+# SYMBOL32-NEXT:     Value (RegisterNumber): 0x8
+# SYMBOL32-NEXT:     Section: .text
+# SYMBOL32-NEXT:     Type: 0x0
+# SYMBOL32-NEXT:     StorageClass: C_RSYM (0x83)
+# SYMBOL32-NEXT:     NumberOfAuxEntries: 0
+# SYMBOL32-NEXT:   }
+# SYMBOL32-NEXT:   Symbol {
+# SYMBOL32-NEXT:     Index: 30
+# SYMBOL32-NEXT:     Name: Unimplemented Debug Name
+# SYMBOL32-NEXT:     Value (OffsetInCommBlock): 0x9
+# SYMBOL32-NEXT:     Section: .text
+# SYMBOL32-NEXT:     Type: 0x0
+# SYMBOL32-NEXT:     StorageClass: C_ECOML (0x88)
+# SYMBOL32-NEXT:     NumberOfAuxEntries: 0
+# SYMBOL32-NEXT:   }
+# SYMBOL32-NEXT:   Symbol {
+# SYMBOL32-NEXT:     Index: 31
+# SYMBOL32-NEXT:     Name: cinfo
+# SYMBOL32-NEXT:     Value (OffsetInCommentSection): 0x2
+# SYMBOL32-NEXT:     Section: .text
+# SYMBOL32-NEXT:     Type: 0x0
+# SYMBOL32-NEXT:     StorageClass: C_INFO (0x6E)
+# SYMBOL32-NEXT:     NumberOfAuxEntries: 0
+# SYMBOL32-NEXT:   }
 # SYMBOL32-NEXT: ]
diff --git a/llvm/tools/llvm-readobj/XCOFFDumper.cpp b/llvm/tools/llvm-readobj/XCOFFDumper.cpp
index 6a099c08e1acad..03764e9ba483da 100644
--- a/llvm/tools/llvm-readobj/XCOFFDumper.cpp
+++ b/llvm/tools/llvm-readobj/XCOFFDumper.cpp
@@ -692,22 +692,26 @@ static StringRef GetSymbolValueName(XCOFF::StorageClass SC) {
   case XCOFF::C_BLOCK:
     return "Value (RelocatableAddress)";
   case XCOFF::C_FILE:
+  case XCOFF::C_BSTAT:
     return "Value (SymbolTableIndex)";
   case XCOFF::C_DWARF:
     return "Value (OffsetInDWARF)";
   case XCOFF::C_FUN:
   case XCOFF::C_STSYM:
+    return "Value (OffsetInCSect)";
   case XCOFF::C_BINCL:
   case XCOFF::C_EINCL:
+    return "Value (OffsetInFile)";
   case XCOFF::C_INFO:
-  case XCOFF::C_BSTAT:
+    return "Value (OffsetInCommentSection)";
   case XCOFF::C_LSYM:
   case XCOFF::C_PSYM:
+    return "Value (OffsetRelToStackFrame)";
   case XCOFF::C_RPSYM:
   case XCOFF::C_RSYM:
+    return "Value (RegisterNumber)";
   case XCOFF::C_ECOML:
-    assert(false && "This StorageClass for the symbol is not yet implemented.");
-    return "";
+    return "Value (OffsetInCommBlock)";
   default:
     return "Value";
   }

[jh7370]

I'd suggest you change the PR title to something like the following:
"[XCOFF][llvm-readobj] Print symbol value kind when dumping symbols"

For the description, just include the second paragraph (the first paragraph just mirrors what the PR title says, so is redundant).

Should symbols64.test be updated in the same manner in this PR?

You've been somewhat inconsistent in which symbol classes you've tested the kind for. I'd suggest writing full coverage for the switch, so have highlighted the different things that need tackling, I believe.

[jh7370]

This symbol seems unnecessary. You already have plenty of C_WEAKEXT symbols in this file demonstrating the behaviour for this symbol class.

[jh7370]

No symbol with C_BSTAT in the test?

[jh7370]

I know this hasn't changed, but since you're extending the test anyway, C_FUN doesn't have coverage in the test.

[jh7370]

No coverage for C_EINCL?

[diggerlin]

since we has a test for C_BINCL and XCOFF::C_EINCL has the same code with XCOF:: C_BINCL , do we need to add a test for XCOFF::C_EINCL?

[jh7370]

No coverage for C_PSYM?

[jh7370]

No coverage for C_RPSYM?