[llvm][yaml2obj] Modify section header overriding timing (#130942)

yaml2obj should determine the program header offset (and other
properties) based on the intended values rather than the final
`sh_offset` of the section header.

`setProgramHeaderLayout` uses section offsets for determining
`p_offset`. Move section header overriding after
`setProgramHeaderLayout` to prevent `ShOffset` from affecting program
header `p_offset`.

This change adjusts the timing of when the section header is overridden
to ensure that the program headers are set correctly.

More details
[here](#126537 (comment)).

---------

Signed-off-by: Ruoyu Qiu <[email protected]>
Signed-off-by: Ruoyu Qiu <[email protected]>
Co-authored-by: Ruoyu Qiu <[email protected]>

Author: cabbaken

llvm/lib/ObjectYAML/ELFEmitter.cpp

@@ -206,6 +206,9 @@ template <class ELFT> class ELFState {
   NameToIdxMap DynSymN2I;
   ELFYAML::Object &Doc;
 
+  std::vector<std::pair<Elf_Shdr *, ELFYAML::Section>>
+      SectionHeadersOverrideHelper;
+
   StringSet<> ExcludedSectionHeaders;
 
   uint64_t LocationCounter = 0;
@@ -226,6 +229,7 @@ template <class ELFT> class ELFState {
                           StringRef SecName, ELFYAML::Section *YAMLSec);
   void initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
                           ContiguousBlobAccumulator &CBA);
+  void overrideSectionHeaders(std::vector<Elf_Shdr> &SHeaders);
   void initSymtabSectionHeader(Elf_Shdr &SHeader, SymtabType STType,
                                ContiguousBlobAccumulator &CBA,
                                ELFYAML::Section *YAMLSec);
@@ -849,7 +853,7 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
       }
 
       LocationCounter += SHeader.sh_size;
-      overrideFields<ELFT>(Sec, SHeader);
+      SectionHeadersOverrideHelper.push_back({&SHeader, *Sec});
       continue;
     }
 
@@ -903,12 +907,17 @@ void ELFState<ELFT>::initSectionHeaders(std::vector<Elf_Shdr> &SHeaders,
     }
 
     LocationCounter += SHeader.sh_size;
-
-    // Override section fields if requested.
-    overrideFields<ELFT>(Sec, SHeader);
+    SectionHeadersOverrideHelper.push_back({&SHeader, *Sec});
   }
 }
 
+template <class ELFT>
+void ELFState<ELFT>::overrideSectionHeaders(std::vector<Elf_Shdr> &SHeaders) {
+  for (std::pair<Elf_Shdr *, ELFYAML::Section> &HeaderAndSec :
+       SectionHeadersOverrideHelper)
+    overrideFields<ELFT>(&HeaderAndSec.second, *HeaderAndSec.first);
+}
+
 template <class ELFT>
 void ELFState<ELFT>::assignSectionAddress(Elf_Shdr &SHeader,
                                           ELFYAML::Section *YAMLSec) {
@@ -2105,6 +2114,11 @@ bool ELFState<ELFT>::writeELF(raw_ostream &OS, ELFYAML::Object &Doc,
   // Now we can decide segment offsets.
   State.setProgramHeaderLayout(PHeaders, SHeaders);
 
+  // Override section fields, if requested. This needs to happen after program
+  // header layout happens, because otherwise the layout will use the new
+  // values.
+  State.overrideSectionHeaders(SHeaders);
+
   bool ReachedLimit = CBA.getOffset() > MaxSize;
   if (Error E = CBA.takeLimitError()) {
     // We report a custom error message instead below.

llvm/test/tools/llvm-readobj/ELF/malformed-pt-dynamic.test

@@ -91,7 +91,7 @@
 
 ## Case D: the same as "Case C", but for a 32-bit object.
 
-# RUN: yaml2obj %s -DBITS=32 -DOFFSET=0xffffffff -o %t5
+# RUN: yaml2obj %s -DBITS=32 -DFILESIZE=8 -DOFFSET=0xffffffff -o %t5
 # RUN: llvm-readobj %t5 --dynamic-table 2>&1 | FileCheck -DFILE=%t5 %s \
 # RUN:   --check-prefix=WARN5 --implicit-check-not=warning:
 # RUN: llvm-readelf %t5 --dynamic-table 2>&1 | FileCheck -DFILE=%t5 %s \
@@ -101,7 +101,7 @@
 # WARN5: warning: '[[FILE]]': unable to read the dynamic table from SHT_DYNAMIC section with index 1: offset (0xffffffff) + size (0x8) is greater than the file size (0x10c8)
 # WARN5: warning: '[[FILE]]': no valid dynamic table was found
 
-# RUN: yaml2obj %s -DNOHEADERS=true -DBITS=32 -DOFFSET=0xffffffff -o %t5.noheaders
+# RUN: yaml2obj %s -DNOHEADERS=true -DBITS=32 -DFILESIZE=8 -DOFFSET=0xffffffff -o %t5.noheaders
 # RUN: llvm-readobj %t5.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t5.noheaders %s --check-prefix=WARN5-NOHEADERS
 # RUN: llvm-readelf %t5.noheaders --dynamic-table 2>&1 | \
@@ -133,14 +133,13 @@ Sections:
     Type:     SHT_DYNAMIC
     Address:  0x1000
     Offset:   0x1000
-    ShOffset: [[OFFSET=<none>]]
+    ShOffset: [[OFFSET=0x1000]]
     Entries:
       - Tag:   DT_NULL
         Value: 0
   - Type: SectionHeaderTable
     NoHeaders: [[NOHEADERS=false]]
 ProgramHeaders:
   - Type:     PT_DYNAMIC
-    FileSize: [[FILESIZE=<none>]]
-    FirstSec: .dynamic
-    LastSec:  .dynamic
+    FileSize: [[FILESIZE=0x10]]
+    Offset:   [[OFFSET=0x1000]]

llvm/test/tools/obj2yaml/ELF/program-headers.yaml

@@ -436,28 +436,38 @@ Sections:
 # NOBITS-NEXT:     Flags:    [ PF_W, PF_R ]
 # NOBITS-NEXT:     FirstSec: .bss
 # NOBITS-NEXT:     LastSec:  .bss
-# NOBITS-NEXT:     Offset:   0x159
+# NOBITS-NEXT:     VAddr:    0x1001
+# NOBITS-NEXT:     Offset:   0x191
 # NOBITS-NEXT:   - Type:     PT_LOAD
 # NOBITS-NEXT:     Flags:    [ PF_W, PF_R ]
 # NOBITS-NEXT:     FirstSec: .data.1
 # NOBITS-NEXT:     LastSec:  .bss
-# NOBITS-NEXT:     Offset:   0x158
+# NOBITS-NEXT:     VAddr:    0x1000
+# NOBITS-NEXT:     Offset:   0x190
 # NOBITS-NEXT:   - Type:     PT_LOAD
 # NOBITS-NEXT:     Flags:    [ PF_W, PF_R ]
 # NOBITS-NEXT:     FirstSec: .data.1
 # NOBITS-NEXT:     LastSec:  .data.2
-# NOBITS-NEXT:     Offset:   0x158
+# NOBITS-NEXT:     VAddr:    0x1000
+# NOBITS-NEXT:     Offset:   0x190
 # NOBITS-NEXT:   - Type:     PT_LOAD
 # NOBITS-NEXT:     Flags:    [ PF_W, PF_R ]
 # NOBITS-NEXT:     FirstSec: .bss
 # NOBITS-NEXT:     LastSec:  .data.2
-# NOBITS-NEXT:     Offset:   0x159
+# NOBITS-NEXT:     VAddr:    0x1001
+# NOBITS-NEXT:     Offset:   0x191
 # NOBITS-NEXT:   - Type:     PT_LOAD
 # NOBITS-NEXT:     Flags:    [ PF_W, PF_R ]
 # NOBITS-NEXT:     FirstSec: .foo.bss
 # NOBITS-NEXT:     LastSec:  .bar.bss
 # NOBITS-NEXT:     VAddr:    0x200000000
-# NOBITS-NEXT:     Offset:   0x15A
+# NOBITS-NEXT:     Offset:   0x193
+# NOBITS-NEXT:   - Type:     PT_LOAD
+# NOBITS-NEXT:     Flags:    [ PF_W, PF_R ]
+# NOBITS-NEXT:     FirstSec: .data.3
+# NOBITS-NEXT:     LastSec:  .bss.large
+# NOBITS-NEXT:     VAddr:    0x200000030
+# NOBITS-NEXT:     Offset:   0x193
 # NOBITS-NEXT: Sections:
 
 --- !ELF
@@ -471,27 +481,37 @@ ProgramHeaders:
     Flags:    [ PF_W, PF_R ]
     FirstSec: .bss
     LastSec:  .bss
+    VAddr:    0x1001
 ## Case 2: the SHT_NOBITS section is the last section in the segment.
   - Type:     PT_LOAD
     Flags:    [ PF_W, PF_R ]
     FirstSec: .data.1
     LastSec:  .bss
+    VAddr:    0x1000
 ## Case 3: the SHT_NOBITS section is in the middle of the segment.
   - Type:     PT_LOAD
     Flags:    [ PF_W, PF_R ]
     FirstSec: .data.1
     LastSec:  .data.2
+    VAddr:    0x1000
 ## Case 4: the SHT_NOBITS section is the first section in the segment.
   - Type:     PT_LOAD
     Flags:    [ PF_W, PF_R ]
     FirstSec: .bss
     LastSec:  .data.2
+    VAddr:    0x1001
 ## Case 5: another two SHT_NOBITS sections in a different segment.
   - Type:     PT_LOAD
     Flags:    [ PF_W, PF_R ]
     FirstSec: .foo.bss
     LastSec:  .bar.bss
     VAddr:    0x200000000
+## Case 6: a SHT_NOBITS section following a normal section, where the SHT_NOBITS size points past the end of the program header.
+  - Type:     PT_LOAD
+    Flags:    [ PF_W, PF_R ]
+    FirstSec: .data.3
+    LastSec:  .bss.large
+    VAddr:    0x200000030
 Sections:
   - Name:    .data.1
     Type:    SHT_PROGBITS
@@ -502,13 +522,13 @@ Sections:
   - Name:   .bss
     Type:   SHT_NOBITS
     Flags:  [ SHF_WRITE, SHF_ALLOC ]
-## Use a size that is larger than the file size.
-    ShSize: 0x00000000FFFFFFFF
-  - Name:  .data.2
-    Type:  SHT_PROGBITS
-    Flags: [ SHF_WRITE, SHF_ALLOC ]
+## Use an arbitrary non-zero size.
+    Size:   0x1
+  - Name:     .data.2
+    Type:     SHT_PROGBITS
+    Flags:    [ SHF_WRITE, SHF_ALLOC ]
 ## Use an arbitrary size.
-    Size:  0x1
+    Size:     0x1
   - Name:    .foo.bss
     Type:    SHT_NOBITS
     Flags:   [ SHF_WRITE, SHF_ALLOC ]
@@ -523,14 +543,26 @@ Sections:
 ## Use an arbitrary size that is different to the size of
 ## the previous section.
     Size:    0x20
+  - Name: .data.3
+    Type: SHT_PROGBITS
+    Flags: [ SHF_WRITE, SHF_ALLOC ]
+## Use an abitrary size. Also use an address that is larger than the previous
+## section, because the tools expect segment addresses to be in order.
+    Size: 0x1
+    Address: 0x200000030
+  - Name: .bss.large
+    Type: SHT_NOBITS
+    Flags: [ SHF_WRITE, SHF_ALLOC ]
+## Use a size that is larger than the file size.
+    Size: 0x00000000FFFFFFFF
+    Address: 0x200000031
 
 ## Check that we require sections in a program header
 ## declaration to be sorted by their offsets.
 # RUN: not yaml2obj --docnum=6 %s -o %t6 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=UNSORTED --implicit-check-not="error:"
 
 # UNSORTED:      error: program header with index 0: the section index of .bar is greater than the index of .foo
-# UNSORTED-NEXT: error: sections in the program header with index 3 are not sorted by their file offset
 
 --- !ELF
 FileHeader:
@@ -552,15 +584,6 @@ ProgramHeaders:
     FirstSec: .foo
     LastSec:  .bar
     VAddr:    0x1000
-## Case 2: the .bar section is placed before the .zed section in the file,
-##         but the sh_offset of .zed is less than the sh_offset of
-##         the .bar section because of the "ShOffset" property.
-##         Document we report an error for such a case.
-  - Type:     PT_LOAD
-    Flags:    [ PF_R ]
-    FirstSec: .bar
-    LastSec:  .zed
-    VAddr:    0x1001
 Sections:
   - Name:    .foo
     Type:    SHT_PROGBITS
@@ -571,11 +594,6 @@ Sections:
     Type:    SHT_PROGBITS
     Flags:   [ SHF_ALLOC ]
     Size:    0x1
-  - Name:     .zed
-    Type:     SHT_PROGBITS
-    Flags:    [ SHF_ALLOC ]
-    Size:     0x1
-    ShOffset: 0x0
 
 ## Check how we dump segments which contain empty sections.
 # RUN: yaml2obj --docnum=7 %s -o %t7

llvm/test/tools/yaml2obj/ELF/program-header-size-offset.yaml

@@ -44,25 +44,25 @@ Sections:
   - Name: .text
     Type: SHT_PROGBITS
     Size: 4
-    ShOffset: 0x1000
+    Offset: 0x1000
     AddressAlign: 0x1000
   - Name: .rodata
     Type: SHT_PROGBITS
     Size: 4
-    ShOffset: 0x2000
+    Offset: 0x2000
     AddressAlign: 0x1000
   - Name: .data
     Type: SHT_PROGBITS
-    ShOffset: 0x2004
+    Offset: 0x2004
     Size: 4
   - Name: .nobits1
     Type: SHT_NOBITS
-    ShOffset: 0x2008
+    Offset: 0x2008
     Size: 1
   - Name: .nobits2
     Type: SHT_NOBITS
     # Intentionally set to 0x2009 though the previous section is SHT_NOBITS.
-    ShOffset: 0x2009
+    Offset: 0x2009
     Size: 1
 ProgramHeaders:
   # Program header with no sections.
@@ -180,15 +180,15 @@ ProgramHeaders:
 
 # INVALID-OFFSET: yaml2obj: error: 'Offset' for segment with index 1 must be less than or equal to the minimum file offset of all included sections (0x78)
 
-## Document that the "Offset" value is checked after the section offset is overriden using "ShOffset".
+## Check "Offset" is used before the section offset is overriden using "ShOffset".
 # RUN: yaml2obj --docnum=4 %s -o %t5
 # RUN: llvm-readelf %t5 --sections --program-headers | FileCheck %s --check-prefix=SHOFFSET
 
 # SHOFFSET: [Nr] Name Type     Address          Off
 # SHOFFSET: [ 1] .foo PROGBITS 0000000000000000 ffffffff
 
 # SHOFFSET:      Type Offset
-# SHOFFSET-NEXT: LOAD 0xffffff00
+# SHOFFSET-NEXT: LOAD 0x000078
 
 --- !ELF
 FileHeader:
@@ -200,11 +200,9 @@ Sections:
     Type:     SHT_PROGBITS
     Flags:    [ SHF_ALLOC ]
     Size:     0x1
-## Note: the real .foo offset is much less than 0xFFFFFFFF or
-##       0xFFFFFF00, but no error is reported.
     ShOffset: 0xFFFFFFFF
 ProgramHeaders:
   - Type:     PT_LOAD
-    Offset:   0xFFFFFF00
+    Offset:   0x78
     FirstSec: .foo
     LastSec:  .foo