Page MenuHomePhabricator

[ELF] Allow references to reserved symbols in linker scripts
ClosedPublic

Authored by phosek on Mar 20 2017, 12:05 PM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

phosek created this revision.Mar 20 2017, 12:05 PM
ruiu edited edge metadata.Mar 21 2017, 2:46 PM

It's not obvious what you are fixing with this patch. Can you describe a little?

In D31147#706917, @ruiu wrote:

It's not obvious what you are fixing with this patch. Can you describe a little?

Yes, we'd like to be able to reference reserved symbols from input linker scripts, e.g. PROVIDE_HIDDEN(newsym = __ehdr_start) (where __ehdr_start could be any other reserved symbol like __edata_start__). This currently fails with __ehdr_start being reported as undefined symbol. This is something that both GNU linker handle fine. One use case for this feature is an alternative solution to D26133.

ruiu added inline comments.Mar 22 2017, 1:19 PM
ELF/LinkerScript.cpp
1884 ↗(On Diff #92369)

You want to add this to Config->Undefined instead of your own new array.

Please add a comment saying that some special symbols (such as __ehdr_start) are defined only when there are undefined symbols for them, so we add undefined symbols to trigger that logic.

phosek added inline comments.Mar 22 2017, 3:33 PM
ELF/LinkerScript.cpp
1884 ↗(On Diff #92369)

I tried as the initial solution, the problem is that the logic for processing Config->Undefined currently only handles Lazy files; it checks if the symbol is defined in any of the input Lazy files and if so it loads it in. However, the reserved symbols are not defined in any input file.

I think the best solution would be to introduce a new InputFile subclass for representing input linker scripts (which is what they really are). It'd require some changes, e.g. we'd have to stop using a single global LinkerScript instance for all linker scripts, instead each input script will have to have its own. I can try to implement that solution if that would be fine with you?

ruiu added inline comments.Mar 22 2017, 3:42 PM
ELF/LinkerScript.cpp
1884 ↗(On Diff #92369)

That sounds too much. I don't think linker scripts are input files but in my view they are rather extended command line options. I prefer this patch over the proposed solution.

By the way, instead of defining LinkerScript<ELFT>::addUndefined, you can do something like this in the driver.

for (StringRef Sym : Script->Opt.UndefinedSymbols)
  Symtab.addUndefined(Sym);
phosek updated this revision to Diff 92753.Mar 22 2017, 6:17 PM
phosek updated this revision to Diff 92754.
phosek marked 3 inline comments as done.
ruiu accepted this revision.Mar 22 2017, 6:25 PM

LGTM

This revision is now accepted and ready to land.Mar 22 2017, 6:25 PM
This revision was automatically updated to reflect the committed changes.