Page MenuHomePhabricator

[LLD][ELF] Use Synthetic Sections for Thunks (try 2)
ClosedPublic

Authored by peter.smith on Jan 31 2017, 6:43 AM.

Details

Summary

This is D29129 with the Windows debug build problems fixed.

The MSVC debug implementation of std::merge enforces the preconditions required by the C++ standard. This includes checking that both ranges are sortable by the predicate and that if (A < B) then !(B > A). D29129 the comparison routine did not conform to the requirements of the standard. This change fixes the predicate.

The changes made are all related to the comparison routine:
SyntheticSections:

  • add getTargetInputSection to ThunkSection
  • correct section flags in constructor. ThunkSections was missing SHF_EXECINSTR

Relocations:

  • Set the OutSecOff of the per-OutputSection ThunkSection to be the limit of the executable sections, previously we set the OutSecOff to size, and checked in the predicate whether the Thunk and InputSection were executable.
  • Rewrite the MergeComp predicate to use only OutSecOff and as a tie-breaker for Mips LA25 whether the next section is our Thunks Target.

I've checked that this builds and runs the tests on MSVC 2015. Sending for review as it is more than just a simple syntax fix.

Diff Detail

Repository
rL LLVM

Event Timeline

peter.smith created this revision.Jan 31 2017, 6:43 AM
ruiu accepted this revision.Jan 31 2017, 11:58 AM

LGTM

ELF/Relocations.cpp
822–824 ↗(On Diff #86420)

What MSVC does is not wrong, so mentioning about MSVC as a special case would probably confuse readers. The thing is that the function needs to be a strict weak ordering (some standard libraries actually check for that in debug mode.)

827–834 ↗(On Diff #86420)

Can't getTargetInputSection point only to a non-thunk input section, can it? I wonder if you can write something like this.

if (if (A->OutSecOff == B->OutSecOff)
  if (auto *TA = dyn_cast<ThunkSection<ELFT>>(A))
    if (TA->getTargetInputSection() == B)
      return true;
This revision is now accepted and ready to land.Jan 31 2017, 11:58 AM
This revision was automatically updated to reflect the committed changes.