From ff2070d9398aff1c44691a90761eb35ea3cd4601 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 17 May 2024 12:07:15 -0400 Subject: [PATCH] runtime: move exit hooks into internal/runtime/exithook This removes a //go:linkname usage in the coverage implementation. For #67401. Change-Id: I0602172c7e372a84465160dbf46d9fa371582fff Reviewed-on: https://go-review.googlesource.com/c/go/+/586259 Auto-Submit: Russ Cox Reviewed-by: Than McIntosh LUCI-TryBot-Result: Go LUCI Reviewed-by: Cherry Mui --- src/cmd/compile/internal/noder/reader.go | 5 +- src/cmd/internal/objabi/pkgspecial.go | 1 + src/go/build/deps_test.go | 2 + src/internal/coverage/cfile/hooks.go | 10 +-- src/internal/runtime/exithook/hooks.go | 64 +++++++++++++++++ src/runtime/exithook.go | 68 ------------------- src/runtime/linkname.go | 3 - src/runtime/proc.go | 7 ++ .../testdata/testexithooks/testexithooks.go | 34 +++++----- 9 files changed, 97 insertions(+), 97 deletions(-) create mode 100644 src/internal/runtime/exithook/hooks.go delete mode 100644 src/runtime/exithook.go diff --git a/src/cmd/compile/internal/noder/reader.go b/src/cmd/compile/internal/noder/reader.go index 042d81bbcd..97865bbfb1 100644 --- a/src/cmd/compile/internal/noder/reader.go +++ b/src/cmd/compile/internal/noder/reader.go @@ -3720,10 +3720,13 @@ func (r *reader) needWrapper(typ *types.Type) { return } + // Special case: runtime must define error even if imported packages mention it (#29304). + forceNeed := typ == types.ErrorType && base.Ctxt.Pkgpath == "runtime" + // If a type was found in an imported package, then we can assume // that package (or one of its transitive dependencies) already // generated method wrappers for it. - if r.importedDef() { + if r.importedDef() && !forceNeed { haveWrapperTypes = append(haveWrapperTypes, typ) } else { needWrapperTypes = append(needWrapperTypes, typ) diff --git a/src/cmd/internal/objabi/pkgspecial.go b/src/cmd/internal/objabi/pkgspecial.go index 2925896bd8..3e99ce9224 100644 --- a/src/cmd/internal/objabi/pkgspecial.go +++ b/src/cmd/internal/objabi/pkgspecial.go @@ -44,6 +44,7 @@ var runtimePkgs = []string{ "runtime", "internal/runtime/atomic", + "internal/runtime/exithook", "runtime/internal/math", "runtime/internal/sys", "internal/runtime/syscall", diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index 1aac76f6a2..503de8f927 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -58,6 +58,7 @@ var depsRules = ` internal/nettrace, internal/platform, internal/profilerecord, + internal/runtime/exithook, internal/trace/traceviewer/format, log/internal, math/bits, @@ -78,6 +79,7 @@ var depsRules = ` internal/goexperiment, internal/goos, internal/profilerecord, + internal/runtime/exithook, math/bits < internal/bytealg < internal/stringslite diff --git a/src/internal/coverage/cfile/hooks.go b/src/internal/coverage/cfile/hooks.go index 003d6ca1e5..3821d1e91e 100644 --- a/src/internal/coverage/cfile/hooks.go +++ b/src/internal/coverage/cfile/hooks.go @@ -4,7 +4,7 @@ package cfile -import _ "unsafe" +import "internal/runtime/exithook" // InitHook is invoked from the main package "init" routine in // programs built with "-cover". This function is intended to be @@ -29,14 +29,10 @@ func InitHook(istest bool) { // Note: hooks are run in reverse registration order, so // register the counter data hook before the meta-data hook // (in the case where two hooks are needed). - runOnNonZeroExit := true - runtime_addExitHook(emitCounterData, runOnNonZeroExit) + exithook.Add(exithook.Hook{F: emitCounterData, RunOnFailure: true}) if istest { - runtime_addExitHook(emitMetaData, runOnNonZeroExit) + exithook.Add(exithook.Hook{F: emitMetaData, RunOnFailure: true}) } else { emitMetaData() } } - -//go:linkname runtime_addExitHook runtime.addExitHook -func runtime_addExitHook(f func(), runOnNonZeroExit bool) diff --git a/src/internal/runtime/exithook/hooks.go b/src/internal/runtime/exithook/hooks.go new file mode 100644 index 0000000000..931154c45d --- /dev/null +++ b/src/internal/runtime/exithook/hooks.go @@ -0,0 +1,64 @@ +// Copyright 2024 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 exithook provides limited support for on-exit cleanup. +// +// CAREFUL! The expectation is that Add should only be called +// from a safe context (e.g. not an error/panic path or signal +// handler, preemption enabled, allocation allowed, write barriers +// allowed, etc), and that the exit function F will be invoked under +// similar circumstances. That is the say, we are expecting that F +// uses normal / high-level Go code as opposed to one of the more +// restricted dialects used for the trickier parts of the runtime. +package exithook + +// A Hook is a function to be run at program termination +// (when someone invokes os.Exit, or when main.main returns). +// Hooks are run in reverse order of registration: +// the first hook added is the last one run. +type Hook struct { + F func() // func to run + RunOnFailure bool // whether to run on non-zero exit code +} + +var ( + hooks []Hook + running bool +) + +// Add adds a new exit hook. +func Add(h Hook) { + hooks = append(hooks, h) +} + +// Run runs the exit hooks. +// It returns an error if Run is already running or +// if one of the hooks panics. +func Run(code int) (err error) { + if running { + return exitError("exit hook invoked exit") + } + running = true + + defer func() { + if x := recover(); x != nil { + err = exitError("exit hook invoked panic") + } + }() + + local := hooks + hooks = nil + for i := len(local) - 1; i >= 0; i-- { + h := local[i] + if code == 0 || h.RunOnFailure { + h.F() + } + } + running = false + return nil +} + +type exitError string + +func (e exitError) Error() string { return string(e) } diff --git a/src/runtime/exithook.go b/src/runtime/exithook.go deleted file mode 100644 index 37d68bd767..0000000000 --- a/src/runtime/exithook.go +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright 2022 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 - -// addExitHook registers the specified function 'f' to be run at -// program termination (e.g. when someone invokes os.Exit(), or when -// main.main returns). Hooks are run in reverse order of registration: -// first hook added is the last one run. -// -// CAREFUL: the expectation is that addExitHook should only be called -// from a safe context (e.g. not an error/panic path or signal -// handler, preemption enabled, allocation allowed, write barriers -// allowed, etc), and that the exit function 'f' will be invoked under -// similar circumstances. That is the say, we are expecting that 'f' -// uses normal / high-level Go code as opposed to one of the more -// restricted dialects used for the trickier parts of the runtime. -func addExitHook(f func(), runOnNonZeroExit bool) { - exitHooks.hooks = append(exitHooks.hooks, exitHook{f: f, runOnNonZeroExit: runOnNonZeroExit}) -} - -// exitHook stores a function to be run on program exit, registered -// by the utility runtime.addExitHook. -type exitHook struct { - f func() // func to run - runOnNonZeroExit bool // whether to run on non-zero exit code -} - -// exitHooks stores state related to hook functions registered to -// run when program execution terminates. -var exitHooks struct { - hooks []exitHook - runningExitHooks bool -} - -// runExitHooks runs any registered exit hook functions (funcs -// previously registered using runtime.addExitHook). Here 'exitCode' -// is the status code being passed to os.Exit, or zero if the program -// is terminating normally without calling os.Exit. -func runExitHooks(exitCode int) { - if exitHooks.runningExitHooks { - throw("internal error: exit hook invoked exit") - } - exitHooks.runningExitHooks = true - - runExitHook := func(f func()) (caughtPanic bool) { - defer func() { - if x := recover(); x != nil { - caughtPanic = true - } - }() - f() - return - } - - for i := range exitHooks.hooks { - h := exitHooks.hooks[len(exitHooks.hooks)-i-1] - if exitCode != 0 && !h.runOnNonZeroExit { - continue - } - if caughtPanic := runExitHook(h.f); caughtPanic { - throw("internal error: exit hook invoked panic") - } - } - exitHooks.hooks = nil - exitHooks.runningExitHooks = false -} diff --git a/src/runtime/linkname.go b/src/runtime/linkname.go index 42d8d245a6..19318cd9a9 100644 --- a/src/runtime/linkname.go +++ b/src/runtime/linkname.go @@ -29,9 +29,6 @@ import _ "unsafe" //go:linkname overflowError //go:linkname divideError -// used in runtime/coverage and in tests -//go:linkname addExitHook - // used in tests //go:linkname extraMInUse //go:linkname getm diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 68296bd1e4..c5bf537a75 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -10,6 +10,7 @@ import ( "internal/goarch" "internal/goos" "internal/runtime/atomic" + "internal/runtime/exithook" "internal/stringslite" "runtime/internal/sys" "unsafe" @@ -309,6 +310,12 @@ func os_beforeExit(exitCode int) { } } +func runExitHooks(code int) { + if err := exithook.Run(code); err != nil { + throw(err.Error()) + } +} + // start forcegc helper goroutine func init() { go forcegchelper() diff --git a/src/runtime/testdata/testexithooks/testexithooks.go b/src/runtime/testdata/testexithooks/testexithooks.go index ceb3326c4f..151b5dc62b 100644 --- a/src/runtime/testdata/testexithooks/testexithooks.go +++ b/src/runtime/testdata/testexithooks/testexithooks.go @@ -7,6 +7,7 @@ package main import ( "flag" "os" + "internal/runtime/exithook" _ "unsafe" ) @@ -30,22 +31,19 @@ func main() { } } -//go:linkname runtime_addExitHook runtime.addExitHook -func runtime_addExitHook(f func(), runOnNonZeroExit bool) - func testSimple() { f1 := func() { println("foo") } f2 := func() { println("bar") } - runtime_addExitHook(f1, false) - runtime_addExitHook(f2, false) + exithook.Add(exithook.Hook{F: f1}) + exithook.Add(exithook.Hook{F: f2}) // no explicit call to os.Exit } func testGoodExit() { f1 := func() { println("apple") } f2 := func() { println("orange") } - runtime_addExitHook(f1, false) - runtime_addExitHook(f2, false) + exithook.Add(exithook.Hook{F: f1}) + exithook.Add(exithook.Hook{F: f2}) // explicit call to os.Exit os.Exit(0) } @@ -56,11 +54,11 @@ func testBadExit() { f3 := func() { println("blek") } f4 := func() { println("blub") } f5 := func() { println("blat") } - runtime_addExitHook(f1, false) - runtime_addExitHook(f2, true) - runtime_addExitHook(f3, false) - runtime_addExitHook(f4, true) - runtime_addExitHook(f5, false) + exithook.Add(exithook.Hook{F: f1}) + exithook.Add(exithook.Hook{F: f2, RunOnFailure: true}) + exithook.Add(exithook.Hook{F: f3}) + exithook.Add(exithook.Hook{F: f4, RunOnFailure: true}) + exithook.Add(exithook.Hook{F: f5}) os.Exit(1) } @@ -68,9 +66,9 @@ func testPanics() { f1 := func() { println("ok") } f2 := func() { panic("BADBADBAD") } f3 := func() { println("good") } - runtime_addExitHook(f1, true) - runtime_addExitHook(f2, true) - runtime_addExitHook(f3, true) + exithook.Add(exithook.Hook{F: f1, RunOnFailure: true}) + exithook.Add(exithook.Hook{F: f2, RunOnFailure: true}) + exithook.Add(exithook.Hook{F: f3, RunOnFailure: true}) os.Exit(0) } @@ -78,8 +76,8 @@ func testHookCallsExit() { f1 := func() { println("ok") } f2 := func() { os.Exit(1) } f3 := func() { println("good") } - runtime_addExitHook(f1, true) - runtime_addExitHook(f2, true) - runtime_addExitHook(f3, true) + exithook.Add(exithook.Hook{F: f1, RunOnFailure: true}) + exithook.Add(exithook.Hook{F: f2, RunOnFailure: true}) + exithook.Add(exithook.Hook{F: f3, RunOnFailure: true}) os.Exit(1) } -- 2.48.1