[lld] [ELF] adjustOutputSections: don't copy SHF_EXECINSTR when an output does not contain input sections (PR #70911)

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 1 10:51:11 PDT 2023


https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/70911

>From 25bc205777844db3f667e060504234a1b3dd0ba7 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Tue, 31 Oct 2023 23:55:10 -0700
Subject: [PATCH] [ELF] adjustOutputSections: don't copy SHF_EXECINSTR when an
 output does not contain input sections

For an output section with no input section, GNU ld eliminates the
output section when there are only symbol assignments (e.g. `.foo : { symbol = 42; }`)
but not for `.foo : { . += 42; }` (`SHF_ALLOC|SHF_WRITE`).

We choose to retain such an output section with a symbol assignment
(unless unreferenced `PROVIDE`). We copy the previous section flag (see
https://reviews.llvm.org/D37736) to hopefully make the current PT_LOAD
segment extend to the current output section:

* decrease the number of PT_LOAD segments
* If a new PT_LOAD segment is introduced without a page-size
  alignment as a separator, there may be a run-time crash.

However, this `flags` copying behavior is not suitable for `.foo : { . += 42; }`
when `flags` contains `SHF_EXECINSTR`. The executable bit is surprising
(https://discourse.llvm.org/t/lld-output-section-flag-assignment-behavior/74359).

I think we should drop SHF_EXECINSTR when copying `flags`.
The risk is a code section followed by `.foo : { symbol = 42; }` will be
broken, which I believe is unrelated as such uses are almost always
related to data sections.

For data-command-only output sections (e.g. `.foo : { QUAD(42) }`),
we keep allowing copyable SHF_WRITE.

Some tests are updated to drop the SHF_EXECINSTR flag. GNU ld doesn't
set SHF_EXECINSTR as well, though it sets SHF_WRITE for some tests while
we don't.
---
 lld/ELF/LinkerScript.cpp                           |  8 +++++---
 lld/docs/ELF/linker_script.rst                     | 10 ++++++++++
 .../linkerscript/empty-synthetic-removed-flags.s   |  1 -
 lld/test/ELF/linkerscript/extend-pt-load2.test     | 13 +++++++------
 lld/test/ELF/linkerscript/insert-before.test       | 11 ++++++-----
 lld/test/ELF/linkerscript/lma-align2.test          | 14 +++++++-------
 6 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index df091613dc0a144..1b8acbe1c908b1d 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -1167,7 +1167,9 @@ void LinkerScript::adjustOutputSections() {
   //   * The address assignment.
   // The other option is to pick flags that minimize the impact the section
   // will have on the rest of the linker. That is why we copy the flags from
-  // the previous sections. Only a few flags are needed to keep the impact low.
+  // the previous sections. We copy just SHF_ALLOC and SHF_WRITE to keep the
+  // impact low. We do not propagate SHF_EXECINSTR as in some cases this can
+  // lead to executable writeable section.
   uint64_t flags = SHF_ALLOC;
 
   SmallVector<StringRef, 0> defPhdrs;
@@ -1193,8 +1195,8 @@ void LinkerScript::adjustOutputSections() {
     // We do not want to keep any special flags for output section
     // in case it is empty.
     if (isEmpty)
-      sec->flags = flags & ((sec->nonAlloc ? 0 : (uint64_t)SHF_ALLOC) |
-                            SHF_WRITE | SHF_EXECINSTR);
+      sec->flags =
+          flags & ((sec->nonAlloc ? 0 : (uint64_t)SHF_ALLOC) | SHF_WRITE);
 
     // The code below may remove empty output sections. We should save the
     // specified program headers (if exist) and propagate them to subsequent
diff --git a/lld/docs/ELF/linker_script.rst b/lld/docs/ELF/linker_script.rst
index fbd96abcb44c5b1..3606ef4fe4b8ee1 100644
--- a/lld/docs/ELF/linker_script.rst
+++ b/lld/docs/ELF/linker_script.rst
@@ -97,6 +97,16 @@ The presence of ``address`` can cause the condition unsatisfied. LLD will warn.
 GNU ld from Binutils 2.35 onwards will reduce sh_addralign so that
 sh_addr=0 (modulo sh_addralign).
 
+When an output section has no input section, GNU ld will eliminate it if it
+only contains symbol assignments (e.g. ``.foo { symbol = 42; }``). LLD will
+retain such sections unless all the symbol assignments are unreferenced
+``PROVIDED``.
+
+When an output section has no input section but advances the location counter,
+GNU ld sets the ``SHF_WRITE`` flag. LLD sets the SHF_WRITE flag only if the
+preceding output section with non-empty input sections also has the SHF_WRITE
+flag.
+
 Output section type
 -------------------
 
diff --git a/lld/test/ELF/linkerscript/empty-synthetic-removed-flags.s b/lld/test/ELF/linkerscript/empty-synthetic-removed-flags.s
index b8d72702425f19f..dfc502e8da9e1d4 100644
--- a/lld/test/ELF/linkerscript/empty-synthetic-removed-flags.s
+++ b/lld/test/ELF/linkerscript/empty-synthetic-removed-flags.s
@@ -29,7 +29,6 @@
 # EMPTY-NEXT:  Type: SHT_PROGBITS
 # EMPTY-NEXT:  Flags [
 # EMPTY-NEXT:    SHF_ALLOC
-# EMPTY-NEXT:    SHF_EXECINSTR
 # EMPTY-NEXT:  ]
 
 .section .foo,"ax"
diff --git a/lld/test/ELF/linkerscript/extend-pt-load2.test b/lld/test/ELF/linkerscript/extend-pt-load2.test
index 3be94a5670d8cfe..6c0e3b2863f9133 100644
--- a/lld/test/ELF/linkerscript/extend-pt-load2.test
+++ b/lld/test/ELF/linkerscript/extend-pt-load2.test
@@ -19,12 +19,13 @@ SECTIONS {
   .data.rel.ro : { *(.data.rel.ro) }
 }
 
-# CHECK:      .rodata      PROGBITS 0000000000000215 000215 000001 00 A  0
-# CHECK-NEXT: foo          PROGBITS 0000000000000216 000216 000000 00 A  0
-# CHECK-NEXT: .text        PROGBITS 0000000000000218 000218 000001 00 AX 0
-# CHECK-NEXT: bar          PROGBITS 0000000000000219 000219 000de7 00 AX 0
+# CHECK:      .rodata      PROGBITS 000000000000024d 00024d 000001 00 A  0
+# CHECK-NEXT: foo          PROGBITS 000000000000024e 00024e 000000 00 A  0
+# CHECK-NEXT: .text        PROGBITS 0000000000000250 000250 000001 00 AX 0
+# CHECK-NEXT: bar          PROGBITS 0000000000000251 000251 000daf 00 A  0
 # CHECK-NEXT: .data.rel.ro PROGBITS 0000000000001000 001000 000001 00 WA 0
 
-# CHECK:      LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x000216 0x000216 R   0x1000
-# CHECK:      LOAD 0x000218 0x0000000000000218 0x0000000000000218 0x000de8 0x000de8 R E 0x1000
+# CHECK:      LOAD 0x000000 0x0000000000000000 0x0000000000000000 0x00024e 0x00024e R   0x1000
+# CHECK-NEXT: LOAD 0x000250 0x0000000000000250 0x0000000000000250 0x000001 0x000001 R E 0x1000
+# CHECK-NEXT: LOAD 0x000251 0x0000000000000251 0x0000000000000251 0x000daf 0x000daf R   0x1000
 # CHECK-NEXT: LOAD 0x001000 0x0000000000001000 0x0000000000001000 0x000068 0x000068 RW  0x1000
diff --git a/lld/test/ELF/linkerscript/insert-before.test b/lld/test/ELF/linkerscript/insert-before.test
index f9611538c013b2b..e6ed413639827a9 100644
--- a/lld/test/ELF/linkerscript/insert-before.test
+++ b/lld/test/ELF/linkerscript/insert-before.test
@@ -9,13 +9,14 @@
 # RUN: llvm-readelf -S -l %t1 | FileCheck %s
 # CHECK:      Name      Type     Address          Off    Size   ES Flg
 # CHECK-NEXT:           NULL
-# CHECK-NEXT: .foo.text PROGBITS 0000000000000000 001000 000008 00  AX
-# CHECK-NEXT: .text     PROGBITS 0000000000000008 001008 000008 00  AX
-# CHECK-NEXT: .byte     PROGBITS 0000000000000010 001010 000001 00  AX
-# CHECK-NEXT: .foo.data PROGBITS 0000000000000011 001011 000008 00  WA
-# CHECK-NEXT: .data     PROGBITS 0000000000000019 001019 000008 00  WA
+# CHECK-NEXT: .foo.text PROGBITS 0000000000000000 001000 000008 00  AX 0
+# CHECK-NEXT: .text     PROGBITS 0000000000000008 001008 000008 00  AX 0
+# CHECK-NEXT: .byte     PROGBITS 0000000000000010 001010 000001 00  A  0
+# CHECK-NEXT: .foo.data PROGBITS 0000000000000011 001011 000008 00  WA 0
+# CHECK-NEXT: .data     PROGBITS 0000000000000019 001019 000008 00  WA 0
 # CHECK:      Type
 # CHECK-NEXT: LOAD {{.*}} R E
+# CHECK-NEXT: LOAD {{.*}} R
 # CHECK-NEXT: LOAD {{.*}} RW
 # CHECK-NEXT: GNU_STACK {{.*}} RW
 
diff --git a/lld/test/ELF/linkerscript/lma-align2.test b/lld/test/ELF/linkerscript/lma-align2.test
index 2177101ff37475d..23b06b179b6e85d 100644
--- a/lld/test/ELF/linkerscript/lma-align2.test
+++ b/lld/test/ELF/linkerscript/lma-align2.test
@@ -7,17 +7,17 @@
 # CHECK:      Name         Type     Address          Off    Size   ES Flg Lk Inf Al
 # CHECK-NEXT:              NULL     0000000000000000 000000 000000 00      0   0  0
 # CHECK-NEXT: .text        PROGBITS 0000000008000000 001000 000001 00  AX  0   0  4
-# CHECK-NEXT: .data        PROGBITS 0000000020000000 002000 000006 00  AX  0   0  8
-# CHECK-NEXT: .data2       PROGBITS 000000000800000e 00200e 000008 00  AX  0   0  1
-# CHECK-NEXT: .data3       PROGBITS 0000000020000008 002018 000000 00  AX  0   0  8
-# CHECK-NEXT: .data4       PROGBITS 0000000008000018 002018 000008 00  AX  0   0  1
+# CHECK-NEXT: .data        PROGBITS 0000000020000000 002000 000006 00  A   0   0  8
+# CHECK-NEXT: .data2       PROGBITS 000000000800000e 00200e 000008 00  A   0   0  1
+# CHECK-NEXT: .data3       PROGBITS 0000000020000008 002018 000000 00  A   0   0  8
+# CHECK-NEXT: .data4       PROGBITS 0000000008000018 002018 000008 00  A   0   0  1
 
 
 # CHECK:      Type  Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
 # CHECK-NEXT: LOAD  0x001000 0x0000000008000000 0x0000000008000000 0x000001 0x000001 R E 0x1000
-# CHECK-NEXT: LOAD  0x002000 0x0000000020000000 0x0000000008000008 0x000006 0x000006 R E 0x1000
-# CHECK-NEXT: LOAD  0x00200e 0x000000000800000e 0x000000000800000e 0x000008 0x000008 R E 0x1000
-# CHECK-NEXT: LOAD  0x002018 0x0000000008000018 0x0000000008000018 0x000008 0x000008 R E 0x1000
+# CHECK-NEXT: LOAD  0x002000 0x0000000020000000 0x0000000008000008 0x000006 0x000006 R   0x1000
+# CHECK-NEXT: LOAD  0x00200e 0x000000000800000e 0x000000000800000e 0x000008 0x000008 R   0x1000
+# CHECK-NEXT: LOAD  0x002018 0x0000000008000018 0x0000000008000018 0x000008 0x000008 R   0x1000
 
 MEMORY {
   CODE (rx) : ORIGIN = 0x08000000, LENGTH = 100K



More information about the llvm-commits mailing list