Skip to content
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

Better stacktrace decoding #2061

Merged
merged 11 commits into from
Jan 13, 2019

Conversation

eggrobin
Copy link
Member

@eggrobin eggrobin commented Jan 6, 2019

The stacktrace provided in #2056 notably differs from the one we recover from the logs in that it includes inline frames.
In order to get those, we need to directly use dbghelp instead of going through dbh. This also solves a lot of clunkiness surrounding base addresses, as well as the issue that dbh did not appear to work with non-ASCII paths (what year is this).
Further, GitHub now automatically renders code snippets, so use that by default instead of [`file:line`](link) markdown.

In order to have more context in the snippet, make the snippet range from the start of the symbol to the line corresponding to the stack frame. Make the trace most-recent-call-last so that the sequence of snippets reads monotonically.

Note that since our stack does not include the line with the failing check, we miss any inline frames between the check and the next non-inline frame, and we do not get a snippet for the check itself.
We should probably omit one frame fewer.

Example with the stacktrace from #2056:

Old output:

<!--- Using Principia base address 7ff9eab60000 -->
<!--- Using Physics base address 7ff9ff390000 -->
[`ksp_plugin/pile_up.cpp:131`](https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/ksp_plugin/pile_up.cpp#L131)
<!--- Nothing for @   00007FF9EAC4ABDD          principia__VesselVelocity [0x00007FF9EAC4ABDC+273788] -->
[`base/thread_pool_body.hpp:15`](https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/base/thread_pool_body.hpp#L15)
[`base/thread_pool_body.hpp:80`](https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/base/thread_pool_body.hpp#L80)
<!--- Nothing for @   00007FF9EAC4B96C          principia__VesselVelocity [0x00007FF9EAC4B96B+277259] -->
<!--- Nothing for @   00007FF9EABE1A79          (No symbol) [0x00007FF9EABE1A78] -->
<!--- Nothing for @   00007FFA3E36DAB5          iswascii [0x00007FFA3E36DAB4+164] -->
<!--- Nothing for @   00007FFA3F181FE4          BaseThreadInitThunk [0x00007FFA3F181FE3+19] -->
<!--- Nothing for @   00007FFA415FCB81          RtlUserThreadStart [0x00007FFA415FCB80+32] -->

Renders as:

ksp_plugin/pile_up.cpp:131

base/thread_pool_body.hpp:15
base/thread_pool_body.hpp:80

New output:

<!---
 Using Principia base address 7FF9EAB60000 --><!---
 Using Physics base address 7FF9FF390000 --><!---
 Not in loaded modules: @   00007FFA415FCB81    RtlUserThreadStart [0x00007FFA415FCB80+32] --><!---
 Not in loaded modules: @   00007FFA3F181FE4    BaseThreadInitThunk [0x00007FFA3F181FE3+19] --><!---
 Not in loaded modules: @   00007FFA3E36DAB5    iswascii [0x00007FFA3E36DAB4+164] --><!---
 Not in Principia code: @   00007FF9EABE1A79    (No symbol) [0x00007FF9EABE1A78] --><!---
 Not in Principia code: @   00007FF9EAC4B96C    principia__VesselVelocity [0x00007FF9EAC4B96B+277259] --><!---
 Inline frame not in Principia code --><!---
 Inline frame not in Principia code --><!---
 Inline frame not in Principia code --><!---
 Inline frame not in Principia code --><!---
 Inline frame not in Principia code --><!---
 Inline frame not in Principia code --><!---
 Inline frame not in Principia code --><!---
 Inline frame not in Principia code --><!---
 Inline frame not in Principia code --><!---
--> https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/base/thread_pool_body.hpp#L57-L80 <!---
--> https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/base/thread_pool_body.hpp#L14-L15 <!---
 Inline frame not in Principia code --><!---
 Not in Principia code: @   00007FF9EAC4ABDD    principia__VesselVelocity [0x00007FF9EAC4ABDC+273788] --><!---
 Inline frame not in Principia code --><!---
 Inline frame not in Principia code --><!---
 Inline frame not in Principia code --><!---
--> https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/ksp_plugin/plugin.cpp#L719-L719 <!---
--> https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/ksp_plugin/pile_up.cpp#L126-L131

Renders as:

void ThreadPool<T>::DequeueCallAndExecute() {
for (;;) {
Call this_call;
// Wait until either the queue contains an element or this class is shutting
// down.
{
absl::MutexLock l(&lock_);
auto const has_calls_or_shutdown = [this] {
return shutdown_ || !calls_.empty();
};
lock_.Await(absl::Condition(&has_calls_or_shutdown));
if (shutdown_) {
break;
}
this_call = std::move(calls_.front());
calls_.pop_front();
}
// Execute the function without holding the |lock_| as it might take some
// time.
internal_thread_pool::ExecuteAndSetValue(this_call.function,
std::promise<T>& promise) {
promise.set_value(function());
return pile_up->DeformAndAdvanceTime(current_time_);
Status PileUp::DeformAndAdvanceTime(Instant const& t) {
absl::MutexLock l(lock_.get());
Status status;
if (psychohistory_->last().time() < t) {
DeformPileUpIfNeeded();
status = AdvanceTime(t);

New output with --no-snippet --no-comment:

[`base/thread_pool_body.hpp:80`](https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/base/thread_pool_body.hpp#L57-L80)
[`base/thread_pool_body.hpp:15`](https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/base/thread_pool_body.hpp#L14-L15)
[`ksp_plugin/plugin.cpp:719`](https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/ksp_plugin/plugin.cpp#L719-L719)
[`ksp_plugin/pile_up.cpp:131`](https://github.com/mockingbirdnest/Principia/blob/3e2334a95bcfc6869b5464ecda5ae48b5412dba0/ksp_plugin/pile_up.cpp#L126-L131)

Renders as:

base/thread_pool_body.hpp:80
base/thread_pool_body.hpp:15
ksp_plugin/plugin.cpp:719
ksp_plugin/pile_up.cpp:131

Int64 qwModuleBaseAddress,
out Int32 pdwDisplacement,
[MarshalAs(UnmanagedType.LPStruct)] IMAGEHLP_LINEW64 Line);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank line.

@pleroy pleroy added the LGTM label Jan 7, 2019
@eggrobin eggrobin force-pushed the better-stacktrace-decoding branch from 80bca5f to ec85c89 Compare January 7, 2019 22:27
@pleroy
Copy link
Member

pleroy commented Jan 13, 2019

retest this please

Sorry, something went wrong.

@pleroy pleroy merged commit 3c84cf6 into mockingbirdnest:master Jan 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants