[ELF] Error if a section address is smaller than image base #140187
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.5-bogner
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-elf Author: Fangrui Song (MaskRay) ChangesWhen -Ttext= or --section-start specifies an address lower than the Full diff: https://github.com/llvm/llvm-project/pull/140187.diff 4 Files Affected:
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 6a0552e808c7b..ea11841731d27 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -1615,17 +1615,31 @@ template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
sec->addr = 0;
// If addrExpr is set, the address may not be a multiple of the alignment.
- // Warn because this is error-prone.
- for (SectionCommand *cmd : ctx.script->sectionCommands)
- if (auto *osd = dyn_cast<OutputDesc>(cmd)) {
- OutputSection *osec = &osd->osec;
- if (osec->addr % osec->addralign != 0)
- Warn(ctx) << "address (0x" << Twine::utohexstr(osec->addr)
- << ") of section " << osec->name
- << " is not a multiple of alignment (" << osec->addralign
- << ")";
+ // Warn because this is error-prone. In addition, warn if the address
+ // is smaller than the image base when SECTIONS is absent (e.g. when -Ttext
+ // is specified and smaller than the default target image base for no-pie).
+ uint64_t imageBase = ctx.script->hasSectionsCommand || ctx.arg.relocatable
+ ? 0
+ : ctx.target->getImageBase();
+ for (SectionCommand *cmd : ctx.script->sectionCommands) {
+ auto *osd = dyn_cast<OutputDesc>(cmd);
+ if (!osd)
+ continue;
+ OutputSection *osec = &osd->osec;
+ if (osec->addr < imageBase && (osec->flags & SHF_ALLOC)) {
+ Warn(ctx) << "section '" << osec->name << "' address (0x"
+ << Twine::utohexstr(osec->addr)
+ << ") is smaller than image base (0x"
+ << Twine::utohexstr(imageBase) << "); specify --image-base";
}
+ if (osec->addr % osec->addralign != 0)
+ Warn(ctx) << "address (0x" << Twine::utohexstr(osec->addr)
+ << ") of section " << osec->name
+ << " is not a multiple of alignment (" << osec->addralign
+ << ")";
+ }
+
// Sizes are no longer allowed to grow, so all allowable spills have been
// taken. Remove any leftover potential spills.
ctx.script->erasePotentialSpillSections();
diff --git a/lld/test/ELF/linkerscript/out-of-order.s b/lld/test/ELF/linkerscript/out-of-order.s
index 9b834cf712038..2f990e0205f0e 100644
--- a/lld/test/ELF/linkerscript/out-of-order.s
+++ b/lld/test/ELF/linkerscript/out-of-order.s
@@ -31,6 +31,18 @@
# CHECK-NEXT: 5 .hash 00000010 000000000000201c
# CHECK-NEXT: 6 .text 00000008 000000000000202c
+# RUN: ld.lld -e 0 -o %t --script %t.script %t.o --fatal-warnings
+# RUN: llvm-readelf -Sl %t | FileCheck %s --check-prefix=CHECK1
+
+# CHECK1: Name Type Address Off Size ES Flg Lk Inf Al
+# CHECK1-NEXT: NULL 0000000000000000 000000 000000 00 0 0 0
+# CHECK1-NEXT: .text PROGBITS 0000000000000000 001000 000008 00 AX 0 0 4
+# CHECK1-NEXT: .data PROGBITS 0000000000004000 002000 000008 00 WA 0 0 1
+# CHECK1: Program Headers:
+# CHECK1-NEXT: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
+# CHECK1-NEXT: LOAD 0x001000 0x0000000000000000 0x0000000000000000 0x000008 0x000008 R E 0x1000
+# CHECK1-NEXT: LOAD 0x002000 0x0000000000004000 0x0000000000004000 0x000008 0x000008 RW 0x1000
+
.quad 0
.data
.quad 0
diff --git a/lld/test/ELF/linkerscript/section-align2.test b/lld/test/ELF/linkerscript/section-align2.test
index 3eb42cadc059e..3269633b92930 100644
--- a/lld/test/ELF/linkerscript/section-align2.test
+++ b/lld/test/ELF/linkerscript/section-align2.test
@@ -10,7 +10,7 @@
# RUN: llvm-readelf -S %t | FileCheck %s
## Check we don't warn in the absence of SECTIONS.
-# RUN: ld.lld --fatal-warnings -Ttext=0x10000 %t.o -o /dev/null
+# RUN: ld.lld --fatal-warnings -Ttext=0x10000 --image-base=0x10000 %t.o -o /dev/null
# WARN: warning: address (0x10004) of section .data.rel.ro is not a multiple of alignment (16)
# WARN: warning: address (0x20001) of section .data2 is not a multiple of alignment (8)
diff --git a/lld/test/ELF/sectionstart.s b/lld/test/ELF/sectionstart.s
index d694c9375fd80..a1202bc185a70 100644
--- a/lld/test/ELF/sectionstart.s
+++ b/lld/test/ELF/sectionstart.s
@@ -1,9 +1,13 @@
# REQUIRES: x86
# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
# RUN: ld.lld %t.o --section-start .text=0x100000 \
-# RUN: --section-start=.data=0x110000 --section-start .bss=0x200000 -o %t
+# RUN: --section-start=.data=0x110000 --section-start .bss=0x200000 -o %t 2>&1 | \
+# RUN: FileCheck %s --check-prefix=LINK --implicit-check-not=warning:
# RUN: llvm-objdump --section-headers %t | FileCheck %s
+# LINK: warning: section '.text' address (0x100000) is smaller than image base (0x200000); specify --image-base
+# LINK-NEXT: warning: section '.data' address (0x110000) is smaller than image base (0x200000); specify --image-base
+
# CHECK: Sections:
# CHECK-NEXT: Idx Name Size VMA Type
# CHECK-NEXT: 0 00000000 0000000000000000
@@ -11,9 +15,14 @@
# CHECK-NEXT: 2 .data 00000004 0000000000110000 DATA
# CHECK-NEXT: 3 .bss 00000004 0000000000200000 BSS
-## The same, but dropped "0x" prefix.
-# RUN: ld.lld %t.o --section-start .text=100000 \
-# RUN: --section-start .data=110000 --section-start .bss=0x200000 -o %t1
+## The warnings go away when the image base is 0.
+# RUN: ld.lld %t.o -pie --section-start .text=0x100000 \
+# RUN: --section-start=.data=0x110000 --section-start .bss=0x200000 -o %t --noinhibit-exec
+# RUN: llvm-objdump --section-headers %t | FileCheck %s
+
+## The same, but dropped "0x" prefix. Specify a smaller --image-base to suppress warnings.
+# RUN: ld.lld %t.o --image-base=0x90000 --section-start .text=100000 \
+# RUN: --section-start .data=110000 --section-start .bss=0x200000 -o %t1 --noinhibit-exec
# RUN: llvm-objdump --section-headers %t1 | FileCheck %s
## Use -Ttext, -Tdata, -Tbss as replacement for --section-start:
|
smithp35
approved these changes
May 16, 2025
There was a problem hiding this comment.
I wish GNU ld had a better specification of what these options are meant to do.
I've set approved as a warning is definitely an improvement. I've got a few suggestions on some alternatives but I don't have a particularly strong case for any of them as this is likely a corner case.
It looks like we're defining --image-base to be the address of the first loadable segment (and not specifically the first loadable text segment as in ld.bfd, or the first loadable ro-data segment using lld order). With that definition any section placed lower than --image-base is now generating a warning, and we're generating a loadable program header (potentially out of order for it anyway).
There's a few alternatives we could consider:
- It is an error if a
SHF_ALLOCis out of bounds of the lowest loadable segment. - We take the instruction literally and the out of bounds section does not get a program header, this includes potentially making the ELF header and segement header non allocateable.
- We could lower (with a warning) the image base to the lowest allocateable section.
A possible addition when this occurs is to set the headers as not allocatable so we don't create the program header for the ELF header and Program Header.
I've got a mild preference to making this an error.
As an aside the ELF spec can be satisfied by sorting the loadable program header entries by p_vaddr. The segments in the file don't have to be in order if we're going by the letter of the specification. That could be worth doing anyway to help out people with strict loaders but without a linker script (or suitable command-line option) that has ascending segment p_vaddr order.
|
Thanks for the analysis! When there is a PT_LOAD segment that covers ELF header/program headers but no sections
I agree that this diagnostic should be elevated to an error for clarity. 37 tests that use -Ttext= will need adjustment. (Assemble sectionstart.s to a.o) |
MaskRay
added a commit
that referenced
this pull request
May 17, 2025
…ss sensitive Many tests specify -Ttext, -Tdata, or --section-start without SECTIONS commands. When the specified address is lower than the image base, * In the default --ro-segment case, there will be a read-only PT_LOAD segment covering ELF header/program headers, which may cover no section (as many tests omit .rodata). This appears unusual and has non-ascending p_vaddr. * In the --no-rosegment case, an `error: output file too large` occurs. We will add a diagnostic (#140187) and possibly change layout. Add --image-base to make these tests less sensitive.
Created using spr 1.3.5-bogner
MaskRay
changed the title
smithp35
reviewed
May 19, 2025
There was a problem hiding this comment.
Change to error LGTM.
Looks to be some Bolt tests using --section-start that need updating (details in CI) as these are triggering the error.
We probably should add this change to the release notes as a potentially breaking change (without using --image-base).
mysterymath
approved these changes
May 19, 2025
MaskRay
added a commit
that referenced
this pull request
May 21, 2025
When using -no-pie without a SECTIONS command, the linker uses the target's default image base. If -Ttext= or --section-start specifies an output section address below this base, the result is likely unintended. LLD will give a diagnostic (#140187) and may change the behavior in the future. It's good to set an explicit image base to avoid relying on its current behavior. BOLT doesn't seem to care whether a PT_PHDR segment is present. Pull Request: #140570
Created using spr 1.3.5-bogner
Created using spr 1.3.5-bogner
MaskRay
deleted the
users/MaskRay/spr/elf-warn-if-specified-section-address-is-smaller-than-image-base
branch
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
May 21, 2025
When using `-no-pie` without a `SECTIONS` command, the linker uses the target's default image base. If `-Ttext=` or `--section-start` specifies an output section address below this base, the result is likely unintended. - With `--no-rosegment`, the PT_LOAD segment covering the ELF header cannot include `.text` if `.text`'s address is too low, causing an `error: output file too large`. - With default `--rosegment`: - If a read-only section (e.g., `.rodata`) exists, a similar `error: output file too large` occurs. - Without read-only sections, the PT_LOAD segment covering the ELF header and program headers includes no sections, which is unusual and likely undesired. This also causes non-ascending PT_LOAD `p_vaddr` values related to the PT_LOAD that overlaps with PT_PHDR (#138584). To prevent these issues, report an error if a section address is below the image base and suggest `--image-base`. This check also applies when `--image-base` is explicitly set but is skipped when a `SECTIONS` command is used. Pull Request: llvm/llvm-project#140187
MaskRay
pushed a commit
that referenced
this pull request
May 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When using
-no-piewithout aSECTIONScommand, the linker uses thetarget's default image base. If
-Ttext=or--section-startspecifiesan output section address below this base, the result is likely
unintended.
--no-rosegment, the PT_LOAD segment covering the ELF header cannot include.textif.text's address is too low, causing anerror: output file too large.--rosegment:.rodata) exists, a similarerror: output file too largeoccurs.p_vaddrvalues related to the PT_LOAD that overlaps with PT_PHDR ([lld] Unsorted loadable program headers with-Ttext=argument #138584).To prevent these issues, report an error if a section address is below
the image base and suggest
--image-base. This check also applies when--image-baseis explicitly set but is skipped when aSECTIONScommand is used.