Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit b1d1ec9

Browse files
author
Elias Naur
committedApr 30, 2018
runtime: perform crashes outside systemstack
CL 93658 moved stack trace printing inside a systemstack call to sidestep complexity in case the runtime is in a inconsistent state. Unfortunately, debuggers generating backtraces for a Go panic will be confused and come up with a technical correct but useless stack. This CL moves just the crash performing - typically a SIGABRT signal - outside the systemstack call to improve backtraces. Unfortunately, the crash function now needs to be marked nosplit and that triggers the no split stackoverflow check. To work around that, split fatalpanic in two: fatalthrow for runtime.throw and fatalpanic for runtime.gopanic. Only Go panics really needs crashes on the right stack and there is enough stack for gopanic. Example program: package main import "runtime/debug" func main() { debug.SetTraceback("crash") crash() } func crash() { panic("panic!") } Before: (lldb) bt * thread #1, name = 'simple', stop reason = signal SIGABRT * frame #0: 0x000000000044ffe4 simple`runtime.raise at <autogenerated>:1 frame #1: 0x0000000000438cfb simple`runtime.dieFromSignal(sig=<unavailable>) at signal_unix.go:424 frame #2: 0x0000000000438ec9 simple`runtime.crash at signal_unix.go:525 frame #3: 0x00000000004268f5 simple`runtime.dopanic_m(gp=<unavailable>, pc=<unavailable>, sp=<unavailable>) at panic.go:758 frame #4: 0x000000000044bead simple`runtime.fatalpanic.func1 at panic.go:657 frame #5: 0x000000000044d066 simple`runtime.systemstack at <autogenerated>:1 frame #6: 0x000000000042a980 simple at proc.go:1094 frame #7: 0x0000000000438ec9 simple`runtime.crash at signal_unix.go:525 frame #8: 0x00000000004268f5 simple`runtime.dopanic_m(gp=<unavailable>, pc=<unavailable>, sp=<unavailable>) at panic.go:758 frame #9: 0x000000000044bead simple`runtime.fatalpanic.func1 at panic.go:657 frame #10: 0x000000000044d066 simple`runtime.systemstack at <autogenerated>:1 frame #11: 0x000000000042a980 simple at proc.go:1094 frame #12: 0x00000000004268f5 simple`runtime.dopanic_m(gp=<unavailable>, pc=<unavailable>, sp=<unavailable>) at panic.go:758 frame #13: 0x000000000044bead simple`runtime.fatalpanic.func1 at panic.go:657 frame #14: 0x000000000044d066 simple`runtime.systemstack at <autogenerated>:1 frame #15: 0x000000000042a980 simple at proc.go:1094 frame #16: 0x000000000044bead simple`runtime.fatalpanic.func1 at panic.go:657 frame #17: 0x000000000044d066 simple`runtime.systemstack at <autogenerated>:1 After: (lldb) bt * thread #7, stop reason = signal SIGABRT * frame #0: 0x0000000000450024 simple`runtime.raise at <autogenerated>:1 frame #1: 0x0000000000438d1b simple`runtime.dieFromSignal(sig=<unavailable>) at signal_unix.go:424 frame #2: 0x0000000000438ee9 simple`runtime.crash at signal_unix.go:525 frame #3: 0x00000000004264e3 simple`runtime.fatalpanic(msgs=<unavailable>) at panic.go:664 frame #4: 0x0000000000425f1b simple`runtime.gopanic(e=<unavailable>) at panic.go:537 frame #5: 0x0000000000470c62 simple`main.crash at simple.go:11 frame #6: 0x0000000000470c00 simple`main.main at simple.go:6 frame #7: 0x0000000000427be7 simple`runtime.main at proc.go:198 frame #8: 0x000000000044ef91 simple`runtime.goexit at <autogenerated>:1 Updates #22716 Change-Id: Ib5fa35c13662c1dac2f1eac8b59c4a5824b98d92 Reviewed-on: https://go-review.googlesource.com/110065 Run-TryBot: Elias Naur <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Austin Clements <[email protected]>
1 parent c789ce3 commit b1d1ec9

File tree

6 files changed

+116
-13
lines changed

6 files changed

+116
-13
lines changed
 

‎src/runtime/os_nacl.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ func signame(sig uint32) string {
131131
return sigtable[sig].name
132132
}
133133

134+
//go:nosplit
134135
func crash() {
135136
*(*int32)(nil) = 0
136137
}

‎src/runtime/os_plan9.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ func osinit() {
296296
notify(unsafe.Pointer(funcPC(sigtramp)))
297297
}
298298

299+
//go:nosplit
299300
func crash() {
300301
notify(nil)
301302
*(*int)(nil) = 0

‎src/runtime/panic.go

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ func throw(s string) {
586586
if gp.m.throwing == 0 {
587587
gp.m.throwing = 1
588588
}
589-
fatalpanic(nil)
589+
fatalthrow()
590590
*(*int)(nil) = 0 // not reached
591591
}
592592

@@ -627,18 +627,43 @@ func recovery(gp *g) {
627627
gogo(&gp.sched)
628628
}
629629

630-
// fatalpanic implements an unrecoverable panic. It freezes the
631-
// system, prints panic messages if msgs != nil, prints stack traces
632-
// starting from its caller, and terminates the process.
630+
// fatalthrow implements an unrecoverable runtime throw. It freezes the
631+
// system, prints stack traces starting from its caller, and terminates the
632+
// process.
633633
//
634-
// If msgs != nil, it also decrements runningPanicDefers once main is
635-
// blocked from exiting.
634+
//go:nosplit
635+
func fatalthrow() {
636+
pc := getcallerpc()
637+
sp := getcallersp()
638+
gp := getg()
639+
// Switch to the system stack to avoid any stack growth, which
640+
// may make things worse if the runtime is in a bad state.
641+
systemstack(func() {
642+
startpanic_m()
643+
644+
if dopanic_m(gp, pc, sp) {
645+
// crash uses a decent amount of nosplit stack and we're already
646+
// low on stack in throw, so crash on the system stack (unlike
647+
// fatalpanic).
648+
crash()
649+
}
650+
651+
exit(2)
652+
})
653+
654+
*(*int)(nil) = 0 // not reached
655+
}
656+
657+
// fatalpanic implements an unrecoverable panic. It is like fatalthrow, except
658+
// that if msgs != nil, fatalpanic also prints panic messages and decrements
659+
// runningPanicDefers once main is blocked from exiting.
636660
//
637661
//go:nosplit
638662
func fatalpanic(msgs *_panic) {
639663
pc := getcallerpc()
640664
sp := getcallersp()
641665
gp := getg()
666+
var docrash bool
642667
// Switch to the system stack to avoid any stack growth, which
643668
// may make things worse if the runtime is in a bad state.
644669
systemstack(func() {
@@ -654,8 +679,20 @@ func fatalpanic(msgs *_panic) {
654679
printpanics(msgs)
655680
}
656681

657-
dopanic_m(gp, pc, sp) // should never return
682+
docrash = dopanic_m(gp, pc, sp)
658683
})
684+
685+
if docrash {
686+
// By crashing outside the above systemstack call, debuggers
687+
// will not be confused when generating a backtrace.
688+
// Function crash is marked nosplit to avoid stack growth.
689+
crash()
690+
}
691+
692+
systemstack(func() {
693+
exit(2)
694+
})
695+
659696
*(*int)(nil) = 0 // not reached
660697
}
661698

@@ -713,7 +750,7 @@ func startpanic_m() bool {
713750
var didothers bool
714751
var deadlock mutex
715752

716-
func dopanic_m(gp *g, pc, sp uintptr) {
753+
func dopanic_m(gp *g, pc, sp uintptr) bool {
717754
if gp.sig != 0 {
718755
signame := signame(gp.sig)
719756
if signame != "" {
@@ -754,11 +791,7 @@ func dopanic_m(gp *g, pc, sp uintptr) {
754791
lock(&deadlock)
755792
}
756793

757-
if docrash {
758-
crash()
759-
}
760-
761-
exit(2)
794+
return docrash
762795
}
763796

764797
// canpanic returns false if a signal should throw instead of

‎src/runtime/runtime-gdb_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,3 +478,69 @@ func TestGdbConst(t *testing.T) {
478478
t.Fatalf("output mismatch")
479479
}
480480
}
481+
482+
const panicSource = `
483+
package main
484+
485+
import "runtime/debug"
486+
487+
func main() {
488+
debug.SetTraceback("crash")
489+
crash()
490+
}
491+
492+
func crash() {
493+
panic("panic!")
494+
}
495+
`
496+
497+
// TestGdbPanic tests that gdb can unwind the stack correctly
498+
// from SIGABRTs from Go panics.
499+
func TestGdbPanic(t *testing.T) {
500+
checkGdbEnvironment(t)
501+
t.Parallel()
502+
checkGdbVersion(t)
503+
504+
dir, err := ioutil.TempDir("", "go-build")
505+
if err != nil {
506+
t.Fatalf("failed to create temp directory: %v", err)
507+
}
508+
defer os.RemoveAll(dir)
509+
510+
// Build the source code.
511+
src := filepath.Join(dir, "main.go")
512+
err = ioutil.WriteFile(src, []byte(panicSource), 0644)
513+
if err != nil {
514+
t.Fatalf("failed to create file: %v", err)
515+
}
516+
cmd := exec.Command(testenv.GoToolPath(t), "build", "-o", "a.exe")
517+
cmd.Dir = dir
518+
out, err := testenv.CleanCmdEnv(cmd).CombinedOutput()
519+
if err != nil {
520+
t.Fatalf("building source %v\n%s", err, out)
521+
}
522+
523+
// Execute gdb commands.
524+
args := []string{"-nx", "-batch",
525+
"-iex", "add-auto-load-safe-path " + filepath.Join(runtime.GOROOT(), "src", "runtime"),
526+
"-ex", "set startup-with-shell off",
527+
"-ex", "run",
528+
"-ex", "backtrace",
529+
filepath.Join(dir, "a.exe"),
530+
}
531+
got, _ := exec.Command("gdb", args...).CombinedOutput()
532+
533+
// Check that the backtrace matches the source code.
534+
bt := []string{
535+
`crash`,
536+
`main`,
537+
}
538+
for _, name := range bt {
539+
s := fmt.Sprintf("#.* .* in main\\.%v", name)
540+
re := regexp.MustCompile(s)
541+
if found := re.Find(got) != nil; !found {
542+
t.Errorf("could not find '%v' in backtrace", s)
543+
t.Fatalf("gdb output:\n%v", string(got))
544+
}
545+
}
546+
}

‎src/runtime/signal_unix.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,7 @@ func raisebadsignal(sig uint32, c *sigctxt) {
509509
setsig(sig, funcPC(sighandler))
510510
}
511511

512+
//go:nosplit
512513
func crash() {
513514
if GOOS == "darwin" {
514515
// OS X core dumps are linear dumps of the mapped memory,

‎src/runtime/signal_windows.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ func signame(sig uint32) string {
224224
return ""
225225
}
226226

227+
//go:nosplit
227228
func crash() {
228229
// TODO: This routine should do whatever is needed
229230
// to make the Windows program abort/crash as it

0 commit comments

Comments
 (0)
Please sign in to comment.