This is an archive of the discontinued LLVM Phabricator instance.

[dsymutil] Copy eh_frame content into the dSYM companion file.
ClosedPublic

Authored by JDevlieghere on Jan 11 2021, 4:17 PM.

Details

Summary

Copy over the __eh_frame from the binary into the dSYM. This helps kernel developers that are working with only dSYMs (i.e. no binaries) when debugging a core file. This only kicks in when the __eh_frame exists in the linked binary. Most of the time ld64 will remove the section in favor of compact unwind info. When it is emitted, it's generally small enough and should not bloat the dSYM.

rdar://69774935

Diff Detail

Event Timeline

JDevlieghere created this revision.Jan 11 2021, 4:17 PM
JDevlieghere requested review of this revision.Jan 11 2021, 4:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 11 2021, 4:17 PM

I think this looks reasonable!

llvm/test/tools/dsymutil/X86/eh_frame.test
2

This test has an odd combination of syntax :-)
Why C-style comments here ...

19

... and not here?

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.
llvm/test/tools/dsymutil/X86/eh_frame.test
2

I had already pasted the source code when I added this line, I think it tricked my brain into writing comment markers subconsciously...

friss added a comment.Jan 11 2021, 5:14 PM

Couple comments/questions, but this generally looks good

llvm/test/tools/dsymutil/X86/eh_frame.test
24

Can we add a check that the generated DWARF segment is well-formed?

llvm/tools/dsymutil/MachOUtils.cpp
294–295

Is reloff set to anything in the original file? I think it's supposed to be the offset to the list of relocations for the section, but eh_frame shouldn't have any relocations at this point. Anyway, I think it's better to force it to 0 as well as nreloc.

JDevlieghere marked 2 inline comments as done.

Address code review feedback

llvm/tools/dsymutil/MachOUtils.cpp
294–295

Yeah that was a typo (copy paste-o?)

jasonmolenda added inline comments.Jan 11 2021, 9:31 PM
llvm/test/tools/dsymutil/X86/eh_frame.test
26

These eh_frame bytes are architecture-specific (and likely codegen-specific if the compiler ever changes how many bytes are emitted). Is it possible to use dwarfdump on the .dSYM binary and search for the function name in the eh_frame dump, and assume that the instructions that it has are correct?

jasonmolenda added inline comments.Jan 11 2021, 9:35 PM
llvm/test/tools/dsymutil/X86/eh_frame.test
26

Or confirm that the eh_frame bytes from the executable are the same as the dSYM's.

JDevlieghere marked 2 inline comments as done.Jan 12 2021, 8:52 AM
JDevlieghere added inline comments.
llvm/test/tools/dsymutil/X86/eh_frame.test
26

The dsymutil tests use checked-in binaries so the test is resilient in that regard. The RUN line on 21 checks the input binary as a sanity check (in case someone decides to regenerate the binary).

friss accepted this revision.Jan 12 2021, 5:46 PM

LGTM, thanks!

This revision is now accepted and ready to land.Jan 12 2021, 5:46 PM
This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.