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.

Sign up for GitHub

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

Jump to bottom

runtime: fix wrapper elision when mid-stack inlining #28640

Closed
randall77 opened this issue Nov 7, 2018 · 3 comments
Closed

runtime: fix wrapper elision when mid-stack inlining #28640

randall77 opened this issue Nov 7, 2018 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.12

Comments

@randall77
Copy link
Contributor

When we print a stack trace we skip wrapper functions, as they aren't user code so they might confuse users.
We currently make the decision on whether to print a stack frame or not, based on some information about the real frame that's on the stack.
However, once mid-stack inlining is enabled, a single real frame on the stack might represent multiple logical user frames. Some of those might be wrappers, some not.

The current behavior is that if the outermost frame is a wrapper, all the user frames inlined into it are not printed (unless the wrapper function is the first in the stack trace, which is a special case).
If the outermost frame is not a wrapper, all wrapper frames inlined into it are printed.

We really need to base the print/noprint decision on logical frames, not real frames.

CL 147361 has a band-aid fix for a test that fails because of this issue.
When this issue is fixed, remove that band-aid.
@randall77 randall77 added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 7, 2018
@randall77 randall77 added this to the Go1.12 milestone Nov 7, 2018
@randall77 randall77 self-assigned this Nov 7, 2018
@randall77 randall77 mentioned this issue Dec 3, 2018
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/152537 mentions this issue: cmd/compile,runtime: redo mid-stack inlining tracebacks

@randall77 randall77 mentioned this issue Dec 10, 2018
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/153477 mentions this issue: runtime: proper panic tracebacks with mid-stack inlining

gopherbot pushed a commit that referenced this issue Jan 4, 2019
@randall77
runtime: proper panic tracebacks with mid-stack inlining
af134b1 
As a followon to CL 152537, modify the panic-printing traceback
to also handle mid-stack inlining correctly.

Also declare -fm functions (aka method functions) as wrappers, so that
they get elided during traceback. This fixes part 2 of #26839.

Fixes #28640
Fixes #24488
Update #26839

Change-Id: I1c535a9b87a9a1ea699621be1e6526877b696c21
Reviewed-on: https://go-review.googlesource.com/c/153477
Reviewed-by: David Chase <drchase@google.com>
@bcmills bcmills mentioned this issue Jan 24, 2019
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/195818 mentions this issue: runtime: remove unneeded noinline directives

gopherbot pushed a commit that referenced this issue Sep 17, 2019
@randall77
runtime: remove unneeded noinline directives
07ad840 
Now that mid-stack inlining reports backtraces correctly, we no
longer need to protect against inlining in a few critical areas.

Update #19348
Update #28640
Update #34276

Change-Id: Ie68487e6482c3a9509ecf7ecbbd40fe43cee8381
Reviewed-on: https://go-review.googlesource.com/c/go/+/195818
Reviewed-by: David Chase <drchase@google.com>
@golang golang locked and limited conversation to collaborators Sep 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
2 participants
@randall77 @gopherbot