[BOLT] Always place new PT_LOAD after existing ones (#182642)

Insert new PT_LOAD segments right after the last existing PT_LOAD in the
program header table, instead of before PT_DYNAMIC or at the end. This
maintains the ascending p_vaddr order required by the ELF specification.

Previously, new segments could end up breaking PT_LOAD p_vaddr order
when PT_LOAD segments followed PT_DYNAMIC or PT_GNU_STACK. This lead to
runtime loader incorrectly assessing dynamic object size and silently
corrupting memory.

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -4583,17 +4583,12 @@ void RewriteInstance::patchELFPHDRTable() {
     return Phdr;
   };
 
-  auto writeNewSegmentPhdrs = [&]() {
-    for (const SegmentInfo &SI : BC->NewSegments) {
-      ELF64LEPhdrTy Phdr = createPhdr(SI);
-      OS.write(reinterpret_cast<const char *>(&Phdr), sizeof(Phdr));
-    }
-  };
+  // Collect modified program headers, then insert new PT_LOAD segments
+  // right after existing PT_LOAD segments to maintain ascending p_vaddr
+  // order required by the ELF specification.
+  SmallVector<ELF64LEPhdrTy, 16> Phdrs;
 
-  bool ModdedGnuStack = false;
-  bool AddedSegment = false;
-
-  // Copy existing program headers with modifications.
+  bool SkippedGnuStack = false;
   for (const ELF64LE::Phdr &Phdr : cantFail(Obj.program_headers())) {
     ELF64LE::Phdr NewPhdr = Phdr;
     switch (Phdr.p_type) {
@@ -4628,35 +4623,35 @@ void RewriteInstance::patchELFPHDRTable() {
     }
     case ELF::PT_GNU_STACK:
       if (opts::UseGnuStack) {
-        // Overwrite the header with the new segment header.
         assert(BC->NewSegments.size() == 1 &&
                "Expected exactly one new segment");
-        NewPhdr = createPhdr(BC->NewSegments.front());
-        ModdedGnuStack = true;
-      }
-      break;
-    case ELF::PT_DYNAMIC:
-      if (!opts::UseGnuStack) {
-        // Insert new headers before DYNAMIC.
-        writeNewSegmentPhdrs();
-        AddedSegment = true;
+        SkippedGnuStack = true;
+        continue; // Remove; new PT_LOAD will be added after existing ones.
       }
       break;
     }
-    OS.write(reinterpret_cast<const char *>(&NewPhdr), sizeof(NewPhdr));
+    Phdrs.push_back(NewPhdr);
   }
 
-  if (!opts::UseGnuStack && !AddedSegment) {
-    // Append new headers to the end of the table.
-    writeNewSegmentPhdrs();
-  }
-
-  if (opts::UseGnuStack && !ModdedGnuStack) {
+  if (opts::UseGnuStack && !SkippedGnuStack) {
     BC->errs()
         << "BOLT-ERROR: could not find PT_GNU_STACK program header to modify\n";
     exit(1);
   }
 
+  // Insert new PT_LOAD segments right after the last existing PT_LOAD to
+  // maintain ascending p_vaddr order.
+  auto LastPTLoad = llvm::find_if(reverse(Phdrs), [](const ELF64LE::Phdr &P) {
+    return P.p_type == ELF::PT_LOAD;
+  });
+  assert(LastPTLoad != Phdrs.rend() && "No existing PT_LOAD found");
+  auto InsertPos = LastPTLoad.base();
+  for (const SegmentInfo &SI : BC->NewSegments)
+    InsertPos = std::next(Phdrs.insert(InsertPos, createPhdr(SI)));
+
+  OS.write(reinterpret_cast<const char *>(Phdrs.data()),
+           sizeof(ELF64LE::Phdr) * Phdrs.size());
+
   OS.seek(SavedPos);
 }
 

bolt/test/X86/phdr-load-order.test

@@ -0,0 +1,69 @@
+## Check that BOLT places new PT_LOAD segments right after existing PT_LOAD
+## segments, maintaining ascending p_vaddr order.
+
+# RUN: split-file %s %t
+# RUN: yaml2obj %t/yaml -o %t.exe --max-size=0
+# RUN: llvm-bolt %t.exe -o %t.bolt --allow-stripped
+# RUN: llvm-readelf -lW %t.bolt | FileCheck %s
+
+# All PT_LOAD segments must be contiguous and in ascending p_vaddr order.
+# The new BOLT segment must follow the original PT_LOAD segments, not appear
+# at the end of the program header table after non-LOAD segments.
+
+# CHECK:      Program Headers:
+# CHECK-NEXT: Type {{.*}}
+# CHECK-NEXT: LOAD
+# CHECK-NEXT: LOAD
+# CHECK-NEXT: LOAD
+# CHECK-NEXT: LOAD
+# CHECK-NOT:  LOAD
+
+#--- yaml
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data: ELFDATA2LSB
+  Type: ET_EXEC
+  Machine: EM_X86_64
+  Entry: 0x400000
+ProgramHeaders:
+  - Type: PT_LOAD
+    Flags: [ PF_R, PF_X ]
+    FirstSec: .text
+    LastSec: .text
+    VAddr: 0x400000
+    Align: 0x1000
+  - Type: PT_LOAD
+    Flags: [ PF_R, PF_W ]
+    FirstSec: .data
+    LastSec: .data
+    VAddr: 0x500000
+    Align: 0x1000
+  - Type: PT_LOAD
+    Flags: [ PF_R ]
+    FirstSec: .rodata
+    LastSec: .rodata
+    VAddr: 0x600000
+    Align: 0x1000
+  - Type: PT_GNU_STACK
+    Flags: [ PF_R, PF_W ]
+Sections:
+  - Name: .text
+    Type: SHT_PROGBITS
+    Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
+    Content: C3
+    AddressAlign: 0x1
+    Address: 0x400000
+  - Name: .data
+    Type: SHT_PROGBITS
+    Flags: [ SHF_ALLOC, SHF_WRITE ]
+    Content: '00000000'
+    AddressAlign: 0x1
+    Address: 0x500000
+  - Name: .rodata
+    Type: SHT_PROGBITS
+    Flags: [ SHF_ALLOC ]
+    Content: '00000000'
+    AddressAlign: 0x1
+    Address: 0x600000
+...