-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PAL_VirtualUnwindOutOfProc for MacOS #39213
Conversation
Add MacOS compact unwind section parsing (__unwind_info) and stepping: SearchCompactEncodingSection, SearchDwarfSection, GetProcInfo, StepWithCompactEncoding*. Add back some (from a previous commit) of the dwarf unwind info parsing functions to use for the dwarf unwind info (__eh_frame section): ReadValue*, ReadULEB128, ReadSLEB128, ReadEncodedPointer, ParseCie, ExtractFde, ExtractProcInfoFromFde. These functions were used and fairly well tested in 3.x. Build libunwind8 source on MacOS for those dwarf unwind info cases.
Tagging subscribers to this area: @tommcdon |
@janvorli, @sdmaclea as far as the changes I made to build libunwind8 for MacOS, I hope we can come up with a way to add them without requiring them to be pushed to the libunwind8 repo. I didn't make any changes to the core files only changed CMakeLists.txt and adding some subdirectories (which could be moved out of the libunwind directory). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not inherently obvious why we wouldn't want to upstream this. Perhaps because MacOS libunwind support is always installed. So there is no need for MacOS support in the libunwind project.
I have mixed feelings about the directory layout. I would wait for @janvorli feedback on the directory style.
As far as push these changes to the libunwind repo: it would require a lot of work because most of the time the dwarf unwind isn't used so libunwind isn't used. It is the compact/compressed unwind info that is used at lot more and requires the compact "step" code I wrote in remote-unwind.cpp. So most of the new code would have to be ported to the libunwind8 coding conventions, etc. In other words, without the compact format parsing/stepping just building it on macOS doesn't have any benefit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo the nit.
The methoddesc's enum memory region code needed add more of the code version manager's data to the coredump.
Hello @mikem8361! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Add MacOS compact unwind section parsing (__unwind_info) and stepping: SearchCompactEncodingSection, SearchDwarfSection, GetProcInfo, StepWithCompactEncoding*.
Add back some (from a previous commit) of the dwarf unwind info parsing functions to use for the dwarf unwind info (__eh_frame section): ReadValue*, ReadULEB128, ReadSLEB128, ReadEncodedPointer, ParseCie, ExtractFde, ExtractProcInfoFromFde. These functions were used and fairly well tested in 3.x.
Build libunwind8 source on MacOS for those dwarf unwind info cases.