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/pprof: theoretical appendLocsForStack panic with SIGPROF between cgocallback and exitsyscall #70529

Open
prattmic opened this issue Nov 22, 2024 · 1 comment
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@prattmic
Copy link
Member

runtime/pprof.appendLocsForStack asserts that an inline-expanded PC always expands to the same number of logical PCs (inlined frames). This is a static property of a given PC, so it should always be true.

When handling SIGPROF, if we are on an extra M running in C, we don't try to traceback since this is C anyway, and just add a sample with stack {pc, runtime._ExternalCode}.

"We are on an extra M running in C" is defined as gp.m.isExtraInC. In cgocallbackg, we clear this field after exitsyscall returns. This leaves a fairly long window when we are in fact running Go code, but the SIGPROF handler will think it is in C.

A lot of this code (particularly in exitsyscall) is reachable from normal Go code as well. If any of this code has more than 2 inlined frames at a single PC, then a SIGPROF from a normal Go context followed by a SIGPROF in this cgocallback context could trigger this appendLocsForStack panic.

I do not know if any code reachable in this window actually has more than 2 inlined frames. Only 2 frames is insufficient, as appendLocsForStack wouldn't actually care that the second frame is runtime._ExternalCode instead of the proper frame.

One potential fix is to attempt to do inline expansion in sigprofNonGoPC in case it actually is a Go PC.

@prattmic prattmic added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 22, 2024
@prattmic prattmic added this to the Go1.25 milestone Nov 22, 2024
@prattmic prattmic self-assigned this Nov 22, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Development

No branches or pull requests

3 participants