This is an archive of the discontinued LLVM Phabricator instance.

Differential D37208

[ELF] - Linkerscript: add test for checking interaction with archive files.
ClosedPublic

Authored by grimar on Aug 28 2017, 3:59 AM.

Details

Reviewers
ruiu
• rafael
Commits
rGdb21e5324a0c: [ELF] - Linkerscript: add test for checking interaction with archive files.
rLLD312004: [ELF] - Linkerscript: add test for checking interaction with archive files.
rL312004: [ELF] - Linkerscript: add test for checking interaction with archive files.
Summary

Imagine we have archive file with symbols foo and bar.
And script that do foo = 1.
In that case correct behavior is not to fetch symbols from archive but
create absolute symbol.
If we have script zed = foo then symbol foo will be fetched from archive.
Currently we have Opt.ReferencedSymbols list of symbols referenced
by script and create them undefined early. That allows archives fetching logic
to work. That is what LLD already do and it seems everything is fine here.

But during writing D37059 I had to add left side of assignments to
Opt.ReferencedSymbols list because wanted to stop optimizing out
these symbols when LTO is involved (LTO also uses this list).
And today I realized that testcase from this patch would have fail if we
would have it before.

So I suggest to add this testcase to be able to catch such situations
during writing other patches.

Diff Detail

Event Timeline

grimar created this revision.Aug 28 2017, 3:59 AM
Herald added subscribers: mehdi_amini, emaste. · View Herald TranscriptAug 28 2017, 3:59 AM
grimar edited the summary of this revision. (Show Details)Aug 28 2017, 3:59 AM
grimar mentioned this in D37059: [ELF] - LTO: do not optimize away symbols accessed from linkerscript..Aug 28 2017, 4:30 AM
ruiu added inline comments.Aug 28 2017, 8:45 AM
test/ELF/linkerscript/assignment-archive.s
3

I believe you can just feed an empty file by doing llvm-mc -filetype=obj -triple=x86_64-pc-linux -o %t.o < /dev/null so that you can merge this file with the one under Innputs directory.

grimar updated this revision to Diff 113052.Aug 29 2017, 3:31 AM
  • Addressed review comment.
ruiu accepted this revision.Aug 29 2017, 7:53 AM

LGTM

This revision is now accepted and ready to land.Aug 29 2017, 7:53 AM
Closed by commit rL312004: [ELF] - Linkerscript: add test for checking interaction with archive files. (authored by grimar). · Explain WhyAug 29 2017, 8:06 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
test/
ELF/
linkerscript/
Inputs/
8 lines
18 lines

Diff 112876

test/ELF/linkerscript/Inputs/assignment-archive.s

.text
.globl foo
foo:
nop
.globl bar
bar:
nop

test/ELF/linkerscript/assignment-archive.s

# REQUIRES: x86
# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/assignment-archive.s -o %ta.o
# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
ruiuUnsubmitted
Not Done
ReplyInline Actions

I believe you can just feed an empty file by doing llvm-mc -filetype=obj -triple=x86_64-pc-linux -o %t.o < /dev/null so that you can merge this file with the one under Innputs directory.

ruiu: I believe you can just feed an empty file by doing `llvm-mc -filetype=obj -triple=x86_64-pc…
# RUN: rm -f %tar.a
# RUN: llvm-ar rcs %tar.a %ta.o
# RUN: echo "SECTIONS { foo = 1; }" > %t1.script
# RUN: ld.lld -o %t1.exe --script %t1.script %tar.a %t.o
# RUN: llvm-readobj -symbols %t1.exe | FileCheck %s
# CHECK-NOT: bar
# CHECK: foo
# CHECK-NOT: bar
# RUN: echo "SECTIONS { zed = foo; }" > %t2.script
# RUN: ld.lld -o %t2.exe --script %t2.script %tar.a %t.o
# RUN: llvm-readobj -symbols %t2.exe | FileCheck %s --check-prefix=SYMS
# SYMS: bar
# SYMS: foo