diff options
| -rw-r--r-- | api/next/25448.txt | 3 | ||||
| -rw-r--r-- | doc/go_spec.html | 20 | ||||
| -rw-r--r-- | src/builtin/builtin.go | 4 | ||||
| -rw-r--r-- | src/net/http/clientserver_test.go | 4 | ||||
| -rw-r--r-- | src/runtime/panic.go | 21 | ||||
| -rw-r--r-- | src/runtime/panicnil_test.go | 36 | ||||
| -rw-r--r-- | src/runtime/runtime.go | 22 | ||||
| -rw-r--r-- | src/runtime/runtime1.go | 159 | ||||
| -rw-r--r-- | test/fixedbugs/issue19658.go | 5 |
9 files changed, 207 insertions, 67 deletions
diff --git a/api/next/25448.txt b/api/next/25448.txt new file mode 100644 index 0000000000..1b8901710c --- /dev/null +++ b/api/next/25448.txt @@ -0,0 +1,3 @@ +pkg runtime, method (*PanicNilError) Error() string #25448 +pkg runtime, method (*PanicNilError) RuntimeError() #25448 +pkg runtime, type PanicNilError struct #25448 diff --git a/doc/go_spec.html b/doc/go_spec.html index f93f2ab9f1..9f0cbb09dc 100644 --- a/doc/go_spec.html +++ b/doc/go_spec.html @@ -1,6 +1,6 @@ <!--{ "Title": "The Go Programming Language Specification", - "Subtitle": "Version of December 15, 2022", + "Subtitle": "Version of January 19, 2022", "Path": "/ref/spec" }--> @@ -7574,19 +7574,13 @@ execution terminates by returning to its caller. </p> <p> -The return value of <code>recover</code> is <code>nil</code> if any of the following conditions holds: +The return value of <code>recover</code> is <code>nil</code> when the +goroutine is not panicking or <code>recover</code> was not called directly by a deferred function. +Conversely, if a goroutine is panicking and <code>recover</code> was called directly by a deferred function, +the return value of <code>recover</code> is guaranteed not to be <code>nil</code>. +To ensure this, calling <code>panic</code> with a <code>nil</code> interface value (or an untyped <code>nil</code>) +causes a <a href="#Run_time_panics">run-time panic</a>. </p> -<ul> -<li> -<code>panic</code>'s argument was <code>nil</code>; -</li> -<li> -the goroutine is not panicking; -</li> -<li> -<code>recover</code> was not called directly by a deferred function. -</li> -</ul> <p> The <code>protect</code> function in the example below invokes diff --git a/src/builtin/builtin.go b/src/builtin/builtin.go index 7feb209bb4..d3637584fe 100644 --- a/src/builtin/builtin.go +++ b/src/builtin/builtin.go @@ -249,6 +249,10 @@ func close(c chan<- Type) // that point, the program is terminated with a non-zero exit code. This // termination sequence is called panicking and can be controlled by the // built-in function recover. +// +// Starting in Go 1.21, calling panic with a nil interface value or an +// untyped nil causes a run-time error (a different panic). +// The GODEBUG setting panicnil=1 disables the run-time error. func panic(v any) // The recover built-in function allows a program to manage behavior of a diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go index da5671d9b9..e49bed113a 100644 --- a/src/net/http/clientserver_test.go +++ b/src/net/http/clientserver_test.go @@ -1239,9 +1239,9 @@ func testTransportRejectsInvalidHeaders(t *testing.T, mode testMode) { func TestInterruptWithPanic(t *testing.T) { run(t, func(t *testing.T, mode testMode) { t.Run("boom", func(t *testing.T) { testInterruptWithPanic(t, mode, "boom") }) - t.Run("nil", func(t *testing.T) { testInterruptWithPanic(t, mode, nil) }) + t.Run("nil", func(t *testing.T) { t.Setenv("GODEBUG", "panicnil=1"); testInterruptWithPanic(t, mode, nil) }) t.Run("ErrAbortHandler", func(t *testing.T) { testInterruptWithPanic(t, mode, ErrAbortHandler) }) - }) + }, testNotParallel) } func testInterruptWithPanic(t *testing.T, mode testMode, panicValue any) { const msg = "hello" diff --git a/src/runtime/panic.go b/src/runtime/panic.go index 26618db7ce..905515f822 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -800,8 +800,29 @@ func deferCallSave(p *_panic, fn func()) { } } +// A PanicNilError happens when code calls panic(nil). +// +// Before Go 1.21, programs that called panic(nil) observed recover returning nil. +// Starting in Go 1.21, programs that call panic(nil) observe recover returning a *PanicNilError. +// Programs can change back to the old behavior by setting GODEBUG=panicnil=1. +type PanicNilError struct { + // This field makes PanicNilError structurally different from + // any other struct in this package, and the _ makes it different + // from any struct in other packages too. + // This avoids any accidental conversions being possible + // between this struct and some other struct sharing the same fields, + // like happened in go.dev/issue/56603. + _ [0]*PanicNilError +} + +func (*PanicNilError) Error() string { return "panic called with nil argument" } +func (*PanicNilError) RuntimeError() {} + // The implementation of the predeclared function panic. func gopanic(e any) { + if e == nil && debug.panicnil.Load() != 1 { + e = new(PanicNilError) + } gp := getg() if gp.m.curg != gp { print("panic: ") diff --git a/src/runtime/panicnil_test.go b/src/runtime/panicnil_test.go new file mode 100644 index 0000000000..441bef3b07 --- /dev/null +++ b/src/runtime/panicnil_test.go @@ -0,0 +1,36 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package runtime_test + +import ( + "reflect" + "runtime" + "testing" +) + +func TestPanicNil(t *testing.T) { + t.Run("default", func(t *testing.T) { + checkPanicNil(t, new(runtime.PanicNilError)) + }) + t.Run("GODEBUG=panicnil=0", func(t *testing.T) { + t.Setenv("GODEBUG", "panicnil=0") + checkPanicNil(t, new(runtime.PanicNilError)) + }) + t.Run("GODEBUG=panicnil=1", func(t *testing.T) { + t.Setenv("GODEBUG", "panicnil=1") + checkPanicNil(t, nil) + }) +} + +func checkPanicNil(t *testing.T, want any) { + defer func() { + e := recover() + if reflect.TypeOf(e) != reflect.TypeOf(want) { + println(e, want) + t.Errorf("recover() = %v, want %v", e, want) + } + }() + panic(nil) +} diff --git a/src/runtime/runtime.go b/src/runtime/runtime.go index 9f68738aa7..ab2a54f00b 100644 --- a/src/runtime/runtime.go +++ b/src/runtime/runtime.go @@ -75,15 +75,19 @@ func godebug_setUpdate(update func(string, string)) { p := new(func(string, string)) *p = update godebugUpdate.Store(p) - godebugNotify() + godebugNotify(false) } -func godebugNotify() { - if update := godebugUpdate.Load(); update != nil { - var env string - if p := godebugEnv.Load(); p != nil { - env = *p - } +func godebugNotify(envChanged bool) { + update := godebugUpdate.Load() + var env string + if p := godebugEnv.Load(); p != nil { + env = *p + } + if envChanged { + reparsedebugvars(env) + } + if update != nil { (*update)(godebugDefault, env) } } @@ -95,7 +99,7 @@ func syscall_runtimeSetenv(key, value string) { p := new(string) *p = value godebugEnv.Store(p) - godebugNotify() + godebugNotify(true) } } @@ -104,7 +108,7 @@ func syscall_runtimeUnsetenv(key string) { unsetenv_c(key) if key == "GODEBUG" { godebugEnv.Store(nil) - godebugNotify() + godebugNotify(true) } } diff --git a/src/runtime/runtime1.go b/src/runtime/runtime1.go index 277f18a5a6..5f9555e404 100644 --- a/src/runtime/runtime1.go +++ b/src/runtime/runtime1.go @@ -296,8 +296,10 @@ func check() { } type dbgVar struct { - name string - value *int32 + name string + value *int32 // for variables that can only be set at startup + atomic *atomic.Int32 // for variables that can be changed during execution + def int32 // default value (ideally zero) } // Holds variables parsed from GODEBUG env var, @@ -330,32 +332,33 @@ var debug struct { allocfreetrace int32 inittrace int32 sbrk int32 -} -var dbgvars = []dbgVar{ - {"allocfreetrace", &debug.allocfreetrace}, - {"clobberfree", &debug.clobberfree}, - {"cgocheck", &debug.cgocheck}, - {"efence", &debug.efence}, - {"gccheckmark", &debug.gccheckmark}, - {"gcpacertrace", &debug.gcpacertrace}, - {"gcshrinkstackoff", &debug.gcshrinkstackoff}, - {"gcstoptheworld", &debug.gcstoptheworld}, - {"gctrace", &debug.gctrace}, - {"invalidptr", &debug.invalidptr}, - {"madvdontneed", &debug.madvdontneed}, - {"sbrk", &debug.sbrk}, - {"scavtrace", &debug.scavtrace}, - {"scheddetail", &debug.scheddetail}, - {"schedtrace", &debug.schedtrace}, - {"tracebackancestors", &debug.tracebackancestors}, - {"asyncpreemptoff", &debug.asyncpreemptoff}, - {"inittrace", &debug.inittrace}, - {"harddecommit", &debug.harddecommit}, - {"adaptivestackstart", &debug.adaptivestackstart}, + panicnil atomic.Int32 } -var globalGODEBUG string +var dbgvars = []*dbgVar{ + {name: "allocfreetrace", value: &debug.allocfreetrace}, + {name: "clobberfree", value: &debug.clobberfree}, + {name: "cgocheck", value: &debug.cgocheck}, + {name: "efence", value: &debug.efence}, + {name: "gccheckmark", value: &debug.gccheckmark}, + {name: "gcpacertrace", value: &debug.gcpacertrace}, + {name: "gcshrinkstackoff", value: &debug.gcshrinkstackoff}, + {name: "gcstoptheworld", value: &debug.gcstoptheworld}, + {name: "gctrace", value: &debug.gctrace}, + {name: "invalidptr", value: &debug.invalidptr}, + {name: "madvdontneed", value: &debug.madvdontneed}, + {name: "sbrk", value: &debug.sbrk}, + {name: "scavtrace", value: &debug.scavtrace}, + {name: "scheddetail", value: &debug.scheddetail}, + {name: "schedtrace", value: &debug.schedtrace}, + {name: "tracebackancestors", value: &debug.tracebackancestors}, + {name: "asyncpreemptoff", value: &debug.asyncpreemptoff}, + {name: "inittrace", value: &debug.inittrace}, + {name: "harddecommit", value: &debug.harddecommit}, + {name: "adaptivestackstart", value: &debug.adaptivestackstart}, + {name: "panicnil", atomic: &debug.panicnil}, +} func parsedebugvars() { // defaults @@ -374,26 +377,101 @@ func parsedebugvars() { debug.madvdontneed = 1 } - globalGODEBUG = gogetenv("GODEBUG") - godebugEnv.StoreNoWB(&globalGODEBUG) - for p := globalGODEBUG; p != ""; { - field := "" - i := bytealg.IndexByteString(p, ',') - if i < 0 { - field, p = p, "" + godebug := gogetenv("GODEBUG") + + p := new(string) + *p = godebug + godebugEnv.Store(p) + + // apply runtime defaults, if any + for _, v := range dbgvars { + if v.def != 0 { + // Every var should have either v.value or v.atomic set. + if v.value != nil { + *v.value = v.def + } else if v.atomic != nil { + v.atomic.Store(v.def) + } + } + } + + // apply compile-time GODEBUG settings + parsegodebug(godebugDefault, nil) + + // apply environment settings + parsegodebug(godebug, nil) + + debug.malloc = (debug.allocfreetrace | debug.inittrace | debug.sbrk) != 0 + + setTraceback(gogetenv("GOTRACEBACK")) + traceback_env = traceback_cache +} + +// reparsedebugvars reparses the runtime's debug variables +// because the environment variable has been changed to env. +func reparsedebugvars(env string) { + seen := make(map[string]bool) + // apply environment settings + parsegodebug(env, seen) + // apply compile-time GODEBUG settings for as-yet-unseen variables + parsegodebug(godebugDefault, seen) + // apply defaults for as-yet-unseen variables + for _, v := range dbgvars { + if v.atomic != nil && !seen[v.name] { + v.atomic.Store(0) + } + } +} + +// parsegodebug parses the godebug string, updating variables listed in dbgvars. +// If seen == nil, this is startup time and we process the string left to right +// overwriting older settings with newer ones. +// If seen != nil, $GODEBUG has changed and we are doing an +// incremental update. To avoid flapping in the case where a value is +// set multiple times (perhaps in the default and the environment, +// or perhaps twice in the environment), we process the string right-to-left +// and only change values not already seen. After doing this for both +// the environment and the default settings, the caller must also call +// cleargodebug(seen) to reset any now-unset values back to their defaults. +func parsegodebug(godebug string, seen map[string]bool) { + for p := godebug; p != ""; { + var field string + if seen == nil { + // startup: process left to right, overwriting older settings with newer + i := bytealg.IndexByteString(p, ',') + if i < 0 { + field, p = p, "" + } else { + field, p = p[:i], p[i+1:] + } } else { - field, p = p[:i], p[i+1:] + // incremental update: process right to left, updating and skipping seen + i := len(p) - 1 + for i >= 0 && p[i] != ',' { + i-- + } + if i < 0 { + p, field = "", p + } else { + p, field = p[:i], p[i+1:] + } } - i = bytealg.IndexByteString(field, '=') + i := bytealg.IndexByteString(field, '=') if i < 0 { continue } key, value := field[:i], field[i+1:] + if seen[key] { + continue + } + if seen != nil { + seen[key] = true + } // Update MemProfileRate directly here since it // is int, not int32, and should only be updated // if specified in GODEBUG. - if key == "memprofilerate" { + if seen == nil && key == "memprofilerate" { if n, ok := atoi(value); ok { MemProfileRate = n } @@ -401,17 +479,16 @@ func parsedebugvars() { for _, v := range dbgvars { if v.name == key { if n, ok := atoi32(value); ok { - *v.value = n + if seen == nil && v.value != nil { + *v.value = n + } else if v.atomic != nil { + v.atomic.Store(n) + } } } } } } - - debug.malloc = (debug.allocfreetrace | debug.inittrace | debug.sbrk) != 0 - - setTraceback(gogetenv("GOTRACEBACK")) - traceback_env = traceback_cache } //go:linkname setTraceback runtime/debug.SetTraceback diff --git a/test/fixedbugs/issue19658.go b/test/fixedbugs/issue19658.go index bab409c6c0..70fa3a65c3 100644 --- a/test/fixedbugs/issue19658.go +++ b/test/fixedbugs/issue19658.go @@ -1,5 +1,5 @@ -// +build !nacl,!js,!gccgo // run +//go:build !nacl && !js && !gccgo // Copyright 2017 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style @@ -46,7 +46,8 @@ func main() { Type string Input string Expect string - }{{"", "nil", "panic: nil"}, + }{ + {"", "nil", "panic: panic called with nil argument"}, {"errors.New", `"test"`, "panic: test"}, {"S", "S{}", "panic: s-stringer"}, {"byte", "8", "panic: 8"}, |