[ELF] Avoid adding an orphan section to a less suitable segment

[ikudrin]

Summary

This is an alternative for D111137.

If segments are defined in a linker script, placing an orphan section before the found closest-rank section can result in adding it in a previous segment and changing flags of that segment. This happens if the orphan section has a lower sort rank than the found section. To avoid that, the patch forces orphan sections to be moved after the found sections if segments are explicitly defined.

[MaskRay]

Adding an orphan before the (previously) first section of a segment can create unintended effects (say the user specifies output section address for the first section).
We have quite refined RankFlags. If the needle section sec mostly matches the first section but has a refined rank, it makes sense to ignore the additional bits in this case.

It may be useful to strengthen the test by leveraging non-PROGBITS and PROGBITS read-only sections (RF_RODATA).

Am I right that this makes our behavior closer to ld.bfd? If yes, this looks quite nice.

---

As of PT_GNU_RELRO, this may be another fallout of our default -z relro. We probably shouldn't change it now.
If a generic improvement like this patch makes the relro issue moot, that is certainly great.

lld/test/ELF/linkerscript/orphan-phdrs2.test

13

CHECK-NEXT:

16

NOT is not needed. A positive CHECK exists.

[ikudrin]

• Added CHECK-NEXT checks
• Added checks for read-only orphan sections

In D111717#3067381, @MaskRay wrote:

Am I right that this makes our behavior closer to ld.bfd? If yes, this looks quite nice.

I hope so.

lld/test/ELF/linkerscript/orphan-phdrs2.test

16

As a section can be put in several segments, I initially added this CHECK-NOT to ensure/demonstrate that .dynamic is not in the first PT_LOAD segment. A positive CHECK can be used, but with a terminating {{$}}.

[MaskRay]

LGTM. Worth checking with @peter.smith as well.

[MaskRay]

lld/ELF/Writer.cpp

1261

the blank line may be deleted

1264

[MaskRay]

Avoid adding orphan sections to a less fit segment

I may use something like: "Make orphan section at the boundary of two sort ranks belong to the latter one"

[ikudrin]

In D111717#3073352, @MaskRay wrote:

Avoid adding orphan sections to a less fit segment

I may use something like: "Make orphan section at the boundary of two sort ranks belong to the latter one"

That does not seem to be accurate enough because the sort rank is adjusted only in presence of PHDRS.

[peter.smith]

I don't have any fundamental objections. I've left my comments on D111137 about disabling RELRO for PHDRS like ld.bfd does and put more burden on the user to give a more complete linker script. However this won't get in the way of a more complete linker script and I prefer this to D111137 so I'm happy to go with the consensus.

lld/ELF/Writer.cpp

findOrphanPos(std::vector<BaseCommand *>::iterator b,
   OutputSection *sec = cast<OutputSection>(*e);
 
   // Find the first element that has as close a rank as possible.
   auto i = std::max_element(b, e, [=](BaseCommand *a, BaseCommand *b) {
     return getRankProximity(sec, a) < getRankProximity(sec, b);
   });
   if (i == e)
     return e;
+  auto foundSec = dyn_cast<OutputSection>(*i);
+  if (!foundSec)
+    return e;
 
   // Consider all existing sections with the same proximity.
   int proximity = getRankProximity(sec, *i);
+  unsigned sortRank = sec->sortRank;
+  if (script->hasPhdrsCommands())
+    // Prevent the orphan section to be placed before the found section because
+    // that can result in adding it to a previous segment and changing flags of
+    // that segment, for example, making a read-only segment writable.
+    sortRank = std::max(sortRank, foundSec->sortRank);
   for (; i != e; ++i) {
     auto *curSec = dyn_cast<OutputSection>(*i);
     if (!curSec || !curSec->hasInputSections)
       continue;
     if (getRankProximity(sec, curSec) != proximity ||
-        sec->sortRank < curSec->sortRank)
+        sortRank < curSec->sortRank)
       break;
   }
 
   auto isOutputSecWithInputSections = [](BaseCommand *cmd) {
     auto *os = dyn_cast<OutputSection>(cmd);
     return os && os->hasInputSections;
   };
   auto j = std::find_if(llvm::make_reverse_iterator(i),

lld/test/ELF/aarch64-thunk-pi.s

  .section .text_low, "ax", %progbits
  .globl low_target
  .type low_target, %function
 low_target:
  // Need thunk to high_target@plt
  bl high_target
  ret
 // CHECK: <low_target>:
-// CHECK-NEXT:       d8:       bl      0xec <__AArch64ADRPThunk_high_target>
+// CHECK-NEXT:        0:       bl      0x14 <__AArch64ADRPThunk_high_target>
 // CHECK-NEXT:                 ret
 
  .hidden low_target2
  .globl low_target2
  .type low_target2, %function
 low_target2:
  // Need thunk to high_target2
  bl high_target2
  // .text_high+8 = high_target2
  bl .text_high+8
  ret
 // CHECK: <low_target2>:
-// CHECK-NEXT:       e0:       bl      0xf8 <__AArch64ADRPThunk_high_target2>
-// CHECK-NEXT:       e4:       bl      0x104 <__AArch64ADRPThunk_>
+// CHECK-NEXT:        8:       bl      0x20 <__AArch64ADRPThunk_high_target2>
+// CHECK-NEXT:        c:       bl      0x2c <__AArch64ADRPThunk_>
 // CHECK-NEXT:                 ret
 
 // Expect range extension thunks for .text_low
 // adrp calculation is (PC + signed immediate) & (!0xfff)
 // CHECK: <__AArch64ADRPThunk_high_target>:
-// CHECK-NEXT:       ec:       adrp    x16, 0x10000000
+// CHECK-NEXT:       14:       adrp    x16, 0x10000000
 // CHECK-NEXT:                 add     x16, x16, #0x40
 // CHECK-NEXT:                 br      x16
 // CHECK: <__AArch64ADRPThunk_high_target2>:
-// CHECK-NEXT:       f8:       adrp    x16, 0x10000000
+// CHECK-NEXT:       20:       adrp    x16, 0x10000000
 // CHECK-NEXT:                 add     x16, x16, #0x8
 // CHECK-NEXT:                 br      x16
 /// Identical to the previous one, but for the target .text_high+8.
 // CHECK: <__AArch64ADRPThunk_>:
-// CHECK-NEXT:      104:       adrp    x16, 0x10000000
+// CHECK-NEXT:       2c:       adrp    x16, 0x10000000
 // CHECK-NEXT:                 add     x16, x16, #0x8
 // CHECK-NEXT:                 br      x16
 
 
  .section .text_high, "ax", %progbits
  .globl high_target
  .type high_target, %function
 high_target:

 // CHECK: <high_target2>:
 // CHECK-NEXT: 10000008:       bl      0x10000010 <__AArch64ADRPThunk_low_target2>
 // CHECK-NEXT:                 ret
 
 // Expect Thunk for .text.high
 
 // CHECK: <__AArch64ADRPThunk_low_target2>:
 // CHECK-NEXT: 10000010:       adrp    x16, 0x0
-// CHECK-NEXT:                 add     x16, x16, #0xe0
+// CHECK-NEXT:                 add     x16, x16, #0x8
 // CHECK-NEXT:                 br      x16
 
 // CHECK: Disassembly of section .plt:
 // CHECK-EMPTY:
 // CHECK-NEXT: <.plt>:
 // CHECK-NEXT: 10000020:       stp     x16, x30, [sp, #-0x10]!
 // CHECK-NEXT:                 adrp    x16, 0x10000000
-// CHECK-NEXT:                 ldr     x17, [x16, #0x120]
-// CHECK-NEXT:                 add     x16, x16, #0x120
+// CHECK-NEXT:                 ldr     x17, [x16, #0x1f8]
+// CHECK-NEXT:                 add     x16, x16, #0x1f8
 // CHECK-NEXT:                 br      x17
 // CHECK-NEXT:                 nop
 // CHECK-NEXT:                 nop
 // CHECK-NEXT:                 nop
 // CHECK-EMPTY:
 // CHECK-NEXT:   <high_target@plt>:
 // CHECK-NEXT: 10000040:       adrp    x16, 0x10000000
-// CHECK-NEXT:                 ldr     x17, [x16, #0x128]
-// CHECK-NEXT:                 add     x16, x16, #0x128
+// CHECK-NEXT:                 ldr     x17, [x16, #0x200]
+// CHECK-NEXT:                 add     x16, x16, #0x200
 // CHECK-NEXT:                 br      x17
 // CHECK-EMPTY:
 // CHECK-NEXT:   <low_target@plt>:
 // CHECK-NEXT: 10000050:       adrp    x16, 0x10000000
-// CHECK-NEXT:                 ldr     x17, [x16, #0x130]
-// CHECK-NEXT:                 add     x16, x16, #0x130
+// CHECK-NEXT:                 ldr     x17, [x16, #0x208]
+// CHECK-NEXT:                 add     x16, x16, #0x208
 // CHECK-NEXT:                 br      x17
 
 //--- lds
 PHDRS {
   low PT_LOAD FLAGS(0x1 | 0x4);
   high PT_LOAD FLAGS(0x1 | 0x4);
 }
 SECTIONS {
   .text_low : { *(.text_low) } :low
   .text_high 0x10000000 : { *(.text_high) } :high
 }

lld/test/ELF/linkerscript/implicit-program-header.test

 # REQUIRES: x86
 
 # RUN: echo '.section .text,"ax"; .quad 0' > %t.s
 # RUN: echo '.section .foo,"ax"; .quad 0' >> %t.s
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %t.s -o %t.o
 # RUN: ld.lld --hash-style=sysv -o %t1 --script %s %t.o -shared
 # RUN: llvm-readelf -l %t1 | FileCheck %s
 
 # CHECK:      Segment Sections...
-# CHECK-NEXT:   00     .dynsym .hash .dynstr .text
-# CHECK-NEXT:   01     .foo .dynamic
-# CHECK-NEXT:   02     .foo .dynamic
+# CHECK-NEXT:   00     .text
+# CHECK-NEXT:   01     .foo .dynsym .hash .dynstr .dynamic
+# CHECK-NEXT:   02     .foo .dynsym .hash .dynstr .dynamic
 
 PHDRS {
   ph_write PT_LOAD FLAGS(2);
   ph_exec  PT_LOAD FLAGS(1);
   ph_note  PT_NOTE;
 }
 
 SECTIONS {
   .text : { *(.text) } : ph_write
   .bar : { *(.bar) } : ph_exec : ph_note
   .foo : { *(.foo) }
 }

lld/test/ELF/linkerscript/orphan-phdrs2.test

+# REQUIRES: x86
+
+# RUN: split-file %s %ts
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %ts/s -o %t.o
+# RUN: ld.lld -pie -o %t -T %ts/t %t.o
+# RUN: llvm-readelf -l %t | FileCheck %s
+
+## Check that an orphan section '.dynamic' is added to the same segment as
+## its closest-rank section '.data', even though its sort rank is lower.
+## Adding '.dynamic' to the first segment would make the segment writable.
+# CHECK:      Program Headers:
+# CHECK-NEXT:   Type {{.*}} Flg Align
+# CHECK-NEXT:   LOAD {{.*}} R E 0x
+# CHECK-NEXT:   LOAD {{.*}} RW  0x
+# CHECK-MEXT:   LOAD {{.*}} R   0x
+
+# CHECK:      Segment Sections...
+# CHECK-NEXT:   00 .text {{$}}
+# CHECK-NEXT:   01 .data .dynamic {{$}}
+## Check that read-only non-PROGBITS orphan sections are placed after the
+## closest-rank section '.rodata' despite their sort ranks are lower.
+# CHECK-NEXT:   02 .rodata .dynsym .gnu.hash .hash .dynstr {{$}}
+
+#--- s
+  .text
+  nop
+
+  .data
+  .quad 0
+
+  .rodata
+  .quad 0
+
+#--- t
+PHDRS {
+  exec PT_LOAD;
+  rw   PT_LOAD;
+  ro   PT_LOAD;
+}
+SECTIONS {
+  .text : { *(.text) } : exec
+  .data : { *(.data) } : rw
+  .rodata : { *(.rodata) } : ro
+}