Page MenuHomePhabricator

ELF: Perform per-section .ARM.exidx processing during combineEhFrameSections(). NFCI.
ClosedPublic

Authored by pcc on Mar 29 2019, 8:46 PM.

Details

Summary

And rename the function to combineEhSections(). This makes the processing
of .ARM.exidx even more similar to .eh_frame and means that we can avoid an
additional loop over InputSections.

Diff Detail

Repository
rL LLVM

Event Timeline

pcc created this revision.Mar 29 2019, 8:46 PM
grimar added a comment.EditedMar 30 2019, 10:34 AM

I like this approach and see no major issues with this patch probably.

(The way combineEhSections works with sections is not very consistent (e.g. the behavior of 'addSection' calls),
but I did not find a nice way to improve it atm, we can change it later I think)

lld/ELF/Writer.cpp
472 ↗(On Diff #192959)

I think it worth to add a comment about what this method does now.
(or perhaps it would be better to add comments to combineEhSections body though)

I'm happy to make the change. One minor reservation is that from combineEHSections to ARMExidxSections::finalize() the section will report its size as 0 and each individual ARMExidx InputSection won't have a parent. To the best of my knowledge this doesn't matter right now as nothing depends on these being set during that time. It may give someone a surprise in the future though.

ruiu accepted this revision.Apr 1 2019, 12:19 AM

LGTM

This revision is now accepted and ready to land.Apr 1 2019, 12:19 AM
pcc added a comment.Apr 1 2019, 10:42 AM

I'm happy to make the change. One minor reservation is that from combineEHSections to ARMExidxSections::finalize() the section will report its size as 0 and each individual ARMExidx InputSection won't have a parent. To the best of my knowledge this doesn't matter right now as nothing depends on these being set during that time. It may give someone a surprise in the future though.

It's normal for synthetic section sizes to not be fixed until the call to finalizeContents(), e.g. EhFrameSection: http://llvm-cs.pcc.me.uk/tools/lld/ELF/SyntheticSections.cpp#505

I believe that the sections already don't have parents until the call to finalizeContents() because the parents are created during processSectionCommands/addOrphanSections.

This revision was automatically updated to reflect the committed changes.
pcc marked an inline comment as done.