This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Convert .got.plt section to input section
ClosedPublic

Authored by evgeny777 on Nov 7 2016, 3:24 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

evgeny777 updated this revision to Diff 77025.Nov 7 2016, 3:24 AM
evgeny777 retitled this revision from to [ELF] Convert .got.plt section to input section.
evgeny777 updated this object.
evgeny777 added reviewers: ruiu, rafael.
evgeny777 set the repository for this revision to rL LLVM.
evgeny777 added a project: lld.
evgeny777 added subscribers: grimar, ikudrin, llvm-commits.
grimar added inline comments.Nov 7 2016, 3:29 AM
ELF/SyntheticSections.h
102 ↗(On Diff #77025)

Unsorted.

ELF/Writer.cpp
755 ↗(On Diff #77025)

Seems you do not use it.

evgeny777 updated this revision to Diff 77026.Nov 7 2016, 3:36 AM

Addressed review comments

evgeny777 updated this revision to Diff 77198.Nov 8 2016, 8:16 AM
evgeny777 removed rL LLVM as the repository for this revision.

r286100 was reverted, so I rebased this patch. Changes are:

  • added new input section type (Synthetic) and class SyntheticSection
  • added checks for Synthetic section type in InputSection<ELFT>::writeTo() and InputSection<ELFT>::getSize()
ruiu edited edge metadata.Nov 8 2016, 5:29 PM

Overall looking good.

ELF/InputSection.cpp
86 ↗(On Diff #77198)

Remove else because the last if ends with return.

ELF/InputSection.h
49 ↗(On Diff #77198)

nit: add ',' at end of the last enum.

ELF/OutputSections.h
599 ↗(On Diff #77198)

You are not using InSecAddr. What is this?

ELF/SyntheticSections.h
27 ↗(On Diff #77198)

If all derived classes define this method, use = 0 instead of providing the default empty function.

ELF/Writer.cpp
857 ↗(On Diff #77198)

I want to avoid calling assignOffsets more than once to redo it. It is not only inefficient but also confusing. Could you add this section earlier than this to avoid this function call?

evgeny777 added inline comments.Nov 9 2016, 1:20 AM
ELF/OutputSections.h
599 ↗(On Diff #77198)

See OutputSections.cpp : 817

ELF/Writer.cpp
857 ↗(On Diff #77198)

The problem with .got.plt is that it shouldn't be added if it is empty. Whether or not it is empty is known only after call to scanRelocs(). So if you add it early you'll have to do a fixup procedure later

ruiu accepted this revision.Nov 9 2016, 3:08 PM
ruiu edited edge metadata.

LGTM with my previous comments addressed.

ELF/Writer.cpp
857 ↗(On Diff #77198)

OK. Please add this as a comment.

This revision is now accepted and ready to land.Nov 9 2016, 3:08 PM
This revision was automatically updated to reflect the committed changes.