[yaml2obj] Add support fot structured section data.

[jacek]

Summary

As suggested in D149091, this makes working with data like CHPE much more readable.

[jacek]

A new version with yaml2obj.rst changes.

[jhenderson]

llvm/test/tools/yaml2obj/COFF/section-data.yaml

20 ↗

(On Diff #518462)

I'm not sure I like the term Data, given that SectionData already exists. I'm guessing renaming SectionData to RawData is impractical due to the number of YAML files that would invalidate, but regardless, perhaps StructuredData might be a better name?

25 ↗

(On Diff #518462)

I'm not convinced both SectionData and Data should be specifiable for the same section. I think it makes more sense to reject the YAML if both are specified.

[jacek]

Thanks for review. The new version uses StructuredData instead of Data and does not allow combining StructuredData and SectionData.

My original thinking was that while SectionData is there for backward compatibility, it's becoming redundant as one may use Data: -Binary: instead. A more explicit name sounds good to me too.

[jacek]

Updated documentation.

[jacek]

Updated with additional use in arm64ec-chpe.yaml.

[jhenderson]

llvm/lib/ObjectYAML/COFFYAML.cpp

604

Test case that shows this bit of code has changed (correctly)?

[jacek]

Updated with missing tests. I also noticed that using uint32_t instead of int32_t would be better, so that we can cover call values with 0x constants.

[jhenderson]

In D149439#4469654, @jacek wrote:

Updated with missing tests. I also noticed that using uint32_t instead of int32_t would be better, so that we can cover call values with 0x constants.

Is there perhaps an argument for supporting both? uint32_t would certainly be my first choice, so I'm happy with just that, but if there's a case where the data might typically represent a negative value, it might make sense to support both. If there isn't a use-case though, it can be deferred until there is.

Anyway, LGTM in its current state.

llvm/lib/ObjectYAML/COFFYAML.cpp

552

No more than food-for-thought, but should it be UInt32 (capitalized "i"), since it's an abbreviation of "unsigned int"? I don't have a strong opinion, so am happy with whatevere your preference is.

[jacek]

I wasn't sure about capitalization when I wrote it. I still don't have a strong preference, but decided to change it to UInt32 after reconsideration.

A signed variant seems unlikely to be important in COFF context. I've been thinking about something like Zeroes (filling with specified number of zeroes) as another possible extension.

[jhenderson]

LGTM.

I'm happy to support any other extensions to this that prove useful. COFF yaml2obj is in need of a lot of love compared to the flexibility of ELF yaml2obj, so any improvements would be gratefully received.

llvm/lib/ObjectYAML/COFFYAML.cpp

604

Tip, clicking the checkbox to mark a comment as "Done", prior to uploading the updated patch makes it easier to see if a comment has been addressed.

llvm/docs/yaml2obj.rst

sections:
     - Name: .text
       Characteristics: [ IMAGE_SCN_CNT_CODE
                        , IMAGE_SCN_ALIGN_16BYTES
                        , IMAGE_SCN_MEM_EXECUTE
                        , IMAGE_SCN_MEM_READ
                        ] # 0x60500020
       SectionData:
         "\x83\xEC\x0C\xC7\x44\x24\x08\x00\x00\x00\x00\xC7\x04\x24\x00\x00\x00\x00\xE8\x00\x00\x00\x00\xE8\x00\x00\x00\x00\x8B\x44\x24\x08\x83\xC4\x0C\xC3" # |....D$.......$...............D$.....|
+    - Name: .rdata
+      Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+      StructuredData:
+        - Binary: {type: str}
+        - UInt32: {type: int}
 
   symbols:
     - Name: .text
       Value: 0
       SectionNumber: 1
       SimpleType: IMAGE_SYM_TYPE_NULL # (0)
       ComplexType: IMAGE_SYM_DTYPE_NULL # (0)
       StorageClass: IMAGE_SYM_CLASS_STATIC # (3)

llvm/include/llvm/ObjectYAML/COFFYAML.h

struct Relocation {
   // Normally a Relocation can refer to the symbol via its name.
   // It can also use a direct symbol table index instead (with no name
   // specified), allowing disambiguating between multiple symbols with the
   // same name or crafting intentionally broken files for testing.
   StringRef SymbolName;
   std::optional<uint32_t> SymbolTableIndex;
 };
 
+struct SectionDataEntry {
+  std::optional<uint32_t> UInt32;
+  yaml::BinaryRef Binary;
+
+  size_t size() const;
+  void writeAsBinary(raw_ostream &OS) const;
+};
+
 struct Section {
   COFF::section Header;
   unsigned Alignment = 0;
   yaml::BinaryRef SectionData;
   std::vector<CodeViewYAML::YAMLDebugSubsection> DebugS;
   std::vector<CodeViewYAML::LeafRecord> DebugT;
   std::vector<CodeViewYAML::LeafRecord> DebugP;
   std::optional<CodeViewYAML::DebugHSection> DebugH;
+  std::vector<SectionDataEntry> StructuredData;
   std::vector<Relocation> Relocations;
   StringRef Name;
 
   Section();
 };
 
 struct Symbol {
   COFF::symbol Header;

 
 } // end namespace COFFYAML
 
 } // end namespace llvm
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(COFFYAML::Section)
 LLVM_YAML_IS_SEQUENCE_VECTOR(COFFYAML::Symbol)
 LLVM_YAML_IS_SEQUENCE_VECTOR(COFFYAML::Relocation)
+LLVM_YAML_IS_SEQUENCE_VECTOR(COFFYAML::SectionDataEntry)
 
 namespace llvm {
 namespace yaml {
 
 template <>
 struct ScalarEnumerationTraits<COFFYAML::WeakExternalCharacteristics> {
   static void enumeration(IO &IO, COFFYAML::WeakExternalCharacteristics &Value);
 };
template <> struct MappingTraits<COFF::AuxiliaryCLRToken> {
   static void mapping(IO &IO, COFF::AuxiliaryCLRToken &ACT);
 };
 
 template <>
 struct MappingTraits<COFFYAML::Symbol> {
   static void mapping(IO &IO, COFFYAML::Symbol &S);
 };
 
+template <> struct MappingTraits<COFFYAML::SectionDataEntry> {
+  static void mapping(IO &IO, COFFYAML::SectionDataEntry &Sec);
+};
+
 template <>
 struct MappingTraits<COFFYAML::Section> {
   static void mapping(IO &IO, COFFYAML::Section &Sec);
 };
 
 template <>
 struct MappingTraits<COFFYAML::Object> {
   static void mapping(IO &IO, COFFYAML::Object &Obj);
 };
 
 } // end namespace yaml
 } // end namespace llvm
 
 #endif // LLVM_OBJECTYAML_COFFYAML_H

llvm/lib/ObjectYAML/COFFEmitter.cpp

for (COFFYAML::Section &S : CP.Obj.Sections) {
     } else if (S.Name == ".debug$P") {
       if (S.SectionData.binary_size() == 0)
         S.SectionData = CodeViewYAML::toDebugT(S.DebugP, CP.Allocator, S.Name);
     } else if (S.Name == ".debug$H") {
       if (S.DebugH && S.SectionData.binary_size() == 0)
         S.SectionData = CodeViewYAML::toDebugH(*S.DebugH, CP.Allocator);
     }
 
-    if (S.SectionData.binary_size() > 0) {
+    size_t DataSize = S.SectionData.binary_size();
+    for (auto E : S.StructuredData)
+      DataSize += E.size();
+    if (DataSize > 0) {
       CurrentSectionDataOffset = alignTo(CurrentSectionDataOffset,
                                          CP.isPE() ? CP.getFileAlignment() : 4);
-      S.Header.SizeOfRawData = S.SectionData.binary_size();
+      S.Header.SizeOfRawData = DataSize;
       if (CP.isPE())
         S.Header.SizeOfRawData =
             alignTo(S.Header.SizeOfRawData, CP.getFileAlignment());
       S.Header.PointerToRawData = CurrentSectionDataOffset;
       CurrentSectionDataOffset += S.Header.SizeOfRawData;
       if (!S.Relocations.empty()) {
         S.Header.PointerToRelocations = CurrentSectionDataOffset;
         if (S.Header.Characteristics & COFF::IMAGE_SCN_LNK_NRELOC_OVFL) {
static bool writeCOFF(COFFParser &CP, raw_ostream &OS) {
   }
 
   // Output section data.
   for (const COFFYAML::Section &S : CP.Obj.Sections) {
     if (S.Header.SizeOfRawData == 0 || S.Header.PointerToRawData == 0)
       continue;
     assert(S.Header.PointerToRawData >= OS.tell());
     OS.write_zeros(S.Header.PointerToRawData - OS.tell());
+    for (auto E : S.StructuredData)
+      E.writeAsBinary(OS);
     S.SectionData.writeAsBinary(OS);
     assert(S.Header.SizeOfRawData >= S.SectionData.binary_size());
-    OS.write_zeros(S.Header.SizeOfRawData - S.SectionData.binary_size());
+    OS.write_zeros(S.Header.PointerToRawData + S.Header.SizeOfRawData -
+                   OS.tell());
     if (S.Header.Characteristics & COFF::IMAGE_SCN_LNK_NRELOC_OVFL)
       OS << binary_le<uint32_t>(/*VirtualAddress=*/ S.Relocations.size() + 1)
          << binary_le<uint32_t>(/*SymbolTableIndex=*/ 0)
          << binary_le<uint16_t>(/*Type=*/ 0);
     for (const COFFYAML::Relocation &R : S.Relocations) {
       uint32_t SymbolTableIndex;
       if (R.SymbolTableIndex) {
         if (!R.SymbolName.empty())
static bool writeCOFF(COFFParser &CP, raw_ostream &OS) {
   }
 
   // Output string table.
   if (CP.Obj.Header.PointerToSymbolTable)
     OS.write(&CP.StringTable[0], CP.StringTable.size());
   return true;
 }
 
+size_t COFFYAML::SectionDataEntry::size() const {
+  size_t Size = Binary.binary_size();
+  if (UInt32)
+    Size += sizeof(*UInt32);
+  return Size;
+}
+
+void COFFYAML::SectionDataEntry::writeAsBinary(raw_ostream &OS) const {
+  if (UInt32)
+    OS << binary_le(*UInt32);
+  Binary.writeAsBinary(OS);
+}
+
 namespace llvm {
 namespace yaml {
 
 bool yaml2coff(llvm::COFFYAML::Object &Doc, raw_ostream &Out,
                ErrorHandler ErrHandler) {
   COFFParser CP(Doc, ErrHandler);
   if (!CP.parse()) {
     ErrHandler("failed to parse YAML file");

llvm/lib/ObjectYAML/COFFYAML.cpp

 
 void MappingTraits<COFF::AuxiliaryCLRToken>::mapping(
     IO &IO, COFF::AuxiliaryCLRToken &ACT) {
   MappingNormalization<NAuxTokenType, uint8_t> NATT(IO, ACT.AuxType);
   IO.mapRequired("AuxType", NATT->AuxType);
   IO.mapRequired("SymbolTableIndex", ACT.SymbolTableIndex);
 }
 
+void MappingTraits<COFFYAML::SectionDataEntry>::mapping(
+    IO &IO, COFFYAML::SectionDataEntry &E) {
+  IO.mapOptional("UInt32", E.UInt32);
+  IO.mapOptional("Binary", E.Binary);
+}
+
 void MappingTraits<COFFYAML::Symbol>::mapping(IO &IO, COFFYAML::Symbol &S) {
   MappingNormalization<NStorageClass, uint8_t> NS(IO, S.Header.StorageClass);
 
   IO.mapRequired("Name", S.Name);
   IO.mapRequired("Value", S.Header.Value);
   IO.mapRequired("SectionNumber", S.Header.SectionNumber);
   IO.mapRequired("SimpleType", S.SimpleType);
   IO.mapRequired("ComplexType", S.ComplexType);
if (Sec.Name == ".debug$S")
     IO.mapOptional("Subsections", Sec.DebugS);
   else if (Sec.Name == ".debug$T")
     IO.mapOptional("Types", Sec.DebugT);
   else if (Sec.Name == ".debug$P")
     IO.mapOptional("PrecompTypes", Sec.DebugP);
   else if (Sec.Name == ".debug$H")
     IO.mapOptional("GlobalHashes", Sec.DebugH);
 
+  IO.mapOptional("StructuredData", Sec.StructuredData);
+
+  if (!Sec.StructuredData.empty() && Sec.SectionData.binary_size()) {
+    IO.setError("StructuredData and SectionData can't be used together");
+    return;
+  }
+
   // Uninitialized sections, such as .bss, typically have no data, but the size
   // is carried in SizeOfRawData, even though PointerToRawData is zero.
-  if (Sec.SectionData.binary_size() == 0 &&
+  if (Sec.SectionData.binary_size() == 0 && Sec.StructuredData.empty() &&
       NC->Characteristics & COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA)
     IO.mapOptional("SizeOfRawData", Sec.Header.SizeOfRawData);
 
   IO.mapOptional("Relocations", Sec.Relocations);
 }
 
 void MappingTraits<COFFYAML::Object>::mapping(IO &IO, COFFYAML::Object &Obj) {
   IO.mapTag("!COFF", true);

llvm/test/tools/llvm-readobj/arm64ec-chpe.yaml

- Name:            .rdata
     Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
     VirtualAddress:  0x4000
     VirtualSize:     328
     SectionData:     '40010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000050008001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000'
   - Name:            .data
     Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
     VirtualAddress:  0x5000
     VirtualSize:     144
-    SectionData:     '010000005050000003000000685000008050000000000000000000000000000000000000000000000000000000000000020000000200000000000000000000000000000000000000000000000000000001100000300000000020000040000000023000005000000000100000201000000010000020100000401000000020000000100000002000002010000030200000'
+    StructuredData:
+      - UInt32: 1       # Version
+      - UInt32: 0x5050  # CodeMap
+      - UInt32: 3       # CodeMapCount
+      - UInt32: 0x5068  # CodeRangesToEntryPoints
+      - UInt32: 0x5080  # RedirectionMetadata
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 2       # CodeRangesToEntryPointsCount
+      - UInt32: 2       # RedirectionMetadataCount
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0x1001  # CodeMap[0]
+      - UInt32: 0x30
+      - UInt32: 0x2000  # CodeMap[1]
+      - UInt32: 0x40
+      - UInt32: 0x3002  # CodeMap[2]
+      - UInt32: 0x50
+      - UInt32: 0x1000  # CodeRangesToEntryPoints[0]
+      - UInt32: 0x1020
+      - UInt32: 0x1000
+      - UInt32: 0x1020  # CodeRangesToEntryPoints[1]
+      - UInt32: 0x1040
+      - UInt32: 0x2000
+      - UInt32: 0x1000  # RedirectionMetadata[0]
+      - UInt32: 0x2000
+      - UInt32: 0x1020  # RedirectionMetadata[1]
+      - UInt32: 0x2030
 symbols:         []
 ...

llvm/test/tools/yaml2obj/COFF/invalid-raw-data.yaml

+# RUN: not yaml2obj %s -o %t 2>&1 | FileCheck %s
+# CHECK: YAML:18:5: error: unknown key 'SizeOfRawData'
+
+--- !COFF
+OptionalHeader:
+  ImageBase:        0x180000000
+  SectionAlignment: 4096
+  FileAlignment:    512
+  DLLCharacteristics: [ ]
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE, IMAGE_FILE_DLL ]
+sections:
+  - Name:            .rdata
+    Characteristics: [ IMAGE_SCN_CNT_UNINITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  0x1000
+    VirtualSize:     20
+    SizeOfRawData:   4
+    StructuredData:
+        - UInt32: 1
+symbols: []
+...

llvm/test/tools/yaml2obj/COFF/mixed-data.yaml

+# RUN: not yaml2obj %s -o %t 2>&1 | FileCheck %s
+# CHECK: error: StructuredData and SectionData can't be used together
+
+--- !COFF
+OptionalHeader:
+  ImageBase:        0x180000000
+  SectionAlignment: 4096
+  FileAlignment:    512
+  DLLCharacteristics: [ ]
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE, IMAGE_FILE_DLL ]
+sections:
+  - Name:            .rdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  0x1000
+    VirtualSize:     20
+    SectionData:     55
+    StructuredData:
+        - UInt32: 1
+symbols: []
+...

llvm/test/tools/yaml2obj/COFF/structured-data.yaml

+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --hex-dump=.rdata %t | FileCheck %s
+# CHECK:  0x180001000 01000000 02000000 11223344 ffffffff
+
+--- !COFF
+OptionalHeader:
+  ImageBase:        0x180000000
+  SectionAlignment: 4096
+  FileAlignment:    512
+  DLLCharacteristics: [ ]
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE, IMAGE_FILE_DLL ]
+sections:
+  - Name:            .rdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  0x1000
+    VirtualSize:     20
+    StructuredData:
+        - UInt32: 1
+        - UInt32: 2
+        - Binary: 11223344
+        - UInt32: 0xffffffff
+symbols: []
+...