This is an archive of the discontinued LLVM Phabricator instance.

[lld] Add more complete support for the INCLUDE command.
ClosedPublic

Authored by itessier on Oct 5 2018, 3:09 PM.

Diff Detail

Event Timeline

itessier created this revision.Oct 5 2018, 3:09 PM
grimar added a comment.Oct 8 2018, 3:18 AM

Looks good!
A suggestion about the test is below.

I think you can write the tests as *.test (see lld/test/ELF/linkerscript/implicit-program-header.test for example).
Idea is to make tests containing large linker scripts input easier to read by avoiding using echo for them.

You can write the following line instead of asm code I think:
# RUN: echo 'nop; .section .data,"aw"; .quad 0' | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %t.o

ruiu added inline comments.Oct 8 2018, 11:07 AM
test/ELF/linkerscript/memory-include.s
3

Please construct a linker script file like this

echo foo > %t.inc
echo bar >> %t.inc
echo baz >> %t.inc

the reason being that %t.inc will have newline characters this way. If you use line concatenation, all lines are concatenated into a single line, and if something goes wrong, its error message is hard to read because a line is too long.

ruiu added a comment.Oct 8 2018, 11:08 AM

Or, as George pointed out, you can make linker script as independent files.

itessier updated this revision to Diff 169311.Oct 11 2018, 2:49 PM

I converted all the tests into .test files. I removed the empty file test in memory-include.test since it requires modifying the linker script. I could add a separate linker script in the input data directory for this case, or maybe just simplify the remaining tests by removing the empty file checks?

ruiu accepted this revision.Oct 11 2018, 2:52 PM

LGTM

This revision is now accepted and ready to land.Oct 11 2018, 2:52 PM
grimar accepted this revision.Oct 12 2018, 1:58 AM

LGTM with a minor nit.

test/ELF/linkerscript/section-include.test
11 ↗(On Diff #169311)

Please align this:

# CHECK1:      .data  00000008 0000000000002000 DATA
# CHECK1-NEXT: .data3 00000008 0000000000002008 DATA
18 ↗(On Diff #169311)

And this.

itessier updated this revision to Diff 169447.Oct 12 2018, 10:01 AM

Done, aligned the check lines.

Can somebody submit this for me? I don't have SVN access.

This revision was automatically updated to reflect the committed changes.