This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Fix for PR28976 - Corrupted section contents when using linker scripts
ClosedPublic

Authored by grimar on Aug 18 2016, 5:26 AM.

Details

Summary

This is alternative solution to fix PR28976.

Problem is that in scanRelocs, we computed relocation offset too early for case when linkerscript was used.
Patch fixes the issue delaying the calculation.

Testcase is provided.

Diff Detail

Event Timeline

grimar updated this revision to Diff 68515.Aug 18 2016, 5:26 AM
grimar retitled this revision from to [ELF] - Fix for PR28976 - Corrupted section contents when using linker scripts.
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, davide, grimar and 3 others.
emaste added a subscriber: emaste.Aug 18 2016, 5:45 AM
Wallbraker accepted this revision.Aug 18 2016, 6:51 AM
Wallbraker added a reviewer: Wallbraker.

This works for my test case as well, nice work! :D

This revision is now accepted and ready to land.Aug 18 2016, 6:51 AM
ruiu added inline comments.Aug 18 2016, 1:25 PM
ELF/Relocations.cpp
559

This seems a cool fix, but is it safe? Specifically, my concern is on this line.

We used to call getOffset only in the following path. If PieceI != PieceE, getOffset was not called. Now you call getOffset on both paths. Didn't change the behavior?

grimar added inline comments.Aug 19 2016, 2:10 AM
ELF/Relocations.cpp
559

Yep, I think it is safe. See:

If PieceI != PieceE that means C is EhInputSection, right ? (see line 528-529 above)

if (auto *Eh = dyn_cast<EhInputSection<ELFT>>(&C))
  Pieces = Eh->Pieces;

and getOffset() for EHFrame always returns just an Offset given:

template <class ELFT>
typename ELFT::uint InputSectionBase<ELFT>::getOffset(uintX_t Offset) const {
  switch (SectionKind) {
...
  case EHFrame:
    // The file crtbeginT.o has relocations pointing to the start of an empty
    // .eh_frame that is known to be the first in the link. It does that to
    // identify the start of the output .eh_frame.
    return Offset;

So in fact I call getOffset on both paths but that does not change anything I believe.

ruiu accepted this revision.Aug 19 2016, 8:44 AM
ruiu edited edge metadata.

Makes sense. LGTM.

This revision was automatically updated to reflect the committed changes.