Enable stack trace symbolization for OSS builds #9687
+13
−504
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D88499314. (Because this pull request was imported automatically, there will not be any future comments.) |
mszabo-wikia
force-pushed
the
enable-stack-trace-symbolization
branch
from
9bd3f7c to
180af97
Compare
Contributor
|
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing. |
mszabo-wikia
force-pushed
the
enable-stack-trace-symbolization
branch
from
180af97 to
6063823
Compare
Contributor
|
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing. |
Historically, HHVM statically linked the system libbfd to power stack trace symbolization in its crash reports and in perf map files. This was disabled for OSS in D2137703[1] because of the wildly differing libbfd ABI across target systems. D3742004[2] and D3855027[3] then brought in folly::Symbolizer for both use cases, but this remained gated behind a Meta-only define. folly::Symbolizer has been stable in upstream folly for a while now, so let's enable it unconditionally and deshim the experimental/ header. This also allows removing the dead libbfd integration. For this to work, the folly build must be built with and able to find libunwind, which isn't a given when building against libc++ and LLVM libunwind on Ubuntu as it installs includes into a subdirectory, so provide an additional hint for this case. This allows us to finally have a proper stacktrace rather than raw memory addresses in crash reports, such as: ``` Core dumped: Segmentation fault Stack trace in /tmp/stacktrace.902095.log Host: hhvm-jammy-vm ProcessID: 902095 ThreadID: 281473658339392 ThreadPID: 902095 Name: /usr/local/bin/hhvm CmdLine: hhvm ../hhvm/hphp/test/quick/dv.php Type: Segmentation fault Runtime: hhvm Version: heads/7.70.0-slack-0-g7be53eff8d22faa0383a128696fae83b224aedd5 DebuggerCount: 0 Arguments: ../hhvm/hphp/test/quick/dv.php ThreadType: CLI -------------------------------Treadmill Information---------------------------- Now: 235093382994348 OldestStartTime: 235093373066177 InflightRequestsSize: 1 Active Requests: 281473658339392 1 235093373066177 (age 9ms) (timeout 0s) CLISession OLDEST ------------------------------------------------------------------------------ CPP Stacktrace: # 0 HPHP::jit::FixupMap::processFixupForVMFrame(HPHP::jit::VMFrame) # 1 HPHP::jit::FixupMap::fixupWork(HPHP::ActRec*, bool) # 2 HPHP::jit::detail::syncVMRegsWork(bool) # 3 HPHP::createBacktrace(HPHP::BacktraceArgs const&) # 4 HPHP::f_debug_backtrace(long, long) # 5 HPHP::throwable_init(HPHP::ObjectData*) # 6 HPHP::ObjectData* HPHP::ObjectData::newInstance<false>(HPHP::Class*) # 7 HPHP::SystemLib::AllocRuntimeExceptionObject(HPHP::Variant const&) # 8 HPHP::SystemLib::throwRuntimeExceptionObject(HPHP::Variant const&) # 9 HPHP::throwMissingArgument(HPHP::Func const*, int) PHP Stacktrace: #0 main() called at [/home/mszabo/hhvm/hphp/test/quick/dv.php:28] #1 main_entry() ``` [1] facebook@a66dd13 [2] facebook@c11ad4e [3] facebook@0e082c4
mszabo-wikia
force-pushed
the
enable-stack-trace-symbolization
branch
from
6063823 to
29831e4
Compare
Contributor
|
@mszabo-wikia has updated the pull request. You must reimport the pull request before landing. |
|
👋 Hi, I'm an automated AI code review bot. I ran some checks on this PR and found 2 points that might be worth attention (could be false positives, please use your judgment):
If you find these suggestions disruptive, you can reply "stop" , and I'll automatically skip this repository in the future. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Historically, HHVM statically linked the system libbfd to power stack trace symbolization in its crash reports and in perf map files. This was disabled for OSS in D2137703[1] because of the wildly differing libbfd ABI across target systems. D3742004[2] and D3855027[3] then brought in folly::Symbolizer for both use cases, but this remained gated behind a Meta-only define.
folly::Symbolizer has been stable in upstream folly for a while now, so let's enable it unconditionally and deshim the experimental/ header. This also allows removing the dead libbfd integration. For this to work, the folly build must be built with and able to find libunwind, which isn't a given when building against libc++ and LLVM libunwind on Ubuntu as it installs includes into a subdirectory, so provide an additional hint for this case.
This allows us to finally have a proper stacktrace rather than raw memory addresses in crash reports, such as:
[1] a66dd13
[2] c11ad4e
[3] 0e082c4