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

runtime: gentraceback no longer skips stack frames when printing #24628

Closed
odeke-em opened this issue Apr 1, 2018 · 6 comments
Closed

runtime: gentraceback no longer skips stack frames when printing #24628

odeke-em opened this issue Apr 1, 2018 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@odeke-em
Copy link
Member

odeke-em commented Apr 1, 2018

I got some time today to finish up and add a rigorous test for CL https://go-review.googlesource.com/#/c/go/+/37222 which involves printing the top and bottom n frames for easy reading in case of a massive stackoverflow.

To detect that we are printing stackframes, we have this condition

printing := pcbuf == nil && callback == nil

and in order for it to work, we need to be able to skip a bunch of frames when printing the bottom n stack frames.

However, with code that we used to skip through physical frames previously, CL https://go-review.googlesource.com/37854 aka commit ee97216 changed that behavior assuming that we'd never use skip unless pcbuf != nil. However, we actually need to skip through physical frames when printing tracebacks otherwise on trying to print top and bottom stackframes, we always end up printing the top irrespective of our skip which is a regression that silently snuck in for an entire year as it wasn't used.

The code that disabled proper skipping is ee97216#diff-5a2de8a1053d4e11fbc71407a5361e93L321
screen shot 2018-04-01 at 1 59 10 am
screen shot 2018-04-01 at 2 07 18 am

/cc @davidlazar @aclements

@bcmills
Copy link
Contributor

bcmills commented Apr 2, 2018

Is this a regression in a user-facing behavior, or just an internal function?

(I assume that it's blocking #7181; is that the only impact?)

@odeke-em
Copy link
Member Author

odeke-em commented Apr 2, 2018

@bcmills correct, it is only blocking #7181, but nonetheless a bug when skipping and printing -- something that hasn't yet been exposed though because we've been printing the entirety of stackframes.

@bcmills bcmills changed the title runtime: stackframe skipping while printing regression as skips now only work if pcbuf != nil runtime: gentraceback no longer skips stack frames when printing Apr 2, 2018
@bcmills bcmills added this to the Unplanned milestone Apr 3, 2018
@andybons andybons added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 16, 2018
@gopherbot
Copy link

Change https://golang.org/cl/37222 mentions this issue: runtime: stack traces of endless recursion now print top and bottom

@odeke-em
Copy link
Member Author

I've mailed a fix for this issue, packaged in the the CL that it was blocking as per https://go-review.googlesource.com/c/go/+/37222.

@odeke-em odeke-em self-assigned this Oct 19, 2020
@randall77 randall77 reopened this Nov 9, 2020
@randall77
Copy link
Contributor

CL was reverted.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@aclements
Copy link
Member

This code has all been completely refactored as of CL 468301. Now that PC buffer filling and traceback printing are completely separate entry-points and printing doesn't take a skip count, there's no longer confusion about the skip count. (Though, joke's on me, I'm about to bring back the printing skip count. But it's much more localized now!)

@golang golang locked and limited conversation to collaborators Mar 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants