Page MenuHomePhabricator

Implement --just-symbols.
ClosedPublic

Authored by ruiu on Oct 26 2017, 3:16 PM.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

ruiu created this revision.Oct 26 2017, 3:16 PM
mcgrathr edited edge metadata.Oct 26 2017, 5:23 PM

This doesn't implement the -R switch name, which is the traditional interface from time immemorial. I can use --just-symbols, but you really should handle -R too.

Just copying symbol values will suffice for my case, but it's not really the right semantics. The rest of the ElfNN_Sym fields matter too.

This crashes on my case. I don't know what a trivial test case that would make it crash might look like, but I'll send you my actual case separately.

lld/ELF/Driver.cpp
1065 ↗(On Diff #120497)

--just-symbols (double dash), better known in traditional Unix as -R.

1068 ↗(On Diff #120497)

s/programs/program/

lld/ELF/InputFiles.cpp
1053 ↗(On Diff #120497)

--just-symbols (double dash)

1055 ↗(On Diff #120497)

The canonical semantics are to copy all the other fields as well, not just st_value.
Only st_shndx is replaced with SHN_ABS. The others are preserved (size, visibility, type, etc.).

lld/ELF/InputFiles.h
62 ↗(On Diff #120497)

--just-symbols

lld/ELF/Options.td
150 ↗(On Diff #120497)

--just-symbols is the GNU long option for this, but -R is the original switch.
Since -R has been overloaded for --rpath, the GNU implementations examine the argument file for -R and treat it as --rpath if it's a directory and as --just-symbols if not.

mcgrathr added inline comments.Oct 26 2017, 5:36 PM
lld/ELF/SymbolTable.cpp
145 ↗(On Diff #120497)

Stray debugging output.

phosek added a subscriber: phosek.Nov 2 2017, 1:53 PM
ruiu marked 5 inline comments as done.Nov 30 2017, 9:49 PM
ruiu added inline comments.
lld/ELF/InputFiles.cpp
1055 ↗(On Diff #120497)

Yeah that's true, but I wonder if real programs depend on it.

ruiu updated this revision to Diff 125075.Nov 30 2017, 9:49 PM
  • address review comments
grimar added a subscriber: grimar.Dec 1 2017, 11:57 PM
PkmX added a subscriber: PkmX.Dec 4 2017, 10:32 PM
pshung added a subscriber: pshung.Dec 10 2017, 9:18 PM
PkmX added a comment.Dec 10 2017, 11:40 PM

If the only input file is specified as the argument of --just-symbol, lld will error out (ld.bfd accepts this):

$ riscv64-unknown-linux-gnu-ld.lld -r --just-symbols foo.o -o bar.o
riscv64-unknown-linux-gnu-ld.lld: error: no input files
riscv64-unknown-linux-gnu-ld.lld: error: target emulation unknown: -m or at least one .o file required

I'd suggest adding a separate kind of InputFile whose symbols only gets added to the symbol table as absolute (but don't add its sections). I have a patch that implements this if you'd like to review it.

In D39348#950733, @PkmX wrote:

If the only input file is specified as the argument of --just-symbol, lld will error out (ld.bfd accepts this):

$ riscv64-unknown-linux-gnu-ld.lld -r --just-symbols foo.o -o bar.o
riscv64-unknown-linux-gnu-ld.lld: error: no input files
riscv64-unknown-linux-gnu-ld.lld: error: target emulation unknown: -m or at least one .o file required
 

I'd suggest adding a separate kind of InputFile whose symbols only gets added to the symbol table as absolute (but don't add its sections). I have a patch that implements this if you'd like to review it.

What is your use case to specify only single --just-symbol file ?

PkmX added a comment.Dec 11 2017, 7:12 AM

What is your use case to specify only single --just-symbol file ?

It can be used to produce an object file containing the offset of symbols in vdso:
https://github.com/riscv/riscv-linux/blob/ae64f9bd1d3621b5e60d7363bc20afb46aede215/arch/riscv/kernel/vdso/Makefile#L41

ruiu added a comment.Dec 11 2017, 7:46 PM

PkmX,

If you already have a patch, please send it to me and llvm-commits.

ruiu updated this revision to Diff 127195.Dec 15 2017, 3:04 PM
  • Copy symbol type, size, binding, etc. when creating an absolute symbol.
mcgrathr accepted this revision.Dec 18 2017, 3:42 PM

Works for me. D41277 works just as well. I don't know how you plan to reconcile the two.

This revision is now accepted and ready to land.Dec 18 2017, 3:42 PM

ping. Will this or D41277 land?

ruiu updated this revision to Diff 129748.Jan 12 2018, 10:22 PM
  • rebased
ruiu added a comment.Jan 12 2018, 10:25 PM

I tried to create file objects for --just-symbols so that the linker command that just specifies --just-symbols without any real object file works fine.

https://reviews.llvm.org/D42025

But I really like this one over that one. The other one isn't necessarily bad, but it looks like it is a bit too complicated for a use case that I doubt is realistic. So my preference is to submit this one.

Closed by commit rLLD326835: Implement --just-symbols. (authored by ruiu, committed by ). · Explain WhyMar 6 2018, 1:27 PM
This revision was automatically updated to reflect the committed changes.