This is an archive of the discontinued LLVM Phabricator instance.

Differential D29640

[ELF] - Linkerscript - fix handling of OUTPUT_ARCH command.
ClosedPublic

Authored by grimar on Feb 7 2017, 7:13 AM.

Details

Reviewers
ruiu
• rafael
Commits
rG4e01c3e8cdf6: [ELF] - Linkerscript - fix handling of OUTPUT_ARCH command.
rLLD294432: [ELF] - Linkerscript - fix handling of OUTPUT_ARCH command.
rL294432: [ELF] - Linkerscript - fix handling of OUTPUT_ARCH command.
Summary

OUTPUT_ARCH command can contain architecture valuse separated with ":", like:

OUTPUT_ARCH(i386:x86-64)

I think r294006 "Handle numbers followed by ":" in linker scripts." broke that,
because handles ":" as separate token. So now LLD does not accept that.

This trivial patch fixes the issue, now whole expression inside OUTPUT_ARCH is just ignored.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Feb 7 2017, 7:13 AM
grimar updated this revision to Diff 87429.Feb 7 2017, 7:16 AM
  • Removed llvm-readobj, probably no need to check output here.
ruiu added inline comments.Feb 7 2017, 7:29 AM
ELF/LinkerScript.cpp
1252–1255 ↗(On Diff #87429)

This is too specific to your test case. In general, you want to skip until a matching closing parenthesis.

expect("(");
while (!Error && !consume(")")
  skip();
grimar added inline comments.Feb 7 2017, 7:40 AM
ELF/LinkerScript.cpp
1252–1255 ↗(On Diff #87429)

That would conflict with testcase we have I think:

# RUN: echo "OUTPUT_ARCH(x, y)" > %t.script
# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-freebsd %s -o %t1
# RUN: not ld.lld -shared -o %t2 %t1 %t.script
# RUN: llvm-readobj %t2 > /dev/null

Isn't that too permissive ?

ruiu added inline comments.Feb 7 2017, 8:19 AM
ELF/LinkerScript.cpp
1252–1255 ↗(On Diff #87429)

No, since we do not really parse it, tweaking this too hard to adopt existing test cases doesn't make much sense.

grimar updated this revision to Diff 87467.Feb 7 2017, 9:13 AM
grimar edited the summary of this revision. (Show Details)
  • Addressed review comments.
grimar updated this revision to Diff 87468.Feb 7 2017, 9:15 AM
  • Simplified testcase.
ruiu accepted this revision.Feb 7 2017, 1:47 PM

LGTM

ELF/LinkerScript.cpp
1249 ↗(On Diff #87468)

// OUTPUT_ARCH is ignored for now.

This revision is now accepted and ready to land.Feb 7 2017, 1:47 PM
Closed by commit rL294432: [ELF] - Linkerscript - fix handling of OUTPUT_ARCH command. (authored by grimar). · Explain WhyFeb 8 2017, 2:10 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
lld/
trunk/
ELF/
6 lines
test/
ELF/
linkerscript/
7 lines

Diff 87619

lld/trunk/ELF/LinkerScript.cpp

Show First 20 Lines • Show All 1,241 Lines • ▼ Show 20 Linesvoid ScriptParser::readOutput() {
expect("("); expect("(");
StringRef Tok = next(); StringRef Tok = next();
if (Config->OutputFile.empty()) if (Config->OutputFile.empty())
Config->OutputFile = unquote(Tok); Config->OutputFile = unquote(Tok);
expect(")"); expect(")");
} }
void ScriptParser::readOutputArch() { void ScriptParser::readOutputArch() {
// Error checking only for now. // OUTPUT_ARCH is ignored for now.
expect("("); expect("(");
while (!Error && !consume(")"))
skip(); skip();
expect(")");
} }
void ScriptParser::readOutputFormat() { void ScriptParser::readOutputFormat() {
// Error checking only for now. // Error checking only for now.
expect("("); expect("(");
skip(); skip();
StringRef Tok = next(); StringRef Tok = next();
if (Tok == ")") if (Tok == ")")
▲ Show 20 Lines • Show All 854 Lines • Show Last 20 Lines

lld/trunk/test/ELF/linkerscript/outputarch.s

# REQUIRES: x86 # REQUIRES: x86
# RUN: echo "OUTPUT_ARCH(x)" > %t.script # RUN: echo "OUTPUT_ARCH(All data written here is ignored)" > %t.script
# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-freebsd %s -o %t1 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-freebsd %s -o %t1
# RUN: ld.lld -shared -o %t2 %t1 %t.script # RUN: ld.lld -shared -o %t2 %t1 %t.script
# RUN: llvm-readobj %t2 > /dev/null
# RUN: echo "OUTPUT_ARCH(x, y)" > %t.script
# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-freebsd %s -o %t1
# RUN: not ld.lld -shared -o %t2 %t1 %t.script