[ELF] --no-rosegment: don't mark read-only PT_LOAD segments executable

[MaskRay]

Once we move .lrodata after .bss (#78521), or if we use SECTIONS
commands, certain read-only sections may be in their own PT_LOAD, not in
the traditional "text segment". Current --no-rosegment code may
unnecessarily mark read-only PT_LOAD executable. Fix it.

[llvmbot]

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: Fangrui Song (MaskRay)

Changes

Once we move .lrodata after .bss (#78521), or if we use SECTIONS
commands, certain read-only sections may be in their own PT_LOAD, not in
the traditional "text segment". Current --no-rosegment code may
unnecessarily mark read-only PT_LOAD executable. Fix it.

---

Full diff: https://github.com/llvm/llvm-project/pull/81281.diff

2 Files Affected:

• (modified) lld/ELF/Writer.cpp (+17-13)
• (modified) lld/test/ELF/segments.s (+1-1)

diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 6df43a34be013a..bd4db1ecedeaa3 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2353,17 +2353,12 @@ static bool needsPtLoad(OutputSection *sec) {
   return true;
 }
 
-// Linker scripts are responsible for aligning addresses. Unfortunately, most
-// linker scripts are designed for creating two PT_LOADs only, one RX and one
-// RW. This means that there is no alignment in the RO to RX transition and we
-// cannot create a PT_LOAD there.
+// Adjust phdr flags according to certain options.
 static uint64_t computeFlags(uint64_t flags) {
   if (config->omagic)
     return PF_R | PF_W | PF_X;
   if (config->executeOnly && (flags & PF_X))
     return flags & ~PF_R;
-  if (config->singleRoRx && !(flags & PF_W))
-    return flags | PF_X;
   return flags;
 }
 
@@ -2451,8 +2446,8 @@ SmallVector<PhdrEntry *, 0> Writer<ELFT>::createPhdrs(Partition &part) {
     // Segments are contiguous memory regions that has the same attributes
     // (e.g. executable or writable). There is one phdr for each segment.
     // Therefore, we need to create a new phdr when the next section has
-    // different flags or is loaded at a discontiguous address or memory region
-    // using AT or AT> linker script command, respectively.
+    // incompatible flags or is loaded at a discontiguous address or memory
+    // region using AT or AT> linker script command, respectively.
     //
     // As an exception, we don't create a separate load segment for the ELF
     // headers, even if the first "real" output has an AT or AT> attribute.
@@ -2465,13 +2460,22 @@ SmallVector<PhdrEntry *, 0> Writer<ELFT>::createPhdrs(Partition &part) {
     // so when hasSectionsCommand, since we cannot introduce the extra alignment
     // needed to create a new LOAD)
     uint64_t newFlags = computeFlags(sec->getPhdrFlags());
+    // When --no-rosegment is specified, RO and RX sections are compatible.
+    uint32_t incompatible = flags ^ newFlags;
+    if (config->singleRoRx && !(newFlags & PF_W))
+      incompatible &= ~PF_X;
+    if (incompatible)
+      load = nullptr;
+
     bool sameLMARegion =
         load && !sec->lmaExpr && sec->lmaRegion == load->firstSec->lmaRegion;
-    if (!(load && newFlags == flags && sec != relroEnd &&
-          sec->memRegion == load->firstSec->memRegion &&
-          (sameLMARegion || load->lastSec == Out::programHeaders) &&
-          (script->hasSectionsCommand || sec->type == SHT_NOBITS ||
-           load->lastSec->type != SHT_NOBITS))) {
+    if (load && sec != relroEnd &&
+        sec->memRegion == load->firstSec->memRegion &&
+        (sameLMARegion || load->lastSec == Out::programHeaders) &&
+        (script->hasSectionsCommand || sec->type == SHT_NOBITS ||
+         load->lastSec->type != SHT_NOBITS)) {
+      load->p_flags |= newFlags;
+    } else {
       load = addHdr(PT_LOAD, newFlags);
       flags = newFlags;
     }
diff --git a/lld/test/ELF/segments.s b/lld/test/ELF/segments.s
index ee171174ac7ca4..1fe248afa88480 100644
--- a/lld/test/ELF/segments.s
+++ b/lld/test/ELF/segments.s
@@ -44,7 +44,7 @@
 # NOROSEGMENT1-NEXT:  LOAD           0x001006 0x0000000000000006 0x0000000000000006 0x000001 0x000001 RW  0x1000
 # NOROSEGMENT1-NEXT:  LOAD           0x001007 0x0000000000000007 0x0000000000000007 0x000002 0x000002 R E 0x1000
 # NOROSEGMENT1-NEXT:  LOAD           0x001009 0x0000000000000009 0x0000000000000009 0x000001 0x000001 RW  0x1000
-# NOROSEGMENT1-NEXT:  LOAD           0x00100a 0x000000000000000a 0x000000000000000a 0x000001 0x000001 R E 0x1000
+# NOROSEGMENT1-NEXT:  LOAD           0x00100a 0x000000000000000a 0x000000000000000a 0x000001 0x000001 R   0x1000
 # NOROSEGMENT1-NEXT:  GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW  0
 
 # RUN: ld.lld -N a.o -o omagic

[MaskRay]

Closing. This was the final form of #81223 but accidentally created as a separate PR. Pushed 5f26b90 to amend #81223