[XCOFF][obj2yaml] support parsing auxiliary symbols for XCOFF #70642

EsmeYi · 2023-10-30T10:13:11Z

This PR adds the support for parsing auxiliary symbols of XCOFF object file for obj2yaml.

…j2yaml.

llvmbot · 2023-10-30T10:14:51Z

@llvm/pr-subscribers-objectyaml

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

Author: Esme (EsmeYi)

Changes

This PR adds the support for parsing auxiliary symbols of XCOFF object file for obj2yaml.

Patch is 25.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/70642.diff

5 Files Affected:

(modified) llvm/include/llvm/Object/XCOFFObjectFile.h (+1-1)
(modified) llvm/lib/ObjectYAML/XCOFFYAML.cpp (+17-10)
(modified) llvm/test/tools/obj2yaml/XCOFF/aix.yaml (+38)
(added) llvm/test/tools/obj2yaml/XCOFF/aux-symbols.yaml (+264)
(modified) llvm/tools/obj2yaml/xcoff2yaml.cpp (+184-1)

diff --git a/llvm/include/llvm/Object/XCOFFObjectFile.h b/llvm/include/llvm/Object/XCOFFObjectFile.h
index 63064abb4d3c322..9a3b4aa2623551c 100644
--- a/llvm/include/llvm/Object/XCOFFObjectFile.h
+++ b/llvm/include/llvm/Object/XCOFFObjectFile.h
@@ -411,13 +411,13 @@ class XCOFFCsectAuxRef {
     return Entry64->AuxType;
   }
 
-private:
   uint8_t getSymbolAlignmentAndType() const {
     return GETVALUE(SymbolAlignmentAndType);
   }
 
 #undef GETVALUE
 
+private:
   const XCOFFCsectAuxEnt32 *Entry32 = nullptr;
   const XCOFFCsectAuxEnt64 *Entry64 = nullptr;
 };
diff --git a/llvm/lib/ObjectYAML/XCOFFYAML.cpp b/llvm/lib/ObjectYAML/XCOFFYAML.cpp
index 44ef33501b65e43..a9af0405546a3c7 100644
--- a/llvm/lib/ObjectYAML/XCOFFYAML.cpp
+++ b/llvm/lib/ObjectYAML/XCOFFYAML.cpp
@@ -282,45 +282,53 @@ static void auxSymMapping(IO &IO, XCOFFYAML::SectAuxEntForStat &AuxSym) {
 
 void MappingTraits<std::unique_ptr<XCOFFYAML::AuxSymbolEnt>>::mapping(
     IO &IO, std::unique_ptr<XCOFFYAML::AuxSymbolEnt> &AuxSym) {
-  assert(!IO.outputting() && "We don't dump aux symbols currently.");
   const bool Is64 =
       static_cast<XCOFFYAML::Object *>(IO.getContext())->Header.Magic ==
       (llvm::yaml::Hex16)XCOFF::XCOFF64;
   XCOFFYAML::AuxSymbolType AuxType;
+  if (IO.outputting())
+    AuxType = AuxSym.get()->Type;
   IO.mapRequired("Type", AuxType);
   switch (AuxType) {
   case XCOFFYAML::AUX_EXCEPT:
     if (!Is64)
       IO.setError("an auxiliary symbol of type AUX_EXCEPT cannot be defined in "
                   "XCOFF32");
-    AuxSym.reset(new XCOFFYAML::ExcpetionAuxEnt());
+    if (!IO.outputting())
+      AuxSym.reset(new XCOFFYAML::ExcpetionAuxEnt());
     auxSymMapping(IO, *cast<XCOFFYAML::ExcpetionAuxEnt>(AuxSym.get()));
     break;
   case XCOFFYAML::AUX_FCN:
-    AuxSym.reset(new XCOFFYAML::FunctionAuxEnt());
+    if (!IO.outputting())
+      AuxSym.reset(new XCOFFYAML::FunctionAuxEnt());
     auxSymMapping(IO, *cast<XCOFFYAML::FunctionAuxEnt>(AuxSym.get()), Is64);
     break;
   case XCOFFYAML::AUX_SYM:
-    AuxSym.reset(new XCOFFYAML::BlockAuxEnt());
+    if (!IO.outputting())
+      AuxSym.reset(new XCOFFYAML::BlockAuxEnt());
     auxSymMapping(IO, *cast<XCOFFYAML::BlockAuxEnt>(AuxSym.get()), Is64);
     break;
   case XCOFFYAML::AUX_FILE:
-    AuxSym.reset(new XCOFFYAML::FileAuxEnt());
+    if (!IO.outputting())
+      AuxSym.reset(new XCOFFYAML::FileAuxEnt());
     auxSymMapping(IO, *cast<XCOFFYAML::FileAuxEnt>(AuxSym.get()));
     break;
   case XCOFFYAML::AUX_CSECT:
-    AuxSym.reset(new XCOFFYAML::CsectAuxEnt());
+    if (!IO.outputting())
+      AuxSym.reset(new XCOFFYAML::CsectAuxEnt());
     auxSymMapping(IO, *cast<XCOFFYAML::CsectAuxEnt>(AuxSym.get()), Is64);
     break;
   case XCOFFYAML::AUX_SECT:
-    AuxSym.reset(new XCOFFYAML::SectAuxEntForDWARF());
+    if (!IO.outputting())
+      AuxSym.reset(new XCOFFYAML::SectAuxEntForDWARF());
     auxSymMapping(IO, *cast<XCOFFYAML::SectAuxEntForDWARF>(AuxSym.get()));
     break;
   case XCOFFYAML::AUX_STAT:
     if (Is64)
       IO.setError(
           "an auxiliary symbol of type AUX_STAT cannot be defined in XCOFF64");
-    AuxSym.reset(new XCOFFYAML::SectAuxEntForStat());
+    if (!IO.outputting())
+      AuxSym.reset(new XCOFFYAML::SectAuxEntForStat());
     auxSymMapping(IO, *cast<XCOFFYAML::SectAuxEntForStat>(AuxSym.get()));
     break;
   }
@@ -334,8 +342,7 @@ void MappingTraits<XCOFFYAML::Symbol>::mapping(IO &IO, XCOFFYAML::Symbol &S) {
   IO.mapOptional("Type", S.Type);
   IO.mapOptional("StorageClass", S.StorageClass);
   IO.mapOptional("NumberOfAuxEntries", S.NumberOfAuxEntries);
-  if (!IO.outputting())
-    IO.mapOptional("AuxEntries", S.AuxEntries);
+  IO.mapOptional("AuxEntries", S.AuxEntries);
 }
 
 void MappingTraits<XCOFFYAML::StringTable>::mapping(IO &IO, XCOFFYAML::StringTable &Str) {
diff --git a/llvm/test/tools/obj2yaml/XCOFF/aix.yaml b/llvm/test/tools/obj2yaml/XCOFF/aix.yaml
index cd1e88dec11d29b..fbd5fa0629d10bd 100644
--- a/llvm/test/tools/obj2yaml/XCOFF/aix.yaml
+++ b/llvm/test/tools/obj2yaml/XCOFF/aix.yaml
@@ -52,12 +52,30 @@
 # CHECK32-NEXT:     Type:            0x0
 # CHECK32-NEXT:     StorageClass:    C_EXT
 # CHECK32-NEXT:     NumberOfAuxEntries: 1
+# CHECK32-NEXT:     AuxEntries:
+# CHECK32-NEXT:       - Type:            AUX_CSECT
+# CHECK32-NEXT:         ParameterHashIndex: 0
+# CHECK32-NEXT:         TypeChkSectNum:  0
+# CHECK32-NEXT:         SymbolAlignmentAndType: 0
+# CHECK32-NEXT:         StorageMappingClass: XMC_PR
+# CHECK32-NEXT:         SectionOrLength: 0
+# CHECK32-NEXT:         StabInfoIndex:   0
+# CHECK32-NEXT:         StabSectNum:     0
 # CHECK32-NEXT:   - Name:            .data
 # CHECK32-NEXT:     Value:           0x70
 # CHECK32-NEXT:     Section:         .data
 # CHECK32-NEXT:     Type:            0x0
 # CHECK32-NEXT:     StorageClass:    C_HIDEXT
 # CHECK32-NEXT:     NumberOfAuxEntries: 1
+# CHECK32-NEXT:     AuxEntries:
+# CHECK32-NEXT:       - Type:            AUX_CSECT
+# CHECK32-NEXT:         ParameterHashIndex: 0
+# CHECK32-NEXT:         TypeChkSectNum:  0
+# CHECK32-NEXT:         SymbolAlignmentAndType: 0
+# CHECK32-NEXT:         StorageMappingClass: XMC_PR
+# CHECK32-NEXT:         SectionOrLength: 0
+# CHECK32-NEXT:         StabInfoIndex:   0
+# CHECK32-NEXT:         StabSectNum:     0
 
 # CHECK64:      --- !XCOFF
 # CHECK64-NEXT: FileHeader:
@@ -106,12 +124,28 @@
 # CHECK64-NEXT:     Type:            0x0
 # CHECK64-NEXT:     StorageClass:    C_EXT
 # CHECK64-NEXT:     NumberOfAuxEntries: 1
+# CHECK64-NEXT:     AuxEntries:
+# CHECK64-NEXT:       - Type:            AUX_CSECT
+# CHECK64-NEXT:         ParameterHashIndex: 0
+# CHECK64-NEXT:         TypeChkSectNum:  0
+# CHECK64-NEXT:         SymbolAlignmentAndType: 0
+# CHECK64-NEXT:         StorageMappingClass: XMC_PR
+# CHECK64-NEXT:         SectionOrLengthLo: 0
+# CHECK64-NEXT:         SectionOrLengthHi: 0
 # CHECK64-NEXT:   - Name:            .data
 # CHECK64-NEXT:     Value:           0x70
 # CHECK64-NEXT:     Section:         .data
 # CHECK64-NEXT:     Type:            0x0
 # CHECK64-NEXT:     StorageClass:    C_HIDEXT
 # CHECK64-NEXT:     NumberOfAuxEntries: 1
+# CHECK64-NEXT:     AuxEntries:
+# CHECK64-NEXT:       - Type:            AUX_CSECT
+# CHECK64-NEXT:         ParameterHashIndex: 0
+# CHECK64-NEXT:         TypeChkSectNum:  0
+# CHECK64-NEXT:         SymbolAlignmentAndType: 0
+# CHECK64-NEXT:         StorageMappingClass: XMC_PR
+# CHECK64-NEXT:         SectionOrLengthLo: 0
+# CHECK64-NEXT:         SectionOrLengthHi: 0
 
 --- !XCOFF
 FileHeader:
@@ -140,9 +174,13 @@ Symbols:
     Type:               0x0
     StorageClass:       C_EXT
     NumberOfAuxEntries: 1
+    AuxEntries:
+       - Type:          AUX_CSECT
   - Name:               .data
     Value:              0x70
     Section:            .data
     Type:               0x0
     StorageClass:       C_HIDEXT
     NumberOfAuxEntries: 1
+    AuxEntries:
+       - Type:          AUX_CSECT
diff --git a/llvm/test/tools/obj2yaml/XCOFF/aux-symbols.yaml b/llvm/test/tools/obj2yaml/XCOFF/aux-symbols.yaml
new file mode 100644
index 000000000000000..a4d1cb1d33aeda3
--- /dev/null
+++ b/llvm/test/tools/obj2yaml/XCOFF/aux-symbols.yaml
@@ -0,0 +1,264 @@
+## Check that obj2yaml can parse auxiliary symbols for XCOFF object file correctly.
+
+## 32-bit
+# RUN: yaml2obj %s --docnum=1 -o %t32
+# RUN: obj2yaml %t32 | FileCheck %s --check-prefix=CHECK32
+
+# CHECK32:      --- !XCOFF
+# CHECK32-NEXT: FileHeader:
+# CHECK32-NEXT:   MagicNumber:     0x1DF
+# CHECK32-NEXT:   NumberOfSections: 0
+# CHECK32-NEXT:   CreationTime:    0
+# CHECK32-NEXT:   OffsetToSymbolTable: 0x14
+# CHECK32-NEXT:   EntriesInSymbolTable: 13
+# CHECK32-NEXT:   AuxiliaryHeaderSize: 0
+# CHECK32-NEXT:   Flags:           0x0
+# CHECK32-NEXT: Symbols:
+# CHECK32-NEXT:   - Name:            ''
+# CHECK32-NEXT:     Value:           0x0
+# CHECK32-NEXT:     Section:         N_UNDEF
+# CHECK32-NEXT:     Type:            0x0
+# CHECK32-NEXT:     StorageClass:    C_FILE
+# CHECK32-NEXT:     NumberOfAuxEntries: 1
+# CHECK32-NEXT:     AuxEntries:
+# CHECK32-NEXT:       - Type:            AUX_FILE
+# CHECK32-NEXT:         FileNameOrString: FileName
+# CHECK32-NEXT:         FileStringType:  XFT_CD
+# CHECK32-NEXT:   - Name:            ''
+# CHECK32-NEXT:     Value:           0x0
+# CHECK32-NEXT:     Section:         N_UNDEF
+# CHECK32-NEXT:     Type:            0x0
+# CHECK32-NEXT:     StorageClass:    C_HIDEXT
+# CHECK32-NEXT:     NumberOfAuxEntries: 1
+# CHECK32-NEXT:     AuxEntries:
+# CHECK32-NEXT:       - Type:            AUX_CSECT
+# CHECK32-NEXT:         ParameterHashIndex: 1
+# CHECK32-NEXT:         TypeChkSectNum:  2
+# CHECK32-NEXT:         SymbolAlignmentAndType: 41
+# CHECK32-NEXT:         StorageMappingClass: XMC_PR
+# CHECK32-NEXT:         SectionOrLength: 3
+# CHECK32-NEXT:         StabInfoIndex:   4
+# CHECK32-NEXT:         StabSectNum:     5
+# CHECK32-NEXT:   - Name:            ''
+# CHECK32-NEXT:     Value:           0x0
+# CHECK32-NEXT:     Section:         N_UNDEF
+# CHECK32-NEXT:     Type:            0x0
+# CHECK32-NEXT:     StorageClass:    C_EXT
+# CHECK32-NEXT:     NumberOfAuxEntries: 2
+# CHECK32-NEXT:     AuxEntries:
+# CHECK32-NEXT:       - Type:            AUX_FCN
+# CHECK32-NEXT:         OffsetToExceptionTbl: 1
+# CHECK32-NEXT:         SizeOfFunction:  2
+# CHECK32-NEXT:         SymIdxOfNextBeyond: 3
+# CHECK32-NEXT:         PtrToLineNum:    4
+# CHECK32-NEXT:       - Type:            AUX_CSECT
+# CHECK32-NEXT:         ParameterHashIndex: 1
+# CHECK32-NEXT:         TypeChkSectNum:  2
+# CHECK32-NEXT:         SymbolAlignmentAndType: 17
+# CHECK32-NEXT:         StorageMappingClass: XMC_PR
+# CHECK32-NEXT:         SectionOrLength: 4
+# CHECK32-NEXT:         StabInfoIndex:   5
+# CHECK32-NEXT:         StabSectNum:     6
+# CHECK32-NEXT:   - Name:            ''
+# CHECK32-NEXT:     Value:           0x0
+# CHECK32-NEXT:     Section:         N_UNDEF
+# CHECK32-NEXT:     Type:            0x0
+# CHECK32-NEXT:     StorageClass:    C_DWARF
+# CHECK32-NEXT:     NumberOfAuxEntries: 1
+# CHECK32-NEXT:     AuxEntries:
+# CHECK32-NEXT:       - Type:            AUX_SECT
+# CHECK32-NEXT:         LengthOfSectionPortion: 44
+# CHECK32-NEXT:         NumberOfRelocEnt: 1
+# CHECK32-NEXT:   - Name:            ''
+# CHECK32-NEXT:     Value:           0x0
+# CHECK32-NEXT:     Section:         N_UNDEF
+# CHECK32-NEXT:     Type:            0x0
+# CHECK32-NEXT:     StorageClass:    C_STAT
+# CHECK32-NEXT:     NumberOfAuxEntries: 1
+# CHECK32-NEXT:     AuxEntries:
+# CHECK32-NEXT:       - Type:            AUX_STAT
+# CHECK32-NEXT:         SectionLength:   1
+# CHECK32-NEXT:         NumberOfRelocEnt: 2
+# CHECK32-NEXT:         NumberOfLineNum: 3
+# CHECK32-NEXT:   - Name:            ''
+# CHECK32-NEXT:     Value:           0x0
+# CHECK32-NEXT:     Section:         N_UNDEF
+# CHECK32-NEXT:     Type:            0x0
+# CHECK32-NEXT:     StorageClass:    C_BLOCK
+# CHECK32-NEXT:     NumberOfAuxEntries: 1
+# CHECK32-NEXT:     AuxEntries:
+# CHECK32-NEXT:       - Type:            AUX_SYM
+# CHECK32-NEXT:         LineNumHi:       1
+# CHECK32-NEXT:         LineNumLo:       2
+
+--- !XCOFF
+FileHeader:
+  MagicNumber: 0x01DF
+Symbols:
+  - StorageClass:    C_FILE
+    AuxEntries:
+      - Type:             AUX_FILE
+        FileNameOrString: FileName
+        FileStringType:   XFT_CD
+  - StorageClass:    C_HIDEXT
+    AuxEntries:
+      - Type:               AUX_CSECT
+        ParameterHashIndex: 1
+        TypeChkSectNum:     2
+        SymbolAlignmentAndType: 41
+        SectionOrLength:    3
+        StabInfoIndex:      4
+        StabSectNum:        5
+  - StorageClass:    C_EXT
+    AuxEntries:
+      - Type:                 AUX_FCN
+        OffsetToExceptionTbl: 1
+        SizeOfFunction:       2
+        SymIdxOfNextBeyond:   3
+        PtrToLineNum:         4
+      - Type:               AUX_CSECT
+        ParameterHashIndex: 1
+        TypeChkSectNum:     2
+        SymbolAlignmentAndType: 17
+        SectionOrLength:    4
+        StabInfoIndex:      5
+        StabSectNum:        6
+  - StorageClass:    C_DWARF
+    AuxEntries:
+      - Type:                   AUX_SECT
+        LengthOfSectionPortion: 44
+        NumberOfRelocEnt:       1
+  - StorageClass:    C_STAT
+    AuxEntries:
+      - Type:             AUX_STAT
+        SectionLength:    1
+        NumberOfRelocEnt: 2
+        NumberOfLineNum:  3
+  - StorageClass:    C_BLOCK
+    AuxEntries:
+      - Type:            AUX_SYM
+        LineNumHi:       1
+        LineNumLo:       2
+
+## 64-bit
+# RUN: yaml2obj %s --docnum=2 -o %t64
+# RUN: obj2yaml %t64 | FileCheck %s --check-prefix=CHECK64
+
+# CHECK64:      --- !XCOFF
+# CHECK64-NEXT: FileHeader:
+# CHECK64-NEXT:   MagicNumber:     0x1F7
+# CHECK64-NEXT:   NumberOfSections: 0
+# CHECK64-NEXT:   CreationTime:    0
+# CHECK64-NEXT:   OffsetToSymbolTable: 0x18
+# CHECK64-NEXT:   EntriesInSymbolTable: 12
+# CHECK64-NEXT:   AuxiliaryHeaderSize: 0
+# CHECK64-NEXT:   Flags:           0x0
+# CHECK64-NEXT: Symbols:
+# CHECK64-NEXT:   - Name:            ''
+# CHECK64-NEXT:     Value:           0x0
+# CHECK64-NEXT:     Section:         N_UNDEF
+# CHECK64-NEXT:     Type:            0x0
+# CHECK64-NEXT:     StorageClass:    C_FILE
+# CHECK64-NEXT:     NumberOfAuxEntries: 1
+# CHECK64-NEXT:     AuxEntries:
+# CHECK64-NEXT:       - Type:            AUX_FILE
+# CHECK64-NEXT:         FileNameOrString: FileName
+# CHECK64-NEXT:         FileStringType:  XFT_CD
+# CHECK64-NEXT:   - Name:            ''
+# CHECK64-NEXT:     Value:           0x0
+# CHECK64-NEXT:     Section:         N_UNDEF
+# CHECK64-NEXT:     Type:            0x0
+# CHECK64-NEXT:     StorageClass:    C_HIDEXT
+# CHECK64-NEXT:     NumberOfAuxEntries: 1
+# CHECK64-NEXT:     AuxEntries:
+# CHECK64-NEXT:       - Type:            AUX_CSECT
+# CHECK64-NEXT:         ParameterHashIndex: 1
+# CHECK64-NEXT:         TypeChkSectNum:  2
+# CHECK64-NEXT:         SymbolAlignmentAndType: 41
+# CHECK64-NEXT:         StorageMappingClass: XMC_PR
+# CHECK64-NEXT:         SectionOrLengthLo: 3
+# CHECK64-NEXT:         SectionOrLengthHi: 4
+# CHECK64-NEXT:   - Name:            ''
+# CHECK64-NEXT:     Value:           0x0
+# CHECK64-NEXT:     Section:         N_UNDEF
+# CHECK64-NEXT:     Type:            0x0
+# CHECK64-NEXT:     StorageClass:    C_EXT
+# CHECK64-NEXT:     NumberOfAuxEntries: 3
+# CHECK64-NEXT:     AuxEntries:
+# CHECK64-NEXT:       - Type:            AUX_FCN
+# CHECK64-NEXT:         SizeOfFunction:  3
+# CHECK64-NEXT:         SymIdxOfNextBeyond: 2
+# CHECK64-NEXT:         PtrToLineNum:    1
+# CHECK64-NEXT:       - Type:            AUX_EXCEPT
+# CHECK64-NEXT:         OffsetToExceptionTbl: 1
+# CHECK64-NEXT:         SizeOfFunction:  2
+# CHECK64-NEXT:         SymIdxOfNextBeyond: 3
+# CHECK64-NEXT:       - Type:            AUX_CSECT
+# CHECK64-NEXT:         ParameterHashIndex: 1
+# CHECK64-NEXT:         TypeChkSectNum:  2
+# CHECK64-NEXT:         SymbolAlignmentAndType: 17
+# CHECK64-NEXT:         StorageMappingClass: XMC_PR
+# CHECK64-NEXT:         SectionOrLengthLo: 3
+# CHECK64-NEXT:         SectionOrLengthHi: 4
+# CHECK64-NEXT:   - Name:            ''
+# CHECK64-NEXT:     Value:           0x0
+# CHECK64-NEXT:     Section:         N_UNDEF
+# CHECK64-NEXT:     Type:            0x0
+# CHECK64-NEXT:     StorageClass:    C_DWARF
+# CHECK64-NEXT:     NumberOfAuxEntries: 1
+# CHECK64-NEXT:     AuxEntries:
+# CHECK64-NEXT:       - Type:            AUX_SECT
+# CHECK64-NEXT:         LengthOfSectionPortion: 44
+# CHECK64-NEXT:         NumberOfRelocEnt: 1
+# CHECK64-NEXT:   - Name:            ''
+# CHECK64-NEXT:     Value:           0x0
+# CHECK64-NEXT:     Section:         N_UNDEF
+# CHECK64-NEXT:     Type:            0x0
+# CHECK64-NEXT:     StorageClass:    C_BLOCK
+# CHECK64-NEXT:     NumberOfAuxEntries: 1
+# CHECK64-NEXT:     AuxEntries:
+# CHECK64-NEXT:       - Type:            AUX_SYM
+# CHECK64-NEXT:         LineNum:         1
+
+--- !XCOFF
+FileHeader:
+  MagicNumber:     0x1F7
+Symbols:
+  - StorageClass:    C_FILE
+    AuxEntries:
+      - Type:             AUX_FILE
+        FileNameOrString: FileName
+        FileStringType:   XFT_CD
+  - StorageClass:    C_HIDEXT
+    AuxEntries:
+      - Type:               AUX_CSECT
+        ParameterHashIndex: 1
+        TypeChkSectNum:     2
+        SymbolAlignmentAndType: 41
+        SectionOrLengthLo:  3
+        SectionOrLengthHi:  4
+  - StorageClass:    C_EXT
+    AuxEntries:
+      - Type:               AUX_FCN
+        SizeOfFunction:     3
+        SymIdxOfNextBeyond: 2
+        PtrToLineNum:       1
+      - Type:                 AUX_EXCEPT
+        OffsetToExceptionTbl: 1
+        SizeOfFunction:       2
+        SymIdxOfNextBeyond:   3
+      - Type:               AUX_CSECT
+        ParameterHashIndex: 1
+        TypeChkSectNum:     2
+        SymbolAlignmentAndType: 17
+        SectionOrLengthLo:  3
+        SectionOrLengthHi:  4
+  - StorageClass:    C_DWARF
+    AuxEntries:
+      - Type:                   AUX_SECT
+        LengthOfSectionPortion: 44
+        NumberOfRelocEnt:       1
+  - StorageClass:    C_BLOCK
+    AuxEntries:
+      - Type:            AUX_SYM
+        LineNum:         1
diff --git a/llvm/tools/obj2yaml/xcoff2yaml.cpp b/llvm/tools/obj2yaml/xcoff2yaml.cpp
index 882c410496012a1..1960513c611e8e7 100644
--- a/llvm/tools/obj2yaml/xcoff2yaml.cpp
+++ b/llvm/tools/obj2yaml/xcoff2yaml.cpp
@@ -29,6 +29,11 @@ class XCOFFDumper {
   XCOFFDumper(const object::XCOFFObjectFile &obj) : Obj(obj) {}
   Error dump();
   XCOFFYAML::Object &getYAMLObj() { return YAMLObj; }
+
+  template <typename T> const T *getAuxEntPtr(uintptr_t AuxAddress) {
+    Obj.checkSymbolEntryPointer(AuxAddress);
+    return reinterpret_cast<const T *>(AuxAddress);
+  }
 };
 } // namespace
 
@@ -106,6 +111,70 @@ Error XCOFFDumper::dumpSections(ArrayRef<Shdr> Sections) {
   return Error::success();
 }
 
+static void
+dumpStatAuxSym(std::vector<std::unique_ptr<XCOFFYAML::AuxSymbolEnt>> *AuxEntTbl,
+               const object::XCOFFSectAuxEntForStat *AuxEntPtr) {
+  XCOFFYAML::SectAuxEntForStat StatAuxSym;
+  StatAuxSym.SectionLength = AuxEntPtr->SectionLength;
+  StatAuxSym.NumberOfLineNum = AuxEntPtr->NumberOfLineNum;
+  StatAuxSym.NumberOfRelocEnt = AuxEntPtr->NumberOfRelocEnt;
+  AuxEntTbl->push_back(
+      std::make_unique<XCOFFYAML::SectAuxEntForStat>(StatAuxSym));
+}
+
+static void
+dumpFunAuxSym(std::vector<std::unique_ptr<XCOFFYAML::AuxSymbolEnt>> *AuxEntTbl,
+              const object::XCOFFFunctionAuxEnt32 *AuxEntPtr) {
+  XCOFFYAML::FunctionAuxEnt FunAuxSym;
+  FunAuxSym.OffsetToExceptionTbl = AuxEntPtr->OffsetToExceptionTbl;
+  FunAuxSym.PtrToLineNum = AuxEntPtr->PtrToLineNum;
+  FunAuxSym.SizeOfFunction = AuxEntPtr->SizeOfFunction;
+  FunAuxSym.SymIdxOfNextBeyond = AuxEntPtr->SymIdxOfNextBeyond;
+  AuxEntTbl->push_back(std::make_unique<XCOFFYAML::FunctionAuxEnt>(FunAuxSym));
+}
+
+static void
+dumpFunAuxSym(std::vector<std::unique_ptr<XCOFFYAML::AuxSymbolEnt>> *AuxEntTbl,
+              const object::XCOFFFunctionAuxEnt64 *AuxEntPtr) {
+  XCOFFYAML::FunctionAuxEnt FunAuxSym;
+  FunAuxSym.PtrToLineNum = AuxEntPtr->PtrToLineNum;
+  FunAuxSym.SizeOfFunction = AuxEntPtr->SizeOfFunction;
+  FunAuxSym.SymIdxOfNextBeyond = AuxEntPtr->SymIdxOfNextBeyond;
+  AuxEntTbl->push_back(std::make_unique<XCOFFYAML::FunctionAuxEnt>(FunAuxSym));
+}
+
+static void
+dumpExpAuxSym(std::vector<std::unique_ptr<XCOFFYAML::AuxSymbolEnt>> *AuxEntTbl,
+              const object::XCOFFExceptionAuxEnt *AuxEntPtr) {
+  XCOFFYAML::ExcpetionAuxEnt ExceptAuxSym;
+  ExceptAuxSym.OffsetToExceptionTbl = AuxEntPtr->OffsetToExceptionTbl;
+  ExceptAuxSym.SizeOfFunction = AuxEntPtr->SizeOfFunction;
+  ExceptAuxSym.SymIdxOfNextBeyond = AuxEntPtr->SymIdxOfNextBeyond;
+  AuxEntTbl->push_back(
+      std::make_unique<XCOFFYAML::ExcpetionAuxEnt>(ExceptAuxSym));
+}
+
+static void dumpCscetAuxSym(
+    std::vector<std::unique_ptr<XCOFFYAML::AuxSymbolEnt>> *AuxEntTbl,
+    const object::XCOFFCsectAuxRef &AuxEntPtr, bool Is64bit) {
+  XCOFFYAML::CsectAuxEnt CsectAuxSym;
+  CsectAuxSym.ParameterHashIndex = AuxEntPtr.getParameterHashIndex();
+  CsectAuxSym.TypeChkSectNum = AuxEntPtr.getTypeChkSectNum();
+  CsectAuxSym.SymbolAlignmentAndType = AuxEntPtr.getSymbolAlignmentAndType();
+  CsectAuxSym.StorageMappingClass = AuxEntPtr.getStorageMappingClass();
+  if (Is64bit) {
+    CsectAuxSym.SectionOrLengthLo =
+        static_cast<uint32_t>(AuxEntPtr.getSectionOrLength64());
+    C...
[truncated]

jh7370

I've got nothing significant to add: one of the more XCOFF-knowledgeable developers needs to review this too.

llvm/tools/obj2yaml/xcoff2yaml.cpp

diggerlin · 2023-11-07T19:50:55Z

llvm/tools/obj2yaml/xcoff2yaml.cpp

+  StatAuxSym.SectionLength = AuxEntPtr.SectionLength;
+  StatAuxSym.NumberOfLineNum = AuxEntPtr.NumberOfLineNum;
+  StatAuxSym.NumberOfRelocEnt = AuxEntPtr.NumberOfRelocEnt;
+  AuxEntTbl.push_back(


how about to change all those dump*AuxSym into constructor function?

for example:

struct SectAuxEntForStat: AuxSymbolEnt { ..... SectAuxEntForStat(object::XCOFFSectAuxEntForStat &AuxEntPtr)) : AuxSymbolEnt(AuxSymbolType::AUX_STAT) { SectionLength = AuxEntPtr.SectionLength; NumberOfLineNum = AuxEntPtr.NumberOfLineNum; NumberOfRelocEnt = AuxEntPtr.NumberOfRelocEnt; } }

and than you do not need the function dumpStatAuxSym , you can use AuxEntTbl.push_back() where you call the dumpStatAuxSym

The suggestion will give you some extra work, but I think it more reasonable.

Thank you for your suggestions! (Sorry, I didn't notice this until after the update).
Although I have no objection about your proposal, actually I'm not sure which implementation is more preferable since current style aligns with other formats like COFF or ELF. And does it make more sense to put the lengthy implementation type code here instead of in the header file?
Hi James @jh7370 , what do you think?

I think the implementation of the constructors can be in the cpp, should you choose to go down that route. I think having constructors will likely make things cleaner, but you lose the POD nature of the objects. I've got no strong feeling on whether that is an issue or not, and leave it up to you two and other XCOFF stakeholders to agree on that (I don't think it needs to match COFF/ELF etc specifically).

The purpose of the function is to create a new object SectAuxEntForStat and initiated with XCOFFSectAuxEntForStat . Using constructor is object oriented.
it is a suggestion , feel free not to modify if you want to keep current implement.

Thank you @diggerlin @jh7370 , well then I would prefer to keep current style.

diggerlin · 2023-11-07T20:38:39Z

llvm/lib/ObjectYAML/XCOFFYAML.cpp

  IO.mapRequired("Type", AuxType);
  switch (AuxType) {
  case XCOFFYAML::AUX_EXCEPT:
    if (!Is64)
      IO.setError("an auxiliary symbol of type AUX_EXCEPT cannot be defined in "
                  "XCOFF32");
-    AuxSym.reset(new XCOFFYAML::ExcpetionAuxEnt());
+    if (!IO.outputting())
+      AuxSym.reset(new XCOFFYAML::ExcpetionAuxEnt());


suggest adding a lambda helper function

auto ResetAuxSym = [&](auto* AuxEnt) { if (!IO.outputting()) AuxSym.reset(AuxEnt); };

and then change the code

if (!IO.outputting()) AuxSym.reset(new XCOFFYAML::ExcpetionAuxEnt());

to

ResetAuxSym (new XCOFFYAML::ExcpetionAuxEnt());

jh7370

@EsmeYi, please don't resolve comments that I've made, as I need to look back through them to check that they've been addressed as I expected. See https://discourse.llvm.org/t/rfc-github-pr-resolve-conversation-button/73178.

Nothing new to add at this point. I'll take a further look through once Digger is basically happy.

jh7370

LGTM, once @diggerlin is happy.

diggerlin · 2023-11-20T14:56:31Z

llvm/lib/ObjectYAML/XCOFFYAML.cpp

  IO.mapRequired("Type", AuxType);
  switch (AuxType) {
  case XCOFFYAML::AUX_EXCEPT:
    if (!Is64)
      IO.setError("an auxiliary symbol of type AUX_EXCEPT cannot be defined in "
                  "XCOFF32");
-    AuxSym.reset(new XCOFFYAML::ExcpetionAuxEnt());
+    ResetAuxSym(new XCOFFYAML::ExcpetionAuxEnt());


in the code, for 32bit, after you set IO.setError m it will still tun into this line, do you forget else ?

The yaml2obj fails to parse immediately once the IO.setError() triggered and the error message is printed directly, therefore we don't need an else here.

after the error print out, but the it do not return function immediately. it go to the ResetAuxSym(new XCOFFYAML::ExcpetionAuxEnt()) , since the ExcpetionAuxEnt is not support in the 32bit, do not need to run the `ResetAuxSym(new XCOFFYAML::ExcpetionAuxEnt());' is it correct ?

OK, I see. Thanks!

diggerlin · 2023-11-20T15:39:01Z

llvm/tools/obj2yaml/xcoff2yaml.cpp

+
+    if (Obj.is64Bit()) {
+      XCOFF::SymbolAuxType Type = *Obj.getSymbolAuxType(AuxAddress);
+      if (Type == XCOFF::SymbolAuxType::AUX_CSECT)


it looks you change the order of auxiliary entry of the CSECT, you always print the CSECT entry in the end of the all auxiliary entries for 64 bits, I think we need to keep the same order of the binary object file.

diggerlin · 2023-11-20T15:53:52Z

llvm/tools/obj2yaml/xcoff2yaml.cpp

+
+void XCOFFDumper::dumpStatAuxSym(XCOFFYAML::Symbol &Sym,
+                                 const XCOFFSymbolRef &SymbolEntRef) {
+  assert(Sym.NumberOfAuxEntries == 1 &&


should we use assert here ? assert is used to check a program error, but if Sym.NumberOfAuxEntries != 1, it is an input object file error, we should report an error. not assert .

diggerlin · 2023-11-20T15:55:35Z

llvm/tools/obj2yaml/xcoff2yaml.cpp

+      else if (Type == XCOFF::SymbolAuxType::AUX_EXCEPT)
+        dumpExpAuxSym(Sym, AuxAddress);
+      else
+        llvm_unreachable("invalid aux symbol entry");


we should report an input object file error, should not xcoff2yaml crash.

diggerlin · 2023-11-20T15:56:11Z

llvm/tools/obj2yaml/xcoff2yaml.cpp

+
+void XCOFFDumper::dumpBlockAuxSym(XCOFFYAML::Symbol &Sym,
+                                  const XCOFFSymbolRef &SymbolEntRef) {
+  assert(Sym.NumberOfAuxEntries == 1 &&


ditto for using assert here

diggerlin · 2023-11-20T16:02:30Z

llvm/tools/obj2yaml/xcoff2yaml.cpp

+
+void XCOFFDumper::dumpDwarfAuxSym(XCOFFYAML::Symbol &Sym,
+                                  const XCOFFSymbolRef &SymbolEntRef) {
+  assert(Sym.NumberOfAuxEntries == 1 &&


ditto for assert.

diggerlin · 2023-11-20T16:06:48Z

llvm/test/tools/obj2yaml/XCOFF/aix.yaml

@@ -52,12 +52,30 @@
 # CHECK32-NEXT:     Type:            0x0
 # CHECK32-NEXT:     StorageClass:    C_EXT
 # CHECK32-NEXT:     NumberOfAuxEntries: 1
+# CHECK32-NEXT:     AuxEntries:


just curiosity, you have AUX_CSECT test in aux0symbols,yaml, why you add test case here ?

NumberOfAuxEntries was set to 1 but non auxiliary symbol is defined in this case, which was allowed before but will be illegal after this PR. And aix.yaml is a basic test for general behavior of xcoff2yaml and I think it would be better to add such test instead of remove the set of NumberOfAuxEntries: 1 here.

thanks for explain.

…use reportError instead of asertion when the input object is illegal.

diggerlin · 2023-11-21T21:30:12Z

llvm/tools/obj2yaml/xcoff2yaml.cpp

+Error XCOFFDumper::dumpStatAuxSym(XCOFFYAML::Symbol &Sym,
+                                  const XCOFFSymbolRef &SymbolEntRef) {
+  if (Sym.NumberOfAuxEntries != 1)
+    return createError("expected a single aux symbol for C_STAT, while got: " +


Suggest that we provide more detail info for invalid Symbol (for example symbol name, it maybe better to print out symbol index in the error info), let user can locate the specific invalid symbol.

and add some test case for Error scenario.

diggerlin · 2023-11-22T16:02:05Z

llvm/tools/obj2yaml/xcoff2yaml.cpp

+      else if (Type == XCOFF::SymbolAuxType::AUX_EXCEPT)
+        dumpExpAuxSym(Sym, AuxAddress);
+      else
+        return createError("failed to parse symbol \"" + Sym.SymbolName +


Sym.SymbolName does not include the storage mapping class; several symbols may have the same name. I prefer to use symbol index here, but if we use symbol index, a new data member 'SymbolIndex' needs to be added to 'XCOFFYAML::Symbol.' I am not sure whether it is worth it for us to do so or not.

If I'm not mistaken, the calling code can know the index by changing the loop it is using. The index could then be passed into this function without changing Symbol.

The index can be easily caught by uint32_t SymbolIndex = Obj.getSymbolIndex(SymbolEntRef.getEntryAddress());
I just pushed the updated commit. Thanks! @diggerlin and @jh7370

diggerlin · 2023-12-04T15:14:05Z

llvm/tools/obj2yaml/xcoff2yaml.cpp

+    XCOFFYAML::FileAuxEnt FileAuxSym;
+    FileAuxSym.FileNameOrString = FileNameOrError.get();
+    FileAuxSym.FileStringType = FileAuxEntPtr->Type;
+    Sym.AuxEntries.push_back(


for all the 64-bits auxiliary symbol entries, there is a field named x_auxtype , I think we need to print it out too, although the field can derived from the content of the auxiliary by user. obj2yaml is dump tool , we need to display all the fields in the auxiliary symbol entries

If I understand you correctly, I think currently we have print the aux type in the yaml file regardless of whether it is 32-bit or 64-bit. There is a constructor in each aux symbol struct, like FileAuxEnt():AuxSymbolEnt(AuxSymbolType::AUX_FILE) {}, and we can verify this in the test file llvm/test/tools/obj2yaml/XCOFF/aux-symbols.yaml

( Although auxiliary symbol entries in XCOFF32 do not really have the x_auxtype field, it's no harm to print the info in yaml and it's helpful when transforming yaml to object.

If I understand you correctly, I think currently we have print the aux type in the yaml file regardless of whether it is 32-bit or 64-bit. There is a constructor in each aux symbol struct, like FileAuxEnt():AuxSymbolEnt(AuxSymbolType::AUX_FILE) {}, and we can verify this in the test file llvm/test/tools/obj2yaml/XCOFF/aux-symbols.yaml

( Although auxiliary symbol entries in XCOFF32 do not really have the x_auxtype field, it's no harm to print the info in yaml and it's helpful when transforming yaml to object.

My understand we should print out the fields as it is in the object file. what is James' opinion ? @jh7370

Sorry, I'm not really sure I follow the conversation, as it is too much in the file format details. Some general comments:

We need enough info in the YAML produced by obj2yaml for yaml2obj to be able to create an identical object as was passed to obj2yaml.

This means we can potentially omit fields from the YAML dump if they have a value matching the default value yaml2obj would use if the field is omitted.

The less data we have in the YAML, the better, as long as the first point is conformed to. This is because it'll be easier to read, and if somebody is using obj2yaml to generate the YAML that will be used as the input in a test case, it is minimal and has no redundant information.

Thanks!
The aux type is not in the XCOFF32 object, but it is needed in YAML to help yaml2obj create the identical object.

diggerlin

LGTM

bzEq · 2023-12-07T05:24:21Z

llvm/lib/ObjectYAML/XCOFFYAML.cpp

+
+  auto ResetAuxSym = [&](auto *AuxEnt) {
+    if (!IO.outputting())
+      AuxSym.reset(AuxEnt);


What if IO.outputting() is true, will AuxEnt be deleted?

Might use a templated helper function to replace this lambda.

template<typename AuxEntT> static void ResetAuxSym(IO &IO, std::unique_ptr<XCOFFYAML::AuxSymbolEnt> &AuxSym) { if (!IO.outputting()) AuxSym.reset(new AuxEntT); }

Thanks, this seems cause a sanitizer error in https://lab.llvm.org/buildbot/#/builders/5/builds/39023
I gonna fix it.

…#70642)" This reverts commit 0e3faa2.

…#70642)" This reverts commit 0e3faa2. Due to a sanitizer error in https://lab.llvm.org/buildbot/#/builders/5/builds/39023. Will be re-landed after repairs and thorough testing.

…#70642)" This PR adds the support for parsing auxiliary symbols of XCOFF object file for obj2yaml. The sanitizer error is clean now.

Add support for parsing auxiliary symbols of XCOFF object file for ob…

7ba6a65

…j2yaml.

EsmeYi added backend:PowerPC platform:aix labels Oct 30, 2023

EsmeYi requested review from chenzheng1030 and jh7370 October 30, 2023 10:13

llvmbot added objectyaml llvm:binary-utilities labels Oct 30, 2023

chenzheng1030 requested a review from diggerlin October 31, 2023 06:48

jh7370 reviewed Oct 31, 2023

View reviewed changes

llvm/tools/obj2yaml/xcoff2yaml.cpp Outdated Show resolved Hide resolved

llvm/tools/obj2yaml/xcoff2yaml.cpp Outdated Show resolved Hide resolved

llvm/tools/obj2yaml/xcoff2yaml.cpp Outdated Show resolved Hide resolved

llvm/tools/obj2yaml/xcoff2yaml.cpp Outdated Show resolved Hide resolved

Address James's comments.

724fb01

jh7370 reviewed Nov 7, 2023

View reviewed changes

diggerlin reviewed Nov 7, 2023

View reviewed changes

EsmeYi added 2 commits November 8, 2023 01:11

Address James's comments; move case bodies to separate functions.

1f12c62

Address part of Digger's comments.

70f8325

jh7370 reviewed Nov 8, 2023

View reviewed changes

jh7370 approved these changes Nov 16, 2023

View reviewed changes

diggerlin reviewed Nov 20, 2023

View reviewed changes

Address Digger's comments: 1) keep the order of AUX_CSECT symbol; 2) …

d9f2abd

…use reportError instead of asertion when the input object is illegal.

diggerlin reviewed Nov 21, 2023

View reviewed changes

Enrich error messages and test them.

be93262

EsmeYi requested a review from diggerlin November 22, 2023 05:54

diggerlin reviewed Nov 22, 2023

View reviewed changes

printf symbol index info for error message.

96aff59

EsmeYi requested a review from diggerlin November 30, 2023 05:39

diggerlin reviewed Dec 4, 2023

View reviewed changes

Merge branch 'llvm:main' into xcoff2yaml_auxsym

bfc5696

EsmeYi requested a review from stephenpeckham December 6, 2023 08:43

diggerlin approved these changes Dec 6, 2023

View reviewed changes

EsmeYi merged commit 0e3faa2 into llvm:main Dec 7, 2023
3 checks passed

bzEq reviewed Dec 7, 2023

View reviewed changes

EsmeYi added a commit that referenced this pull request Dec 7, 2023

Revert "[XCOFF][obj2yaml] support parsing auxiliary symbols for XCOFF (…

662487d

…#70642)" This reverts commit 0e3faa2.

EsmeYi added a commit that referenced this pull request Dec 7, 2023

Reland "[XCOFF][obj2yaml] support parsing auxiliary symbols for XCOFF (…

e462937

…#70642)" This PR adds the support for parsing auxiliary symbols of XCOFF object file for obj2yaml. The sanitizer error is clean now.

[XCOFF][obj2yaml] support parsing auxiliary symbols for XCOFF #70642

[XCOFF][obj2yaml] support parsing auxiliary symbols for XCOFF #70642

Conversation

EsmeYi commented Oct 30, 2023

llvmbot commented Oct 30, 2023 • edited

jh7370 left a comment

Choose a reason for hiding this comment

diggerlin Nov 7, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diggerlin Nov 8, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diggerlin Nov 7, 2023 • edited

Choose a reason for hiding this comment

jh7370 left a comment

Choose a reason for hiding this comment

jh7370 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diggerlin Nov 21, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diggerlin Nov 20, 2023 • edited

Choose a reason for hiding this comment

diggerlin Nov 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diggerlin Nov 21, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diggerlin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

llvmbot commented Oct 30, 2023 •

edited

diggerlin Nov 7, 2023 •

edited

diggerlin Nov 8, 2023 •

edited

diggerlin Nov 7, 2023 •

edited

diggerlin Nov 21, 2023 •

edited

diggerlin Nov 20, 2023 •

edited

diggerlin Nov 20, 2023 •

edited

diggerlin Nov 21, 2023 •

edited