This is an archive of the discontinued LLVM Phabricator instance.

Rework findUnwindSectionsByPhdr to be more optimal.
ClosedPublic

Authored by saugustine on Mar 6 2020, 4:02 PM.

Details

Summary
  • Executable segment is usually segment 3. Look there for the address first.
  • GNU_EH_FRAME_HEADER segment is usually near the end. Iterate from the end.
  • Exit early if both phdrs have been found.

A subsequent patch will add a cache of found phdrs.

Diff Detail

Event Timeline

saugustine created this revision.Mar 6 2020, 4:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 6 2020, 4:02 PM
saugustine edited the summary of this revision. (Show Details)Mar 9 2020, 11:54 AM
saugustine added reviewers: mstorsjo, miyuki, jgorbe.
jgorbe added inline comments.Mar 9 2020, 3:28 PM
libunwind/src/AddressSpace.hpp
463–464

Is this early check no longer necessary/beneficial?

saugustine marked an inline comment as done.Mar 9 2020, 4:37 PM
saugustine added inline comments.
libunwind/src/AddressSpace.hpp
463–464

I think it is still needed by the reverse-iteration below, which assumes there is one or more phdrs.

This is likely always true--Would dl_iterate_phdr ever pass a phdr-less pinfo? But I don't think dl_iterate_phdr actually guarantees that, at least not in any documentation I have read.

jgorbe added inline comments.Mar 9 2020, 5:12 PM
libunwind/src/AddressSpace.hpp
502–503

The old code only touches dwarf_section_length if both phdrs are found, whereas this patch always resets it to 0 after the loop if it didn't find both. Is this intentional?

saugustine marked 3 inline comments as done.Mar 9 2020, 5:22 PM
saugustine added inline comments.
libunwind/src/AddressSpace.hpp
463–464

Actually, I responded to the wrong question. You are right that this code is still useful. Restored.

502–503

It is. I believe the code is equivalent if dwarf_section_length is always zero on entry (which it is). The new code avoids carrying object_length as a separate variable, and trying to figure out what it means.

saugustine updated this revision to Diff 249255.Mar 9 2020, 5:35 PM

Update for upstream comments

jgorbe accepted this revision.Mar 9 2020, 6:00 PM

Thanks!

This revision is now accepted and ready to land.Mar 9 2020, 6:00 PM
This revision was automatically updated to reflect the committed changes.