From 30c18878730434027dbefd343aad74963a1fdc48 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Thu, 1 Oct 2020 17:22:38 -0400 Subject: runtime,cmd/cgo: simplify C -> Go call path This redesigns the way calls work from C to exported Go functions. It removes several steps from the call path, makes cmd/cgo no longer sensitive to the Go calling convention, and eliminates the use of reflectcall from cgo. In order to avoid generating a large amount of FFI glue between the C and Go ABIs, the cgo tool has long depended on generating a C function that marshals the arguments into a struct, and then the actual ABI switch happens in functions with fixed signatures that simply take a pointer to this struct. In a way, this CL simply pushes this idea further. Currently, the cgo tool generates this argument struct in the exact layout of the Go stack frame and depends on reflectcall to unpack it into the appropriate Go call (even though it's actually reflectcall'ing a function generated by cgo). In this CL, we decouple this struct from the Go stack layout. Instead, cgo generates a Go function that takes the struct, unpacks it, and calls the exported function. Since this generated function has a generic signature (like the rest of the call path), we don't need reflectcall and can instead depend on the Go compiler itself to implement the call to the exported Go function. One complication is that syscall.NewCallback on Windows, which converts a Go function into a C function pointer, depends on cgocallback's current dynamic calling approach since the signatures of the callbacks aren't known statically. For this specific case, we continue to depend on reflectcall. Really, the current approach makes some overly simplistic assumptions about translating the C ABI to the Go ABI. Now we're at least in a much better position to do a proper ABI translation. For comparison, the current cgo call path looks like: GoF (generated C function) -> crosscall2 (in cgo/asm_*.s) -> _cgoexp_GoF (generated Go function) -> cgocallback (in asm_*.s) -> cgocallback_gofunc (in asm_*.s) -> cgocallbackg (in cgocall.go) -> cgocallbackg1 (in cgocall.go) -> reflectcall (in asm_*.s) -> _cgoexpwrap_GoF (generated Go function) -> p.GoF Now the call path looks like: GoF (generated C function) -> crosscall2 (in cgo/asm_*.s) -> cgocallback (in asm_*.s) -> cgocallbackg (in cgocall.go) -> cgocallbackg1 (in cgocall.go) -> _cgoexp_GoF (generated Go function) -> p.GoF Notably: 1. We combine _cgoexp_GoF and _cgoexpwrap_GoF and move the combined operation to the end of the sequence. This combined function also handles reflectcall's previous role. 2. We combined cgocallback and cgocallback_gofunc since the only purpose of having both was to convert a raw PC into a Go function value. We instead construct the Go function value in cgocallbackg1. 3. cgocallbackg1 no longer reaches backwards through the stack to get the arguments to cgocallback_gofunc. Instead, we just pass the arguments down. 4. Currently, we need an explicit msanwrite to mark the results struct as written because reflectcall doesn't do this. Now, the results are written by regular Go assignments, so the Go compiler generates the necessary MSAN annotations. This also means we no longer need to track the size of the arguments frame. Updates #40724, since now we don't need to teach cgo about the register ABI or change how it uses reflectcall. Change-Id: I7840489a2597962aeb670e0c1798a16a7359c94f Reviewed-on: https://go-review.googlesource.com/c/go/+/258938 Trust: Austin Clements Run-TryBot: Austin Clements TryBot-Result: Go Bot Reviewed-by: Cherry Zhang --- src/runtime/traceback.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/runtime/traceback.go') diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index 94f4a44976..f3df152535 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -450,7 +450,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in } n++ - if f.funcID == funcID_cgocallback_gofunc && len(cgoCtxt) > 0 { + if f.funcID == funcID_cgocallback && len(cgoCtxt) > 0 { ctxt := cgoCtxt[len(cgoCtxt)-1] cgoCtxt = cgoCtxt[:len(cgoCtxt)-1] -- cgit v1.3 From 3a81338622eb5c8b94f11001855e2a68a9e36bed Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Sat, 18 Feb 2017 03:03:32 -0700 Subject: runtime: make stack traces of endless recursion print only top and bottom 50 This CL makes it so that instead of printing massive stack traces during endless recursion, which spams users and aren't useful, it now prints out the top and bottom 50 frames. If the number of frames <= 100 (_TracebackMaxFrames), we'll just print all the frames out. Modified gentraceback to return counts of: * ntotalframes * nregularframes which allows us to get accurate counts of the various kinds of frames. While here, also fixed a bug that resulted from CL 37222, in which we no longer accounted for decrementing requested frame skips, and assumed that when printing, that skip would always be 0. The fix is instead to add precondition that we'll only print if skip <= 0, but also decrement skip as we iterate. Fixes #7181. Fixes #24628. Change-Id: Ie31ec6413fdfbe43827b254fef7d99ea26a5277f Reviewed-on: https://go-review.googlesource.com/c/go/+/37222 Run-TryBot: Emmanuel Odeke TryBot-Result: Go Bot Reviewed-by: Keith Randall Trust: Emmanuel Odeke --- src/runtime/crash_test.go | 126 ++++++++++++++++++++++ src/runtime/testdata/testprog/deadlock.go | 13 +++ src/runtime/traceback.go | 173 ++++++++++++++++++++++-------- 3 files changed, 265 insertions(+), 47 deletions(-) (limited to 'src/runtime/traceback.go') diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go index 5e22b7593e..66822e5cde 100644 --- a/src/runtime/crash_test.go +++ b/src/runtime/crash_test.go @@ -240,6 +240,132 @@ func TestStackOverflow(t *testing.T) { } } +func TestStackOverflowTopAndBottomTraces(t *testing.T) { + output := runTestProg(t, "testprog", "StackOverflowTopAndBottomTraces") + + // 1. First things first, we expect to traverse from + // runtime: goroutine stack exceeds 10000-byte limit + // and down to the very end until we see: + // runtime.goexit() + mustHaves := []string{ + // Top half expectations + "\\s*runtime: goroutine stack exceeds 10000-byte limit\n", + "\\s*fatal error: stack overflow\n", + "\\s*runtime stack:\n", + "\\s*runtime.throw[^\n]+\n\t.+:\\d+ [^\n]+", + "\\s+runtime\\.newstack[^\n]+\n\t.+:\\d+ [^\n]+", + "\\s+runtime.morestack[^\n]+\n\t.+:\\d+ [^\n]+", + "\\s+goroutine 1 \\[running\\]:", + + // Bottom half expectations + "\\s*main.main\\(\\)\n", + "\\s*runtime.main\\(\\)\n", + "\\s*runtime.goexit\\(\\)\n", + } + + for _, pat := range mustHaves { + reg := regexp.MustCompile(pat) + match := reg.FindAllString(output, -1) + if len(match) == 0 { + t.Errorf("Failed to find pattern %q", pat) + } + } + + // 2. Split top and bottom halves by the "... ({n} stack frames omitted)" message + regHalving := regexp.MustCompile("\\.{3} \\(\\d+ stack frames omitted\\)") + halverMatches := regHalving.FindAllString(output, -1) + if len(halverMatches) != 1 { + t.Fatal("Failed to find the `stack frames omitted` pattern") + } + str := string(output) + halver := halverMatches[0] + midIndex := strings.Index(str, halver) + topHalf, bottomHalf := str[:midIndex], str[midIndex+len(halver):] + // 2.1. Sanity check, len(topHalf) >= halver || len(bottomHalf) >= halver + if len(topHalf) < len(halver) || len(bottomHalf) < len(halver) { + t.Fatalf("Sanity check len(topHalf) = %d len(bottomHalf) = %d; both must be >= len(halver) %d", + len(topHalf), len(bottomHalf), len(halver)) + } + + // 3. In each of the halves, we should have an equal number + // of stacktraces before and after the "omitted frames" message. + regStackTraces := regexp.MustCompile("\n[^\n]+\n\t.+:\\d+ .+ fp=0x.+ sp=0x.+ pc=0x.+") + topHalfStackTraces := regStackTraces.FindAllString(topHalf, -1) + bottomHalfStackTraces := regStackTraces.FindAllString(bottomHalf, -1) + nTopHalf, nBottomHalf := len(topHalfStackTraces), len(bottomHalfStackTraces) + if nTopHalf == 0 || nBottomHalf == 0 { + t.Fatal("Both lengths of stack-halves should be non-zero") + } + // The bottom half will always have the 50 non-runtime frames along with these 3 frames: + // * main.main() + // * "runtime.main" + // * "runtime.goexit" + // hence we need to decrement 3 counted lines. + if nTopHalf != nBottomHalf-3 { + t.Errorf("len(topHalfStackTraces)=%d len(bottomHalfStackTraces)-3=%d yet must be equal\n", nTopHalf, nBottomHalf-3) + } + + // 4. Next, prune out the: + // func... + // line... + // pairs in both of the halves. + prunes := []struct { + src *string + matches []string + }{ + {src: &topHalf, matches: topHalfStackTraces}, + {src: &bottomHalf, matches: bottomHalfStackTraces}, + } + + for _, prune := range prunes { + str := *prune.src + for _, match := range prune.matches { + index := strings.Index(str, match) + str = str[:index] + str[index+len(match):] + } + *prune.src = str + } + + // 5. Now match and prune out the remaining patterns in the top and bottom halves. + // We aren't touching the bottom stack since its patterns are already matched + // by the: + // func... + // line... + // pairs + topPartPrunables := []string{ + "^\\s*runtime: goroutine stack exceeds 10000-byte limit\n", + "\\s*fatal error: stack overflow\n", + "\\s*runtime stack:\n", + "\\s*runtime.throw[^\n]+\n\t.+:\\d+ [^\n]+", + "\\s+runtime\\.newstack[^\n]+\n\t.+:\\d+ [^\n]+", + "\\s+runtime.morestack[^\n]+\n\t.+:\\d+ [^\n]+", + "\\s+goroutine 1 \\[running\\]:", + } + + for _, pat := range topPartPrunables { + reg := regexp.MustCompile(pat) + matches := reg.FindAllString(topHalf, -1) + if len(matches) == 0 { + t.Errorf("top stack traces do not contain pattern: %q", reg) + } else if len(matches) != 1 { + t.Errorf("inconsistent state got %d matches want only 1", len(matches)) + } else { + match := matches[0] + idx := strings.Index(topHalf, match) + topHalf = topHalf[:idx] + topHalf[idx+len(match):] + } + } + + // 6. At the end we should only be left with + // newlines in both the top and bottom halves. + topHalf = strings.TrimSpace(topHalf) + bottomHalf = strings.TrimSpace(bottomHalf) + if topHalf != "" && bottomHalf != "" { + t.Fatalf("len(topHalf)=%d len(bottomHalf)=%d\ntopHalf=\n%s\n\nbottomHalf=\n%s", + len(topHalf), len(bottomHalf), topHalf, bottomHalf) + } +} + func TestThreadExhaustion(t *testing.T) { output := runTestProg(t, "testprog", "ThreadExhaustion") want := "runtime: program exceeds 10-thread limit\nfatal error: thread exhaustion" diff --git a/src/runtime/testdata/testprog/deadlock.go b/src/runtime/testdata/testprog/deadlock.go index 105d6a5faa..0ee1557b13 100644 --- a/src/runtime/testdata/testprog/deadlock.go +++ b/src/runtime/testdata/testprog/deadlock.go @@ -20,6 +20,7 @@ func init() { register("LockedDeadlock2", LockedDeadlock2) register("GoexitDeadlock", GoexitDeadlock) register("StackOverflow", StackOverflow) + register("StackOverflowTopAndBottomTraces", StackOverflowTopAndBottomTraces) register("ThreadExhaustion", ThreadExhaustion) register("RecursivePanic", RecursivePanic) register("RecursivePanic2", RecursivePanic2) @@ -85,6 +86,18 @@ func StackOverflow() { f() } +func StackOverflowTopAndBottomTraces() { + var fi, gi func() + fi = func() { + gi() + } + gi = func() { + fi() + } + debug.SetMaxStack(10000) + fi() +} + func ThreadExhaustion() { debug.SetMaxThreads(10) c := make(chan int) diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index f3df152535..389ae87185 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -73,17 +73,34 @@ func tracebackdefers(gp *g, callback func(*stkframe, unsafe.Pointer) bool, v uns const sizeofSkipFunction = 256 -// Generic traceback. Handles runtime stack prints (pcbuf == nil), +// Generic traceback. Handles runtime stack prints (pcbuf == nil && callback == nil), // the runtime.Callers function (pcbuf != nil), as well as the garbage // collector (callback != nil). A little clunky to merge these, but avoids // duplicating the code and all its subtlety. // -// The skip argument is only valid with pcbuf != nil and counts the number -// of logical frames to skip rather than physical frames (with inlining, a -// PC in pcbuf can represent multiple calls). If a PC is partially skipped -// and max > 1, pcbuf[1] will be runtime.skipPleaseUseCallersFrames+N where -// N indicates the number of logical frames to skip in pcbuf[0]. +// The skip argument counts the number of logical frames to skip rather +// than physical frames (with inlining, a PC in pcbuf can represent multiple calls). func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max int, callback func(*stkframe, unsafe.Pointer) bool, v unsafe.Pointer, flags uint) int { + var op operation = traversing + if pcbuf == nil && callback == nil { + op = printing + } + n, _ := ggentraceback(pc0, sp0, lr0, gp, skip, pcbuf, max, op, callback, v, flags) + return n +} + +type operation int8 + +const ( + traversing operation = 1 << iota + countingframes + printing +) + +// n always returns the number of total frames <= max. +// nregularframes is the count of non-runtime frames. +// nregularframes is only valid if op == countingframes. +func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max int, op operation, callback func(*stkframe, unsafe.Pointer) bool, v unsafe.Pointer, flags uint) (ntotalframes, nregularframes int) { if skip > 0 && callback != nil { throw("gentraceback callback cannot be used with non-zero skip") } @@ -135,7 +152,6 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in } waspanic := false cgoCtxt := gp.cgoCtxt - printing := pcbuf == nil && callback == nil // If the PC is zero, it's likely a nil function call. // Start in the caller's frame. @@ -149,6 +165,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in } } + printing := op == printing f := findfunc(frame.pc) if !f.valid() { if callback != nil || printing { @@ -158,15 +175,14 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in if callback != nil { throw("unknown pc") } - return 0 + return 0, 0 } frame.fn = f var cache pcvalueCache lastFuncID := funcID_normal - n := 0 - for n < max { + for ntotalframes < max { // Typically: // pc is the PC of the running function. // sp is the stack pointer at that program counter. @@ -229,7 +245,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in } else { var lrPtr uintptr if usesLR { - if n == 0 && frame.sp < frame.fp || frame.lr == 0 { + if ntotalframes == 0 && frame.sp < frame.fp || frame.lr == 0 { lrPtr = frame.sp frame.lr = *(*uintptr)(unsafe.Pointer(lrPtr)) } @@ -320,11 +336,15 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in if callback != nil { if !callback((*stkframe)(noescape(unsafe.Pointer(&frame))), v) { - return n + return } } - if pcbuf != nil { + if pcbuf == nil && skip > 0 { + // In this case we are printing and we still need to count + // the number of frames. See https://golang.org/issues/24628. + skip-- + } else if pcbuf != nil { pc := frame.pc // backup to CALL instruction to read inlining info (same logic as below) tracepc := pc @@ -339,7 +359,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in // See issue 34123. // The pc can be at function entry when the frame is initialized without // actually running code, like runtime.mstart. - if (n == 0 && flags&_TraceTrap != 0) || waspanic || pc == f.entry { + if (ntotalframes == 0 && flags&_TraceTrap != 0) || waspanic || pc == f.entry { pc++ } else { tracepc-- @@ -357,9 +377,9 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in // ignore wrappers } else if skip > 0 { skip-- - } else if n < max { - (*[1 << 20]uintptr)(unsafe.Pointer(pcbuf))[n] = pc - n++ + } else if ntotalframes < max { + (*[1 << 20]uintptr)(unsafe.Pointer(pcbuf))[ntotalframes] = pc + ntotalframes++ } lastFuncID = inltree[ix].funcID // Back up to an instruction in the "caller". @@ -372,17 +392,15 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in // Ignore wrapper functions (except when they trigger panics). } else if skip > 0 { skip-- - } else if n < max { - (*[1 << 20]uintptr)(unsafe.Pointer(pcbuf))[n] = pc - n++ + } else if ntotalframes < max { + (*[1 << 20]uintptr)(unsafe.Pointer(pcbuf))[ntotalframes] = pc + ntotalframes++ } lastFuncID = f.funcID - n-- // offset n++ below + ntotalframes-- // offset ntotalframes++ below } - if printing { - // assume skip=0 for printing. - // + if printing && skip <= 0 { // Never elide wrappers if we haven't printed // any frames. And don't elide wrappers that // called panic rather than the wrapped @@ -390,7 +408,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in // backup to CALL instruction to read inlining info (same logic as below) tracepc := frame.pc - if (n > 0 || flags&_TraceTrap == 0) && frame.pc > f.entry && !waspanic { + if (ntotalframes > 0 || flags&_TraceTrap == 0) && frame.pc > f.entry && !waspanic { tracepc-- } // If there is inlining info, print the inner frames. @@ -448,7 +466,14 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in } lastFuncID = f.funcID } - n++ + + if op == countingframes { + name := fullfuncname(f, frame.pc) + if len(name) < len("runtime.") || name[:len("runtime.")] != "runtime." { + nregularframes++ + } + } + ntotalframes++ if f.funcID == funcID_cgocallback && len(cgoCtxt) > 0 { ctxt := cgoCtxt[len(cgoCtxt)-1] @@ -458,7 +483,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in // callback != nil only used when we only care // about Go frames. if skip == 0 && callback == nil { - n = tracebackCgoContext(pcbuf, printing, ctxt, n, max) + ntotalframes = tracebackCgoContext(pcbuf, printing, ctxt, ntotalframes, max) } } @@ -498,7 +523,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in } if printing { - n = nprint + ntotalframes = nprint } // Note that panic != nil is okay here: there can be leftover panics, @@ -541,13 +566,13 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in // At other times, such as when gathering a stack for a profiling signal // or when printing a traceback during a crash, everything may not be // stopped nicely, and the stack walk may not be able to complete. - if callback != nil && n < max && frame.sp != gp.stktopsp { + if callback != nil && ntotalframes < max && frame.sp != gp.stktopsp { print("runtime: g", gp.goid, ": frame.sp=", hex(frame.sp), " top=", hex(gp.stktopsp), "\n") - print("\tstack=[", hex(gp.stack.lo), "-", hex(gp.stack.hi), "] n=", n, " max=", max, "\n") + print("\tstack=[", hex(gp.stack.lo), "-", hex(gp.stack.hi), "] n=", ntotalframes, " max=", max, "\n") throw("traceback did not unwind completely") } - return n + return } // reflectMethodValue is a partial duplicate of reflect.makeFuncImpl @@ -712,24 +737,15 @@ func traceback1(pc, sp, lr uintptr, gp *g, flags uint) { printCgoTraceback(&cgoCallers) } - var n int if readgstatus(gp)&^_Gscan == _Gsyscall { // Override registers if blocked in system call. pc = gp.syscallpc sp = gp.syscallsp flags &^= _TraceTrap } - // Print traceback. By default, omits runtime frames. - // If that means we print nothing at all, repeat forcing all frames printed. - n = gentraceback(pc, sp, lr, gp, 0, nil, _TracebackMaxFrames, nil, nil, flags) - if n == 0 && (flags&_TraceRuntimeFrames) == 0 { - n = gentraceback(pc, sp, lr, gp, 0, nil, _TracebackMaxFrames, nil, nil, flags|_TraceRuntimeFrames) - } - if n == _TracebackMaxFrames { - print("...additional frames elided...\n") - } - printcreatedby(gp) + printtraceback(pc, sp, lr, gp, flags) + printcreatedby(gp) if gp.ancestors == nil { return } @@ -738,6 +754,62 @@ func traceback1(pc, sp, lr uintptr, gp *g, flags uint) { } } +// countframes traverses the current stacktrace from the top of pc0 to its bottom, excluding runtime frames. +// If flags&_TraceRuntimeframes != 0, it'll include the number of runtime frames in the count. +func countframes(pc0, sp0, lr0 uintptr, gp *g, flags uint) (nframes int) { + ntotalframes, nregularframes := ggentraceback(pc0, sp0, lr0, gp, 0, nil, 1<<31-1, countingframes, nil, nil, flags) + nframes = nregularframes + if flags&_TraceRuntimeFrames != 0 { + nframes = ntotalframes + } + return nframes +} + +func printtraceback(pc, sp, lr uintptr, gp *g, flags uint) { + // We'd like to print: + // * top nMaxFramesPerPrint frames + // * bottom nMaxFramesPerPrint frames. + // See https://golang.org/issue/7181. + + nMaxFramesPerPrint := _TracebackMaxFrames / 2 + nTop := gentraceback(pc, sp, lr, gp, 0, nil, nMaxFramesPerPrint, nil, nil, flags) + if nTop < nMaxFramesPerPrint { + // The common case, in which the traceback has less than nMaxFramesPerPrint. + // By default, omits runtime frames. + // If nTop == 0, it means we printed nothing at all, so repeat, + // and this time force all frames to be printed. + if nTop == 0 && (flags&_TraceRuntimeFrames) == 0 { + // Try again to print the frames, but this time with _TraceRuntimeFrames. + printtraceback(pc, sp, lr, gp, flags|_TraceRuntimeFrames) + } + return + } + + // Figure out the stack size in order to print the bottom max(nMaxFramesPerPrint) frames. + // + // TODO(odeke-em, iant, khr): perhaps investigate and revise the solution in + // https://go-review.googlesource.com/c/go/+/37222/9/src/runtime/traceback.go + // so that we'll always only need 1 stack walk, instead of 2 as in this worst case. + nframes := countframes(pc, sp, lr, gp, flags) + + if nframes <= _TracebackMaxFrames { + // In this case, we'll just print out from where we left off until the end. + gentraceback(pc, sp, lr, gp, nMaxFramesPerPrint /* skip */, nil, 1<<31-1, nil, nil, flags) + return + } + + // Otherwise, now skip until the bottom last nMaxFramesPerPrint. + + // Calculate the number of stack frames to elide since we + // are printing top and bottom each of nMaxFramesPerPrint. + if elide := nframes - _TracebackMaxFrames; elide > 0 { + print("\n... (") + println(elide, "stack frames omitted)\n") + } + skip := nframes - nMaxFramesPerPrint + _ = gentraceback(pc, sp, lr, gp, skip, nil, 1<<31-1 /* max int32 as the biggest frame number */, nil, nil, flags) +} + // printAncestorTraceback prints the traceback of the given ancestor. // TODO: Unify this with gentraceback and CallersFrames. func printAncestorTraceback(ancestor ancestorInfo) { @@ -758,11 +830,9 @@ func printAncestorTraceback(ancestor ancestorInfo) { } } -// printAncestorTraceback prints the given function info at a given pc -// within an ancestor traceback. The precision of this info is reduced -// due to only have access to the pcs at the time of the caller -// goroutine being created. -func printAncestorTracebackFuncInfo(f funcInfo, pc uintptr) { +// fullfuncname retrieves the name for a funcInfo, but if perhaps it was inlined, it'll retrieve +// unwind and retrieve the original name. +func fullfuncname(f funcInfo, pc uintptr) string { name := funcname(f) if inldata := funcdata(f, _FUNCDATA_InlTree); inldata != nil { inltree := (*[1 << 20]inlinedCall)(inldata) @@ -771,6 +841,15 @@ func printAncestorTracebackFuncInfo(f funcInfo, pc uintptr) { name = funcnameFromNameoff(f, inltree[ix].func_) } } + return name +} + +// printAncestorTraceback prints the given function info at a given pc +// within an ancestor traceback. The precision of this info is reduced +// due to only have access to the pcs at the time of the caller +// goroutine being created. +func printAncestorTracebackFuncInfo(f funcInfo, pc uintptr) { + name := fullfuncname(f, pc) file, line := funcline(f, pc) if name == "runtime.gopanic" { name = "panic" -- cgit v1.3 From d4957122ee8a45aa73b0e8700d3a41c0ee9f4442 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Mon, 9 Nov 2020 20:31:58 +0000 Subject: Revert "runtime: make stack traces of endless recursion print only top and bottom 50" This reverts commit 3a81338622eb5c8b94f11001855e2a68a9e36bed. Reason for revert: Some edge cases not properly covered due to changes within runtime traceback generation since 2017, that need to be examined. This change landed very late in the Go1.16 cycle. Change-Id: I8cf6f46ea0ef6161d878e79943e6c7cdac94bccf Reviewed-on: https://go-review.googlesource.com/c/go/+/268577 Trust: Emmanuel Odeke Run-TryBot: Emmanuel Odeke TryBot-Result: Go Bot Reviewed-by: Keith Randall --- src/runtime/crash_test.go | 126 ---------------------- src/runtime/testdata/testprog/deadlock.go | 13 --- src/runtime/traceback.go | 173 ++++++++---------------------- 3 files changed, 47 insertions(+), 265 deletions(-) (limited to 'src/runtime/traceback.go') diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go index 66822e5cde..5e22b7593e 100644 --- a/src/runtime/crash_test.go +++ b/src/runtime/crash_test.go @@ -240,132 +240,6 @@ func TestStackOverflow(t *testing.T) { } } -func TestStackOverflowTopAndBottomTraces(t *testing.T) { - output := runTestProg(t, "testprog", "StackOverflowTopAndBottomTraces") - - // 1. First things first, we expect to traverse from - // runtime: goroutine stack exceeds 10000-byte limit - // and down to the very end until we see: - // runtime.goexit() - mustHaves := []string{ - // Top half expectations - "\\s*runtime: goroutine stack exceeds 10000-byte limit\n", - "\\s*fatal error: stack overflow\n", - "\\s*runtime stack:\n", - "\\s*runtime.throw[^\n]+\n\t.+:\\d+ [^\n]+", - "\\s+runtime\\.newstack[^\n]+\n\t.+:\\d+ [^\n]+", - "\\s+runtime.morestack[^\n]+\n\t.+:\\d+ [^\n]+", - "\\s+goroutine 1 \\[running\\]:", - - // Bottom half expectations - "\\s*main.main\\(\\)\n", - "\\s*runtime.main\\(\\)\n", - "\\s*runtime.goexit\\(\\)\n", - } - - for _, pat := range mustHaves { - reg := regexp.MustCompile(pat) - match := reg.FindAllString(output, -1) - if len(match) == 0 { - t.Errorf("Failed to find pattern %q", pat) - } - } - - // 2. Split top and bottom halves by the "... ({n} stack frames omitted)" message - regHalving := regexp.MustCompile("\\.{3} \\(\\d+ stack frames omitted\\)") - halverMatches := regHalving.FindAllString(output, -1) - if len(halverMatches) != 1 { - t.Fatal("Failed to find the `stack frames omitted` pattern") - } - str := string(output) - halver := halverMatches[0] - midIndex := strings.Index(str, halver) - topHalf, bottomHalf := str[:midIndex], str[midIndex+len(halver):] - // 2.1. Sanity check, len(topHalf) >= halver || len(bottomHalf) >= halver - if len(topHalf) < len(halver) || len(bottomHalf) < len(halver) { - t.Fatalf("Sanity check len(topHalf) = %d len(bottomHalf) = %d; both must be >= len(halver) %d", - len(topHalf), len(bottomHalf), len(halver)) - } - - // 3. In each of the halves, we should have an equal number - // of stacktraces before and after the "omitted frames" message. - regStackTraces := regexp.MustCompile("\n[^\n]+\n\t.+:\\d+ .+ fp=0x.+ sp=0x.+ pc=0x.+") - topHalfStackTraces := regStackTraces.FindAllString(topHalf, -1) - bottomHalfStackTraces := regStackTraces.FindAllString(bottomHalf, -1) - nTopHalf, nBottomHalf := len(topHalfStackTraces), len(bottomHalfStackTraces) - if nTopHalf == 0 || nBottomHalf == 0 { - t.Fatal("Both lengths of stack-halves should be non-zero") - } - // The bottom half will always have the 50 non-runtime frames along with these 3 frames: - // * main.main() - // * "runtime.main" - // * "runtime.goexit" - // hence we need to decrement 3 counted lines. - if nTopHalf != nBottomHalf-3 { - t.Errorf("len(topHalfStackTraces)=%d len(bottomHalfStackTraces)-3=%d yet must be equal\n", nTopHalf, nBottomHalf-3) - } - - // 4. Next, prune out the: - // func... - // line... - // pairs in both of the halves. - prunes := []struct { - src *string - matches []string - }{ - {src: &topHalf, matches: topHalfStackTraces}, - {src: &bottomHalf, matches: bottomHalfStackTraces}, - } - - for _, prune := range prunes { - str := *prune.src - for _, match := range prune.matches { - index := strings.Index(str, match) - str = str[:index] + str[index+len(match):] - } - *prune.src = str - } - - // 5. Now match and prune out the remaining patterns in the top and bottom halves. - // We aren't touching the bottom stack since its patterns are already matched - // by the: - // func... - // line... - // pairs - topPartPrunables := []string{ - "^\\s*runtime: goroutine stack exceeds 10000-byte limit\n", - "\\s*fatal error: stack overflow\n", - "\\s*runtime stack:\n", - "\\s*runtime.throw[^\n]+\n\t.+:\\d+ [^\n]+", - "\\s+runtime\\.newstack[^\n]+\n\t.+:\\d+ [^\n]+", - "\\s+runtime.morestack[^\n]+\n\t.+:\\d+ [^\n]+", - "\\s+goroutine 1 \\[running\\]:", - } - - for _, pat := range topPartPrunables { - reg := regexp.MustCompile(pat) - matches := reg.FindAllString(topHalf, -1) - if len(matches) == 0 { - t.Errorf("top stack traces do not contain pattern: %q", reg) - } else if len(matches) != 1 { - t.Errorf("inconsistent state got %d matches want only 1", len(matches)) - } else { - match := matches[0] - idx := strings.Index(topHalf, match) - topHalf = topHalf[:idx] + topHalf[idx+len(match):] - } - } - - // 6. At the end we should only be left with - // newlines in both the top and bottom halves. - topHalf = strings.TrimSpace(topHalf) - bottomHalf = strings.TrimSpace(bottomHalf) - if topHalf != "" && bottomHalf != "" { - t.Fatalf("len(topHalf)=%d len(bottomHalf)=%d\ntopHalf=\n%s\n\nbottomHalf=\n%s", - len(topHalf), len(bottomHalf), topHalf, bottomHalf) - } -} - func TestThreadExhaustion(t *testing.T) { output := runTestProg(t, "testprog", "ThreadExhaustion") want := "runtime: program exceeds 10-thread limit\nfatal error: thread exhaustion" diff --git a/src/runtime/testdata/testprog/deadlock.go b/src/runtime/testdata/testprog/deadlock.go index 0ee1557b13..105d6a5faa 100644 --- a/src/runtime/testdata/testprog/deadlock.go +++ b/src/runtime/testdata/testprog/deadlock.go @@ -20,7 +20,6 @@ func init() { register("LockedDeadlock2", LockedDeadlock2) register("GoexitDeadlock", GoexitDeadlock) register("StackOverflow", StackOverflow) - register("StackOverflowTopAndBottomTraces", StackOverflowTopAndBottomTraces) register("ThreadExhaustion", ThreadExhaustion) register("RecursivePanic", RecursivePanic) register("RecursivePanic2", RecursivePanic2) @@ -86,18 +85,6 @@ func StackOverflow() { f() } -func StackOverflowTopAndBottomTraces() { - var fi, gi func() - fi = func() { - gi() - } - gi = func() { - fi() - } - debug.SetMaxStack(10000) - fi() -} - func ThreadExhaustion() { debug.SetMaxThreads(10) c := make(chan int) diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index 389ae87185..f3df152535 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -73,34 +73,17 @@ func tracebackdefers(gp *g, callback func(*stkframe, unsafe.Pointer) bool, v uns const sizeofSkipFunction = 256 -// Generic traceback. Handles runtime stack prints (pcbuf == nil && callback == nil), +// Generic traceback. Handles runtime stack prints (pcbuf == nil), // the runtime.Callers function (pcbuf != nil), as well as the garbage // collector (callback != nil). A little clunky to merge these, but avoids // duplicating the code and all its subtlety. // -// The skip argument counts the number of logical frames to skip rather -// than physical frames (with inlining, a PC in pcbuf can represent multiple calls). +// The skip argument is only valid with pcbuf != nil and counts the number +// of logical frames to skip rather than physical frames (with inlining, a +// PC in pcbuf can represent multiple calls). If a PC is partially skipped +// and max > 1, pcbuf[1] will be runtime.skipPleaseUseCallersFrames+N where +// N indicates the number of logical frames to skip in pcbuf[0]. func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max int, callback func(*stkframe, unsafe.Pointer) bool, v unsafe.Pointer, flags uint) int { - var op operation = traversing - if pcbuf == nil && callback == nil { - op = printing - } - n, _ := ggentraceback(pc0, sp0, lr0, gp, skip, pcbuf, max, op, callback, v, flags) - return n -} - -type operation int8 - -const ( - traversing operation = 1 << iota - countingframes - printing -) - -// n always returns the number of total frames <= max. -// nregularframes is the count of non-runtime frames. -// nregularframes is only valid if op == countingframes. -func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max int, op operation, callback func(*stkframe, unsafe.Pointer) bool, v unsafe.Pointer, flags uint) (ntotalframes, nregularframes int) { if skip > 0 && callback != nil { throw("gentraceback callback cannot be used with non-zero skip") } @@ -152,6 +135,7 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i } waspanic := false cgoCtxt := gp.cgoCtxt + printing := pcbuf == nil && callback == nil // If the PC is zero, it's likely a nil function call. // Start in the caller's frame. @@ -165,7 +149,6 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i } } - printing := op == printing f := findfunc(frame.pc) if !f.valid() { if callback != nil || printing { @@ -175,14 +158,15 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i if callback != nil { throw("unknown pc") } - return 0, 0 + return 0 } frame.fn = f var cache pcvalueCache lastFuncID := funcID_normal - for ntotalframes < max { + n := 0 + for n < max { // Typically: // pc is the PC of the running function. // sp is the stack pointer at that program counter. @@ -245,7 +229,7 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i } else { var lrPtr uintptr if usesLR { - if ntotalframes == 0 && frame.sp < frame.fp || frame.lr == 0 { + if n == 0 && frame.sp < frame.fp || frame.lr == 0 { lrPtr = frame.sp frame.lr = *(*uintptr)(unsafe.Pointer(lrPtr)) } @@ -336,15 +320,11 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i if callback != nil { if !callback((*stkframe)(noescape(unsafe.Pointer(&frame))), v) { - return + return n } } - if pcbuf == nil && skip > 0 { - // In this case we are printing and we still need to count - // the number of frames. See https://golang.org/issues/24628. - skip-- - } else if pcbuf != nil { + if pcbuf != nil { pc := frame.pc // backup to CALL instruction to read inlining info (same logic as below) tracepc := pc @@ -359,7 +339,7 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i // See issue 34123. // The pc can be at function entry when the frame is initialized without // actually running code, like runtime.mstart. - if (ntotalframes == 0 && flags&_TraceTrap != 0) || waspanic || pc == f.entry { + if (n == 0 && flags&_TraceTrap != 0) || waspanic || pc == f.entry { pc++ } else { tracepc-- @@ -377,9 +357,9 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i // ignore wrappers } else if skip > 0 { skip-- - } else if ntotalframes < max { - (*[1 << 20]uintptr)(unsafe.Pointer(pcbuf))[ntotalframes] = pc - ntotalframes++ + } else if n < max { + (*[1 << 20]uintptr)(unsafe.Pointer(pcbuf))[n] = pc + n++ } lastFuncID = inltree[ix].funcID // Back up to an instruction in the "caller". @@ -392,15 +372,17 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i // Ignore wrapper functions (except when they trigger panics). } else if skip > 0 { skip-- - } else if ntotalframes < max { - (*[1 << 20]uintptr)(unsafe.Pointer(pcbuf))[ntotalframes] = pc - ntotalframes++ + } else if n < max { + (*[1 << 20]uintptr)(unsafe.Pointer(pcbuf))[n] = pc + n++ } lastFuncID = f.funcID - ntotalframes-- // offset ntotalframes++ below + n-- // offset n++ below } - if printing && skip <= 0 { + if printing { + // assume skip=0 for printing. + // // Never elide wrappers if we haven't printed // any frames. And don't elide wrappers that // called panic rather than the wrapped @@ -408,7 +390,7 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i // backup to CALL instruction to read inlining info (same logic as below) tracepc := frame.pc - if (ntotalframes > 0 || flags&_TraceTrap == 0) && frame.pc > f.entry && !waspanic { + if (n > 0 || flags&_TraceTrap == 0) && frame.pc > f.entry && !waspanic { tracepc-- } // If there is inlining info, print the inner frames. @@ -466,14 +448,7 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i } lastFuncID = f.funcID } - - if op == countingframes { - name := fullfuncname(f, frame.pc) - if len(name) < len("runtime.") || name[:len("runtime.")] != "runtime." { - nregularframes++ - } - } - ntotalframes++ + n++ if f.funcID == funcID_cgocallback && len(cgoCtxt) > 0 { ctxt := cgoCtxt[len(cgoCtxt)-1] @@ -483,7 +458,7 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i // callback != nil only used when we only care // about Go frames. if skip == 0 && callback == nil { - ntotalframes = tracebackCgoContext(pcbuf, printing, ctxt, ntotalframes, max) + n = tracebackCgoContext(pcbuf, printing, ctxt, n, max) } } @@ -523,7 +498,7 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i } if printing { - ntotalframes = nprint + n = nprint } // Note that panic != nil is okay here: there can be leftover panics, @@ -566,13 +541,13 @@ func ggentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max i // At other times, such as when gathering a stack for a profiling signal // or when printing a traceback during a crash, everything may not be // stopped nicely, and the stack walk may not be able to complete. - if callback != nil && ntotalframes < max && frame.sp != gp.stktopsp { + if callback != nil && n < max && frame.sp != gp.stktopsp { print("runtime: g", gp.goid, ": frame.sp=", hex(frame.sp), " top=", hex(gp.stktopsp), "\n") - print("\tstack=[", hex(gp.stack.lo), "-", hex(gp.stack.hi), "] n=", ntotalframes, " max=", max, "\n") + print("\tstack=[", hex(gp.stack.lo), "-", hex(gp.stack.hi), "] n=", n, " max=", max, "\n") throw("traceback did not unwind completely") } - return + return n } // reflectMethodValue is a partial duplicate of reflect.makeFuncImpl @@ -737,15 +712,24 @@ func traceback1(pc, sp, lr uintptr, gp *g, flags uint) { printCgoTraceback(&cgoCallers) } + var n int if readgstatus(gp)&^_Gscan == _Gsyscall { // Override registers if blocked in system call. pc = gp.syscallpc sp = gp.syscallsp flags &^= _TraceTrap } - - printtraceback(pc, sp, lr, gp, flags) + // Print traceback. By default, omits runtime frames. + // If that means we print nothing at all, repeat forcing all frames printed. + n = gentraceback(pc, sp, lr, gp, 0, nil, _TracebackMaxFrames, nil, nil, flags) + if n == 0 && (flags&_TraceRuntimeFrames) == 0 { + n = gentraceback(pc, sp, lr, gp, 0, nil, _TracebackMaxFrames, nil, nil, flags|_TraceRuntimeFrames) + } + if n == _TracebackMaxFrames { + print("...additional frames elided...\n") + } printcreatedby(gp) + if gp.ancestors == nil { return } @@ -754,62 +738,6 @@ func traceback1(pc, sp, lr uintptr, gp *g, flags uint) { } } -// countframes traverses the current stacktrace from the top of pc0 to its bottom, excluding runtime frames. -// If flags&_TraceRuntimeframes != 0, it'll include the number of runtime frames in the count. -func countframes(pc0, sp0, lr0 uintptr, gp *g, flags uint) (nframes int) { - ntotalframes, nregularframes := ggentraceback(pc0, sp0, lr0, gp, 0, nil, 1<<31-1, countingframes, nil, nil, flags) - nframes = nregularframes - if flags&_TraceRuntimeFrames != 0 { - nframes = ntotalframes - } - return nframes -} - -func printtraceback(pc, sp, lr uintptr, gp *g, flags uint) { - // We'd like to print: - // * top nMaxFramesPerPrint frames - // * bottom nMaxFramesPerPrint frames. - // See https://golang.org/issue/7181. - - nMaxFramesPerPrint := _TracebackMaxFrames / 2 - nTop := gentraceback(pc, sp, lr, gp, 0, nil, nMaxFramesPerPrint, nil, nil, flags) - if nTop < nMaxFramesPerPrint { - // The common case, in which the traceback has less than nMaxFramesPerPrint. - // By default, omits runtime frames. - // If nTop == 0, it means we printed nothing at all, so repeat, - // and this time force all frames to be printed. - if nTop == 0 && (flags&_TraceRuntimeFrames) == 0 { - // Try again to print the frames, but this time with _TraceRuntimeFrames. - printtraceback(pc, sp, lr, gp, flags|_TraceRuntimeFrames) - } - return - } - - // Figure out the stack size in order to print the bottom max(nMaxFramesPerPrint) frames. - // - // TODO(odeke-em, iant, khr): perhaps investigate and revise the solution in - // https://go-review.googlesource.com/c/go/+/37222/9/src/runtime/traceback.go - // so that we'll always only need 1 stack walk, instead of 2 as in this worst case. - nframes := countframes(pc, sp, lr, gp, flags) - - if nframes <= _TracebackMaxFrames { - // In this case, we'll just print out from where we left off until the end. - gentraceback(pc, sp, lr, gp, nMaxFramesPerPrint /* skip */, nil, 1<<31-1, nil, nil, flags) - return - } - - // Otherwise, now skip until the bottom last nMaxFramesPerPrint. - - // Calculate the number of stack frames to elide since we - // are printing top and bottom each of nMaxFramesPerPrint. - if elide := nframes - _TracebackMaxFrames; elide > 0 { - print("\n... (") - println(elide, "stack frames omitted)\n") - } - skip := nframes - nMaxFramesPerPrint - _ = gentraceback(pc, sp, lr, gp, skip, nil, 1<<31-1 /* max int32 as the biggest frame number */, nil, nil, flags) -} - // printAncestorTraceback prints the traceback of the given ancestor. // TODO: Unify this with gentraceback and CallersFrames. func printAncestorTraceback(ancestor ancestorInfo) { @@ -830,9 +758,11 @@ func printAncestorTraceback(ancestor ancestorInfo) { } } -// fullfuncname retrieves the name for a funcInfo, but if perhaps it was inlined, it'll retrieve -// unwind and retrieve the original name. -func fullfuncname(f funcInfo, pc uintptr) string { +// printAncestorTraceback prints the given function info at a given pc +// within an ancestor traceback. The precision of this info is reduced +// due to only have access to the pcs at the time of the caller +// goroutine being created. +func printAncestorTracebackFuncInfo(f funcInfo, pc uintptr) { name := funcname(f) if inldata := funcdata(f, _FUNCDATA_InlTree); inldata != nil { inltree := (*[1 << 20]inlinedCall)(inldata) @@ -841,15 +771,6 @@ func fullfuncname(f funcInfo, pc uintptr) string { name = funcnameFromNameoff(f, inltree[ix].func_) } } - return name -} - -// printAncestorTraceback prints the given function info at a given pc -// within an ancestor traceback. The precision of this info is reduced -// due to only have access to the pcs at the time of the caller -// goroutine being created. -func printAncestorTracebackFuncInfo(f funcInfo, pc uintptr) { - name := fullfuncname(f, pc) file, line := funcline(f, pc) if name == "runtime.gopanic" { name = "panic" -- cgit v1.3 From e8de596f04d0ea7fb6fb68b036760bf088a9c6c2 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 20 Nov 2020 17:32:46 -0500 Subject: runtime: use inlined function name for traceback elision Currently, gentraceback decides which frames to print or elide when unwinding inlined frames using only the name of the outermost function. If the outermost function should be elided, then inlined functions will also be elided, even if they shouldn't be. This happens in practice in at least one situation. As of CL 258938, exported Go functions (and functions they call) can now be inlined into the generated _cgoexp_HASH_FN function. The runtime elides _cgoexp_HASH_FN from tracebacks because it doesn't contain a ".". Because of this bug, it also elides anything that was inlined into it. This CL fixes this by synthesizing a funcInfo for the inlined functions to pass to showframe. Fixes #42754. Change-Id: Ie6c663a4a1ac7f0d4beb1aa60bc26fc8cddd0f9d Reviewed-on: https://go-review.googlesource.com/c/go/+/272131 Trust: Austin Clements Run-TryBot: Austin Clements TryBot-Result: Go Bot Reviewed-by: Cherry Zhang --- src/runtime/crash_cgo_test.go | 18 +++++++++++ src/runtime/stack_test.go | 41 ++++++++++++++++++++++++++ src/runtime/testdata/testprogcgo/traceback.go | 21 +++++++++++++ src/runtime/testdata/testprogcgo/traceback_c.c | 11 +++++-- src/runtime/traceback.go | 15 ++++++++-- 5 files changed, 102 insertions(+), 4 deletions(-) (limited to 'src/runtime/traceback.go') diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index 0680d07a32..140c170ddc 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -254,6 +254,24 @@ func TestCgoCrashTraceback(t *testing.T) { } } +func TestCgoCrashTracebackGo(t *testing.T) { + t.Parallel() + switch platform := runtime.GOOS + "/" + runtime.GOARCH; platform { + case "darwin/amd64": + case "linux/amd64": + case "linux/ppc64le": + default: + t.Skipf("not yet supported on %s", platform) + } + got := runTestProg(t, "testprogcgo", "CrashTracebackGo") + for i := 1; i <= 3; i++ { + want := fmt.Sprintf("main.h%d", i) + if !strings.Contains(got, want) { + t.Errorf("missing %s", want) + } + } +} + func TestCgoTracebackContext(t *testing.T) { t.Parallel() got := runTestProg(t, "testprogcgo", "TracebackContext") diff --git a/src/runtime/stack_test.go b/src/runtime/stack_test.go index adfc65384a..43fc5cac55 100644 --- a/src/runtime/stack_test.go +++ b/src/runtime/stack_test.go @@ -17,6 +17,7 @@ import ( "sync/atomic" "testing" "time" + _ "unsafe" // for go:linkname ) // TestStackMem measures per-thread stack segment cache behavior. @@ -851,3 +852,43 @@ func deferHeapAndStack(n int) (r int) { // Pass a value to escapeMe to force it to escape. var escapeMe = func(x interface{}) {} + +// Test that when F -> G is inlined and F is excluded from stack +// traces, G still appears. +func TestTracebackInlineExcluded(t *testing.T) { + defer func() { + recover() + buf := make([]byte, 4<<10) + stk := string(buf[:Stack(buf, false)]) + + t.Log(stk) + + if not := "tracebackExcluded"; strings.Contains(stk, not) { + t.Errorf("found but did not expect %q", not) + } + if want := "tracebackNotExcluded"; !strings.Contains(stk, want) { + t.Errorf("expected %q in stack", want) + } + }() + tracebackExcluded() +} + +// tracebackExcluded should be excluded from tracebacks. There are +// various ways this could come up. Linking it to a "runtime." name is +// rather synthetic, but it's easy and reliable. See issue #42754 for +// one way this happened in real code. +// +//go:linkname tracebackExcluded runtime.tracebackExcluded +//go:noinline +func tracebackExcluded() { + // Call an inlined function that should not itself be excluded + // from tracebacks. + tracebackNotExcluded() +} + +// tracebackNotExcluded should be inlined into tracebackExcluded, but +// should not itself be excluded from the traceback. +func tracebackNotExcluded() { + var x *int + *x = 0 +} diff --git a/src/runtime/testdata/testprogcgo/traceback.go b/src/runtime/testdata/testprogcgo/traceback.go index 03de894c89..e2d7599131 100644 --- a/src/runtime/testdata/testprogcgo/traceback.go +++ b/src/runtime/testdata/testprogcgo/traceback.go @@ -12,6 +12,7 @@ package main #cgo CFLAGS: -g -O0 // Defined in traceback_c.c. +extern int crashInGo; int tracebackF1(void); void cgoTraceback(void* parg); void cgoSymbolizer(void* parg); @@ -25,9 +26,29 @@ import ( func init() { register("CrashTraceback", CrashTraceback) + register("CrashTracebackGo", CrashTracebackGo) } func CrashTraceback() { runtime.SetCgoTraceback(0, unsafe.Pointer(C.cgoTraceback), nil, unsafe.Pointer(C.cgoSymbolizer)) C.tracebackF1() } + +func CrashTracebackGo() { + C.crashInGo = 1 + CrashTraceback() +} + +//export h1 +func h1() { + h2() +} + +func h2() { + h3() +} + +func h3() { + var x *int + *x = 0 +} diff --git a/src/runtime/testdata/testprogcgo/traceback_c.c b/src/runtime/testdata/testprogcgo/traceback_c.c index 54f44e11fc..56eda8fa8c 100644 --- a/src/runtime/testdata/testprogcgo/traceback_c.c +++ b/src/runtime/testdata/testprogcgo/traceback_c.c @@ -2,14 +2,21 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// The C definitions for traceback.go. +// The C definitions for traceback.go. That file uses //export so +// it can't put function definitions in the "C" import comment. #include char *p; +int crashInGo; +extern void h1(void); + int tracebackF3(void) { - *p = 0; + if (crashInGo) + h1(); + else + *p = 0; return 0; } diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go index f3df152535..0825e9e707 100644 --- a/src/runtime/traceback.go +++ b/src/runtime/traceback.go @@ -396,13 +396,21 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in // If there is inlining info, print the inner frames. if inldata := funcdata(f, _FUNCDATA_InlTree); inldata != nil { inltree := (*[1 << 20]inlinedCall)(inldata) + var inlFunc _func + inlFuncInfo := funcInfo{&inlFunc, f.datap} for { ix := pcdatavalue(f, _PCDATA_InlTreeIndex, tracepc, nil) if ix < 0 { break } - if (flags&_TraceRuntimeFrames) != 0 || showframe(f, gp, nprint == 0, inltree[ix].funcID, lastFuncID) { - name := funcnameFromNameoff(f, inltree[ix].func_) + + // Create a fake _func for the + // inlined function. + inlFunc.nameoff = inltree[ix].func_ + inlFunc.funcID = inltree[ix].funcID + + if (flags&_TraceRuntimeFrames) != 0 || showframe(inlFuncInfo, gp, nprint == 0, inlFuncInfo.funcID, lastFuncID) { + name := funcname(inlFuncInfo) file, line := funcline(f, tracepc) print(name, "(...)\n") print("\t", file, ":", line, "\n") @@ -811,6 +819,9 @@ func showframe(f funcInfo, gp *g, firstFrame bool, funcID, childID funcID) bool // showfuncinfo reports whether a function with the given characteristics should // be printed during a traceback. func showfuncinfo(f funcInfo, firstFrame bool, funcID, childID funcID) bool { + // Note that f may be a synthesized funcInfo for an inlined + // function, in which case only nameoff and funcID are set. + level, _, _ := gotraceback() if level > 1 { // Show all frames. -- cgit v1.3