Page MenuHomePhabricator

[LLD][ELF] Use Synthetic Sections for Thunks
ClosedPublic

Authored by peter.smith on Jan 25 2017, 8:21 AM.

Details

Summary

Thunks are now implemented by redirecting a relocation to the symbol S, to a symbol TS defined in a Thunk. The Thunk will transfer control to S. This has the following implications:

  • All the side-effects of Thunks happen within createThunks()
  • Thunks are no longer stored in InputSections and Symbols no longer need to hold a pointer to a Thunk
  • The synthetic Thunk sections need to be merged into OutputSections

This implementation is almost a direct conversion of the existing Thunks with the following exceptions:

  • Mips LA25 Thunks are placed before the InputSection that defines the symbol that needs a Thunk.
  • All ARM Thunks are placed at the end of the OutputSection of the first caller to the Thunk.

Range extension Thunks are not supported yet so it is optimistically assumed that all Thunks can be reused.

Design decisions:
I've used the synthetic section as a container for multiple thunks. This minimises the number of insertions into the InputSections, makes it easier to insert multiple Mips LA25 thunks before the target InputSection, and will make it easier to support range-extension thunks as Gold and ld.bfd implement them (thunk pools placed at regular intervals). Alternatives would be a single Thunk per SyntheticSection or derive each Thunk from SyntheticSection.

I've put the Mips LA25 Thunks ahead of the Target Input Section as Simon has mentioned that this will enable an optimization to allow control to drop through into the next section without a jump instruction. I have not implemented that optimization.

As the Thunks define symbols they need to be ahead of the code that adds symbols into the symbol table so I've moved the createThunks call to before the code to finalize the SymTab.

There isn't a standard naming convention for Thunks, every ARM linker does it differently, I've somewhat arbitrarily chosen names that are close to the Thunk class names, but do not have a strong opinion what we should use. I find it helpful to have the symbol for the Thunk displayed in disassembly.

I expect that supporting range extension thunks will need quite a bit of change to the code in createThunks(). The current implementation is only good enough to handle Mips LA25 Thunks that need precise insertion, and ARM Thunks that can go at the end of the OutputSection.

Diff Detail

Repository
rL LLVM

Event Timeline

peter.smith created this revision.Jan 25 2017, 8:21 AM
peter.smith retitled this revision from [LLD][ELF] Use Thunks for Synthetic Sections to [LLD][ELF] Use Synthetic Sections for Thunks.Jan 25 2017, 8:22 AM
ruiu added inline comments.Jan 25 2017, 11:33 AM
ELF/Relocations.cpp
809 ↗(On Diff #85754)

Capitalize all local variables in this function (i.e. thunkCmp, a, b, tmp and mergeCmp).

823–826 ↗(On Diff #85754)

Could you describe not only what it is doing but why we want that behavior?

831 ↗(On Diff #85754)

Probably move assignment is more C++11-ish than swap.

863 ↗(On Diff #85754)

Flip the condition to use continue.

872–874 ↗(On Diff #85754)

Can you flip the conditions of these ifs to reduce indentation?

ELF/Symbols.h
419–420 ↗(On Diff #85754)

clang-format-diff?

ELF/SyntheticSections.h
708 ↗(On Diff #85754)

nit: add a blank line.

ELF/Thunks.cpp
264–266 ↗(On Diff #85754)

Remove else after return

Thanks very much for the comments.

I've updated the diff to apply the changes. I wasn't able to find a way to reduce the indentation createThunks by inverting some the if statements.
What I've done instead is extracted the main operations into lambdas. This hopefully makes the main loop processing relocations easier to follow.

ruiu accepted this revision.Jan 26 2017, 11:59 AM

LGTM

ELF/Relocations.cpp
861–866 ↗(On Diff #85886)

This code looks up ThunkedSymbols twice with the same key. I wonder if you can avoid that by doing something like this with a reference.

Thunk<ELFT> *&T = ThunkedSymbols[&Body];
if (T)
  return std::make_pair(T, false);
T = addThunk<ELFT>(Type, Body);
return std::make_pair(T, true);
ELF/Thunks.h
47 ↗(On Diff #85886)

nit: Can you make this a member variable set by ctor to avoid the cost of virtual function call?

This revision is now accepted and ready to land.Jan 26 2017, 11:59 AM
This revision was automatically updated to reflect the committed changes.

Thanks for the suggestions. Now committed r293283. I'll now go about thinking about how best to add range extension thunks.