golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.27k stars 17.7k forks source link

runtime/pprof: theoretical appendLocsForStack panic with SIGPROF between cgocallback and exitsyscall #70529

Open prattmic opened 2 days ago

prattmic commented 2 days ago

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.

gabyhelp commented 2 days ago

Related Issues

Related Code Changes

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)