This is an archive of the discontinued LLVM Phabricator instance.

Differential D43069

[ELF] - Report error if removed empty output section declaration used undefined symbols.
ClosedPublic

Authored by grimar on Feb 8 2018, 5:45 AM.

Details

Reviewers
ruiu
• rafael
Commits
rG3cdf0d969a81: [ELF] - Report error if removed empty output section declaration used undefined…
rL325875: [ELF] - Report error if removed empty output section declaration used undefined…
rLLD325875: [ELF] - Report error if removed empty output section declaration used undefined…
Summary

This is for fixing PR36297.

Issue itself is that if we have SECTIONS { .bar (a+b) : { *(.stub) } };
script and no section .stub, when LLD will remove .bar, but
produce output with undefined symbols a and b.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

grimar created this revision.Feb 8 2018, 5:45 AM
Herald added a subscriber: emaste. · View Herald TranscriptFeb 8 2018, 5:45 AM
grimar edited the summary of this revision. (Show Details)Feb 8 2018, 5:45 AM
grimar planned changes to this revision.Feb 8 2018, 7:11 AM

Will update soon.

grimar updated this revision to Diff 133420.Feb 8 2018, 7:22 AM
grimar edited the summary of this revision. (Show Details)
  • Simplified implementation, added missing testcases.
grimar retitled this revision from [ELF] - Report error if removed empty output section used undefined symbols. to [ELF] - Report error if removed empty output section declaration used undefined symbols..Feb 8 2018, 7:36 AM
ruiu added a comment.Feb 8 2018, 8:25 PM

This patch seems somewhat irrelevant to me because the issue you found is not actually related to the issue you described in the patch description. The issue you found is that we do not recognize "(INFO)" as a special tag, and that's different from ignoring symbols in dead sections.

grimar added a comment.EditedFeb 9 2018, 6:19 AM
In D43069#1002870, @ruiu wrote:

This patch seems somewhat irrelevant to me because the issue you found is not actually related to the issue you described in the patch description. The issue you found is that we do not recognize "(INFO)" as a special tag, and that's different from ignoring symbols in dead sections.

I described how I found it. Patch that adds support for "INFO" and all its aliases is D43071, but still if you have undefined symbol in expression,
it should not be emited in output as undefined. And that is what this patch fixes. I'll remove part about "INFO" from description to avoid possible confusion.

Herald added a subscriber: arichardson. · View Herald TranscriptFeb 9 2018, 6:19 AM
grimar edited the summary of this revision. (Show Details)Feb 9 2018, 6:20 AM
grimar planned changes to this revision.Feb 9 2018, 7:09 AM
grimar updated this revision to Diff 133625.Feb 9 2018, 8:29 AM
  • Reimplemented to keep output sections with custom address/align expressions alive in isAllSectionDescription.
grimar updated this revision to Diff 134856.Feb 18 2018, 11:43 PM
  • Comment update. NFC.
grimar mentioned this in D43571: [ELF] - Do not remove empty output sections that are explicitly assigned to phdr in script..Feb 21 2018, 5:52 AM
This revision was not accepted when it landed; it landed in state Needs Review.Feb 23 2018, 2:17 AM
Closed by commit rLLD325875: [ELF] - Report error if removed empty output section declaration used undefined… (authored by grimar). · Explain Why
This revision was automatically updated to reflect the committed changes.
grimar mentioned this in rL325887: [ELF] - Do not remove empty output sections that are explicitly assigned to….Feb 23 2018, 2:55 AM
grimar mentioned this in rLLD325887: [ELF] - Do not remove empty output sections that are explicitly assigned to….

Revision Contents

PathSize
ELF/
7 lines
test/
ELF/
linkerscript/
15 lines

Diff 135604

ELF/LinkerScript.cpp

Show First 20 Lines • Show All 761 Lines • ▼ Show 20 Linesvoid LinkerScript::removeEmptyCommands() {
llvm::erase_if(SectionCommands, [&](BaseCommand *Base) { llvm::erase_if(SectionCommands, [&](BaseCommand *Base) {
if (auto *Sec = dyn_cast<OutputSection>(Base)) if (auto *Sec = dyn_cast<OutputSection>(Base))
return !Sec->Live; return !Sec->Live;
return false; return false;
}); });
} }
static bool isAllSectionDescription(const OutputSection &Cmd) { static bool isAllSectionDescription(const OutputSection &Cmd) {
// We do not want to remove sections that have custom address or align
// expressions set even if them are empty. We keep them because we
// want to be sure that any expressions can be evaluated and report
// an error otherwise.
if (Cmd.AddrExpr || Cmd.AlignExpr || Cmd.LMAExpr)
return false;
for (BaseCommand *Base : Cmd.SectionCommands) for (BaseCommand *Base : Cmd.SectionCommands)
if (!isa<InputSectionDescription>(*Base)) if (!isa<InputSectionDescription>(*Base))
return false; return false;
return true; return true;
} }
void LinkerScript::adjustSectionsBeforeSorting() { void LinkerScript::adjustSectionsBeforeSorting() {
// If the output section contains only symbol assignments, create a // If the output section contains only symbol assignments, create a
▲ Show 20 Lines • Show All 277 Lines • Show Last 20 Lines

test/ELF/linkerscript/address-expr-symbols.s

# REQUIRES: x86
# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
# RUN: echo "SECTIONS { .bar (foo) : { } };" > %t.script
# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
# CHECK: symbol not found: foo
# RUN: echo "SECTIONS { .bar : AT(foo) { } };" > %t.script
# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
# RUN: echo "SECTIONS { .bar : ALIGN(foo) { } };" > %t.script
# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s
# RUN: echo "SECTIONS { .bar : SUBALIGN(foo) { } };" > %t.script
# RUN: not ld.lld -o %t --script %t.script %t.o 2>&1 | FileCheck %s