[ELF] -r/--emit-relocs: Fix crash when processing .rela.text before .text #156354

mykouHW · 2025-09-01T16:32:50Z

When the relocation section is placed before the relocated section and the relocated section is not defined in the linker script, an error will occur during the linking process.

Issue Cause:
In a.ro, .rela.text precedes its relocated InputSection .text. addOrphanSections doesn't handle this scenario.
When it processes .rela.text, in the called getOutputSectionName, rel->getOutputSection() is nullptr (input .text doesn't yet have a parent output section), leading to an assertion failure.

Solution:
For --emit-relocs and -r, ensure the output section for .text.foo is created before the output section for .rela.text.foo.

github-actions · 2025-09-01T16:33:08Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-09-01T16:33:40Z

@llvm/pr-subscribers-lld

Author: None (mykouHW)

Changes

Fix the error generated during the linking process when the relocation section is placed before the relocated section and the relocated section is not defined in the linker script.

Issue Cause:
In the judgment logic, addOrphanSections assumes that the RelocatedSection must be processed before the RelocationSection. Under this assumption, the OutputSection for the RelocatedSection has already been constructed, and the parent relationship associated with the InputSectionBase has been established.

If the RelocationSection is processed before the RelocatedSection, this assumption is violated. As a result, the condition rel->parent evaluates to null, causing add(relIS) to not execute. This skips the registration and construction process of the RelocatedSection, since its createOutputSection and recordSection methods have not yet been called at this point.

However, during the construction and registration of the RelocationSection in the addInputSec function, the RelocatedSection is accessed. Since the RelocatedSection has not been constructed yet, attempting to access it results in a null pointer error.

Solution:
Before processing the RelocationSection, ensure that the OutputSection for the RelocatedSection is created and registered. The creation and registration logic is protected by the add function, which prevents duplicate creation. However, it may result in duplicate establishment of the parent relationship, which does not affect correctness.

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

2 Files Affected:

(modified) lld/ELF/LinkerScript.cpp (+6-2)
(added) lld/test/ELF/linkerscript/orphan-sections-init.s (+30)

diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index 921128dae2bdb..067abbc42a13d 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -1037,10 +1037,14 @@ void LinkerScript::addOrphanSections() {
     if (ctx.arg.relocatable && (isec->flags & SHF_LINK_ORDER))
       continue;
 
-    if (auto *sec = dyn_cast<InputSection>(isec))
-      if (InputSectionBase *rel = sec->getRelocatedSection())
+    if (auto *sec = dyn_cast<InputSection>(isec)){
+      if (InputSectionBase *rel = sec->getRelocatedSection()){
+        if (auto *relIS = dyn_cast_or_null<InputSectionBase>(rel))
+          add(relIS);
         if (auto *relIS = dyn_cast_or_null<InputSectionBase>(rel->parent))
           add(relIS);
+      }
+    }
     add(isec);
     if (ctx.arg.relocatable)
       for (InputSectionBase *depSec : isec->dependentSections)
diff --git a/lld/test/ELF/linkerscript/orphan-sections-init.s b/lld/test/ELF/linkerscript/orphan-sections-init.s
new file mode 100644
index 0000000000000..1701336f098e2
--- /dev/null
+++ b/lld/test/ELF/linkerscript/orphan-sections-init.s
@@ -0,0 +1,30 @@
+# REQUIRES: x86
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: split-file %s %t && cd %t
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 foo.s -o foo.o
+
+# RUN: ld.lld -r  foo.o -T script.ld -o foo_mc.o
+
+# RUN: llvm-objcopy --rename-section .text=.com.text foo_mc.o foo_mc.o
+# RUN: llvm-objcopy --rename-section .rela.text=.rela.com.text foo_mc.o foo_mc.o
+
+# RUN: ld.lld -r foo_mc.o  -T script.ld -o foo_mc_after.o
+
+#--- foo.s
+  .text
+  .globl	foo
+  .p2align	4
+  .type	foo,@function
+foo:
+  mov $bar, %rax
+
+
+
+#--- script.ld
+SECTIONS
+{
+  .rela.text    0 : { *(.rela.text) }
+  .text         0 : { *(.text) }
+}
+

llvmbot · 2025-09-01T16:33:40Z

@llvm/pr-subscribers-lld-elf

Author: None (mykouHW)

Changes

Fix the error generated during the linking process when the relocation section is placed before the relocated section and the relocated section is not defined in the linker script.

Issue Cause:
In the judgment logic, addOrphanSections assumes that the RelocatedSection must be processed before the RelocationSection. Under this assumption, the OutputSection for the RelocatedSection has already been constructed, and the parent relationship associated with the InputSectionBase has been established.

If the RelocationSection is processed before the RelocatedSection, this assumption is violated. As a result, the condition rel->parent evaluates to null, causing add(relIS) to not execute. This skips the registration and construction process of the RelocatedSection, since its createOutputSection and recordSection methods have not yet been called at this point.

However, during the construction and registration of the RelocationSection in the addInputSec function, the RelocatedSection is accessed. Since the RelocatedSection has not been constructed yet, attempting to access it results in a null pointer error.

Solution:
Before processing the RelocationSection, ensure that the OutputSection for the RelocatedSection is created and registered. The creation and registration logic is protected by the add function, which prevents duplicate creation. However, it may result in duplicate establishment of the parent relationship, which does not affect correctness.

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

2 Files Affected:

(modified) lld/ELF/LinkerScript.cpp (+6-2)
(added) lld/test/ELF/linkerscript/orphan-sections-init.s (+30)

diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index 921128dae2bdb..067abbc42a13d 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -1037,10 +1037,14 @@ void LinkerScript::addOrphanSections() {
     if (ctx.arg.relocatable && (isec->flags & SHF_LINK_ORDER))
       continue;
 
-    if (auto *sec = dyn_cast<InputSection>(isec))
-      if (InputSectionBase *rel = sec->getRelocatedSection())
+    if (auto *sec = dyn_cast<InputSection>(isec)){
+      if (InputSectionBase *rel = sec->getRelocatedSection()){
+        if (auto *relIS = dyn_cast_or_null<InputSectionBase>(rel))
+          add(relIS);
         if (auto *relIS = dyn_cast_or_null<InputSectionBase>(rel->parent))
           add(relIS);
+      }
+    }
     add(isec);
     if (ctx.arg.relocatable)
       for (InputSectionBase *depSec : isec->dependentSections)
diff --git a/lld/test/ELF/linkerscript/orphan-sections-init.s b/lld/test/ELF/linkerscript/orphan-sections-init.s
new file mode 100644
index 0000000000000..1701336f098e2
--- /dev/null
+++ b/lld/test/ELF/linkerscript/orphan-sections-init.s
@@ -0,0 +1,30 @@
+# REQUIRES: x86
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: split-file %s %t && cd %t
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 foo.s -o foo.o
+
+# RUN: ld.lld -r  foo.o -T script.ld -o foo_mc.o
+
+# RUN: llvm-objcopy --rename-section .text=.com.text foo_mc.o foo_mc.o
+# RUN: llvm-objcopy --rename-section .rela.text=.rela.com.text foo_mc.o foo_mc.o
+
+# RUN: ld.lld -r foo_mc.o  -T script.ld -o foo_mc_after.o
+
+#--- foo.s
+  .text
+  .globl	foo
+  .p2align	4
+  .type	foo,@function
+foo:
+  mov $bar, %rax
+
+
+
+#--- script.ld
+SECTIONS
+{
+  .rela.text    0 : { *(.rela.text) }
+  .text         0 : { *(.text) }
+}
+

smithp35

I recommend not using the word vulnerability in titles and description as this is usually reserved for security bugs.

I suggest something like `[lld][ELF] Fix crash when relocations proceed relocated section.

I also recommend putting the analysis in the linked issue into the description as this will form the commit message. It is much easier to find and search for changes with git log when the commit message has all the details.

smithp35 · 2025-09-02T11:59:21Z

lld/ELF/LinkerScript.cpp

could rel be renamed to relocated here?

Not part of your change, but I think it would be an improvement. When reviewing just the diff, it was difficult to tell from the name rel whether we are referring to the relocation section, or the section being relocated.

smithp35 · 2025-09-02T12:05:54Z

lld/ELF/LinkerScript.cpp

Can you git clang-format the patch? I would expect a space between ){ i.e. (isec)) {

smithp35 · 2025-09-02T12:16:06Z

lld/ELF/LinkerScript.cpp

Could we add a comment before the if statement like
// Ensure creation of OutputSection for relocated section before relocation section.

smithp35 · 2025-09-02T12:16:44Z

lld/test/ELF/linkerscript/orphan-sections-init.s

You can merge do both -rename-section commands with a single instance of llvm-objcopy.

smithp35 · 2025-09-02T12:18:50Z

lld/test/ELF/linkerscript/orphan-sections-init.s

I recommend a CHECK of the llvm-readelf --sections output to make sure lld has created the link from the relocations correctly.

I've seen some scripts been run to find tests with no FileCheck output before so it is worth putting at least one in.

smithp35 · 2025-09-02T12:30:03Z

lld/test/ELF/linkerscript/orphan-sections-init.s

Could you add a comment to explain the test. Without context the steps look strange. LLD uses an additional comment character to distinguish comments from lit commands, so comment lines would start ##

For example:

Test that lld's orphan section placement can handle a relocatable link where the relocation section is seen before the relocated section. To create a test case we need to first need to create a relocatable object with the relocations before the relocated section. Then we rename these to make these orphans.

I tried to see if I could create the test case without llvm-objcopy but it seemed like I needed the linker script to have .rela.text as the pattern to get it to place first.

The llvm-objcopy is unnecessary. Apply https://github.com/MaskRay/llvm-project/tree/lld-156354 to get the code/comment/test changes

Fix the error generated during the linking process when the relocation section is placed before the relocated section and the relocated section is not defined in the linker script.

smithp35

Thanks for the update. I've enabled the CI. Assuming that goes well then this looks good to me.

I've added the maintainer @MaskRay to see if he has any opinions.

mykouHW · 2025-09-18T03:47:49Z

@MaskRay Hello, do you have any suggestions regarding this PR? :)

MaskRay · 2025-09-18T16:44:55Z

I think the description needs to be clarified. My comment at 156417 could be used. I'll push the change like
https://github.com/MaskRay/llvm-project/tree/lld-156354 and merge this for you.

mykouHW · 2025-09-19T02:12:47Z

I think the description needs to be clarified. My comment at 156417 could be used. I'll push the change like https://github.com/MaskRay/llvm-project/tree/lld-156354 and merge this for you.

That's great, thank you for your revisions and assistance!

github-actions · 2025-09-20T16:42:13Z

@mykouHW Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

MaskRay · 2025-09-20T21:11:51Z

/cherry-pick 507f394

llvmbot · 2025-09-20T21:18:25Z

/pull-request #159948

…text (llvm#156354) fixes llvm#156417 When the relocation section is placed before the relocated section and the relocated section is not defined in the linker script, an error will occur during the linking process. **Issue Cause:** In a.ro, `.rela.text` precedes its relocated `InputSection` `.text`. `addOrphanSections` doesn't handle this scenario. When it processes `.rela.text`, in the called `getOutputSectionName`, `rel->getOutputSection()` is nullptr (input `.text` doesn't yet have a parent output section), leading to an assertion failure. **Solution:** For --emit-relocs and -r, ensure the output section for `.text.foo` is created before the output section for `.rela.text.foo`. --------- Co-authored-by: Fangrui Song <i@maskray.me> (cherry picked from commit 507f394)

llvmbot added lld lld:ELF labels Sep 1, 2025

smithp35 reviewed Sep 2, 2025

View reviewed changes

mykouHW changed the title ~~[lld][ELF] Improve the vulnerability in Orphan Sections initialization~~ [lld][ELF] Fix crash when relocations proceed relocated section Sep 3, 2025

[lld][ELF] Fix crash when relocations proceed relocated section

3cc58a3

Fix the error generated during the linking process when the relocation section is placed before the relocated section and the relocated section is not defined in the linker script.

mykouHW force-pushed the fix-orphan-sections-init branch from fb38a50 to 3cc58a3 Compare September 4, 2025 08:43

smithp35 requested a review from MaskRay September 4, 2025 12:40

smithp35 reviewed Sep 4, 2025

View reviewed changes

smithp35 mentioned this pull request Sep 11, 2025

gcc -fpatchable-function-entry= before the COMDAT fix caused ld.lld to crash #158051

Closed

Merge branch 'main' into fix-orphan-sections-init

2b54668

Better fix for 156417

f1e714b

delete old test

cc33e42

MaskRay changed the title ~~[lld][ELF] Fix crash when relocations proceed relocated section~~ [ELF] -r/--emit-relocs: Fix crash when processing .rela.text before .text Sep 19, 2025

MaskRay approved these changes Sep 19, 2025

View reviewed changes

MaskRay enabled auto-merge (squash) September 19, 2025 03:36

Merge branch 'main' into fix-orphan-sections-init

280cfd7

MaskRay disabled auto-merge September 20, 2025 05:34

MaskRay enabled auto-merge (squash) September 20, 2025 05:34

MaskRay merged commit 507f394 into llvm:main Sep 20, 2025
9 checks passed

MaskRay added this to the LLVM 21.x Release milestone Sep 20, 2025

github-project-automation bot added this to LLVM Release Status Sep 20, 2025

github-project-automation bot moved this to Needs Triage in LLVM Release Status Sep 20, 2025

llvmbot moved this from Needs Triage to Done in LLVM Release Status Sep 20, 2025

[ELF] -r/--emit-relocs: Fix crash when processing .rela.text before .text #156354

[ELF] -r/--emit-relocs: Fix crash when processing .rela.text before .text #156354

Conversation

mykouHW commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 1, 2025

Uh oh!

llvmbot commented Sep 1, 2025

Uh oh!

llvmbot commented Sep 1, 2025

Uh oh!

smithp35 left a comment

Choose a reason for hiding this comment

Uh oh!

smithp35 Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

smithp35 Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

smithp35 Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

smithp35 Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

smithp35 Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

smithp35 Sep 2, 2025

Choose a reason for hiding this comment

Uh oh!

MaskRay Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

smithp35 left a comment

Choose a reason for hiding this comment

Uh oh!

mykouHW commented Sep 18, 2025

Uh oh!

MaskRay commented Sep 18, 2025

Uh oh!

mykouHW commented Sep 19, 2025

Uh oh!

Uh oh!

github-actions bot commented Sep 20, 2025

Uh oh!

MaskRay commented Sep 20, 2025

Uh oh!

llvmbot commented Sep 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mykouHW commented Sep 1, 2025 •

edited

Loading