Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions lld/ELF/LinkerScript.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be worth updating the comment on line 1170

  // 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.

Perhaps
"// We do not propagate SHF_EXECINSTR as in some cases this can lead to executable writeable SHT_NOBITS section."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. Reworded a bit

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
Expand Down
10 changes: 10 additions & 0 deletions lld/docs/ELF/linker_script.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------------------

Expand Down
1 change: 0 additions & 1 deletion lld/test/ELF/linkerscript/empty-synthetic-removed-flags.s
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 7 additions & 6 deletions lld/test/ELF/linkerscript/extend-pt-load2.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 6 additions & 5 deletions lld/test/ELF/linkerscript/insert-before.test
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
14 changes: 7 additions & 7 deletions lld/test/ELF/linkerscript/lma-align2.test
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down