This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Move the addition of synthetics from addPredefinedSections()
ClosedPublic

Authored by grimar on Jan 11 2017, 6:16 AM.

Details

Summary

These were 3 last synthetics that were added in addPredefinedSections() instead
of createSyntheticSections(). Now it is possible to move addition to correct common place.

Also patch fixes testcase which discards .shstrtab, by restricting doing that.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 83968.Jan 11 2017, 6:16 AM
grimar retitled this revision from to [ELF] - Move the addition of synthetics from addPredefinedSections().
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar, evgeny777.
ruiu accepted this revision.Jan 11 2017, 10:26 AM
ruiu edited edge metadata.

LGTM

ELF/Writer.cpp
1598–1600 ↗(On Diff #83968)

I'd think this is slightly more readable.

if (In<ELFT>::ShStrTab->OutSec)
  EHdr->e_shstrndx = In<ELFT>::ShStrTab->OutSec->SectionIndex;
This revision is now accepted and ready to land.Jan 11 2017, 10:26 AM
grimar added inline comments.Jan 12 2017, 12:39 AM
ELF/Writer.cpp
1598–1600 ↗(On Diff #83968)

May be. At least it shorter. Though this form does not explicitly assign SHN_UNDEF, it is a luck that SHN_UNDEF == 0.
I usually prefer not to assume values for named constants, even for such cases.

I'll use your suggestion to land this patch after resolving D28560 dependency as its seems a matter of taste in this place.

grimar updated this revision to Diff 84277.Jan 13 2017, 3:52 AM
grimar edited edge metadata.
  • Discarding .shstrtab is restricted.
grimar requested a review of this revision.Jan 13 2017, 3:53 AM
grimar edited edge metadata.
grimar updated this object.Jan 13 2017, 8:28 AM
grimar edited edge metadata.
This revision was automatically updated to reflect the committed changes.