diff options
| author | Michael Pratt <mpratt@google.com> | 2022-02-25 14:21:25 -0500 |
|---|---|---|
| committer | Michael Pratt <mpratt@google.com> | 2022-04-21 18:06:38 +0000 |
| commit | 342b495301bbd8b75c2721212a08ce41e3f82265 (patch) | |
| tree | 6fce352fb0c644e25a3360f709b8f3a3da69f103 /src/cmd/compile/internal/noder | |
| parent | e25a5ce120673835a7e6c92bf0d88f44a61e9079 (diff) | |
| download | go-342b495301bbd8b75c2721212a08ce41e3f82265.tar.xz | |
cmd/compile: add //go:uintptrkeepalive
This CL exports the existing ir.UintptrKeepAlive via the new directive
//go:uintptrkeepalive. This makes the compiler insert KeepAlives for
pointers converted to uintptr in calls, keeping them alive for the
duration of the call.
//go:uintptrkeepalive requires //go:nosplit, as stack growth can't
handle these arguments (it cannot know which are pointers). We currently
check this on the immediate function, but the actual restriction applies
to all transitive calls.
The existing //go:uintptrescapes is an extension of
//go:uintptrkeepalive which forces pointers to escape to the heap, thus
eliminating the stack growth issue.
This pragma is limited to the standard library.
For #51087
Change-Id: If9a19d484d3561b4219e5539b70c11a3cc09391e
Reviewed-on: https://go-review.googlesource.com/c/go/+/388095
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Diffstat (limited to 'src/cmd/compile/internal/noder')
| -rw-r--r-- | src/cmd/compile/internal/noder/decl.go | 22 | ||||
| -rw-r--r-- | src/cmd/compile/internal/noder/lex.go | 19 | ||||
| -rw-r--r-- | src/cmd/compile/internal/noder/noder.go | 3 | ||||
| -rw-r--r-- | src/cmd/compile/internal/noder/writer.go | 16 |
4 files changed, 46 insertions, 14 deletions
diff --git a/src/cmd/compile/internal/noder/decl.go b/src/cmd/compile/internal/noder/decl.go index f985648c66..91a90d9e09 100644 --- a/src/cmd/compile/internal/noder/decl.go +++ b/src/cmd/compile/internal/noder/decl.go @@ -125,8 +125,26 @@ func (g *irgen) funcDecl(out *ir.Nodes, decl *syntax.FuncDecl) { } } - if decl.Body != nil && fn.Pragma&ir.Noescape != 0 { - base.ErrorfAt(fn.Pos(), "can only use //go:noescape with external func implementations") + if decl.Body != nil { + if fn.Pragma&ir.Noescape != 0 { + base.ErrorfAt(fn.Pos(), "can only use //go:noescape with external func implementations") + } + if (fn.Pragma&ir.UintptrKeepAlive != 0 && fn.Pragma&ir.UintptrEscapes == 0) && fn.Pragma&ir.Nosplit == 0 { + // Stack growth can't handle uintptr arguments that may + // be pointers (as we don't know which are pointers + // when creating the stack map). Thus uintptrkeepalive + // functions (and all transitive callees) must be + // nosplit. + // + // N.B. uintptrescapes implies uintptrkeepalive but it + // is OK since the arguments must escape to the heap. + // + // TODO(prattmic): Add recursive nosplit check of callees. + // TODO(prattmic): Functions with no body (i.e., + // assembly) must also be nosplit, but we can't check + // that here. + base.ErrorfAt(fn.Pos(), "go:uintptrkeepalive requires go:nosplit") + } } if decl.Name.Value == "init" && decl.Recv == nil { diff --git a/src/cmd/compile/internal/noder/lex.go b/src/cmd/compile/internal/noder/lex.go index 66a56a50ec..cef0f082ca 100644 --- a/src/cmd/compile/internal/noder/lex.go +++ b/src/cmd/compile/internal/noder/lex.go @@ -30,6 +30,7 @@ const ( ir.NoCheckPtr | ir.RegisterParams | // TODO(register args) remove after register abi is working ir.CgoUnsafeArgs | + ir.UintptrKeepAlive | ir.UintptrEscapes | ir.Systemstack | ir.Nowritebarrier | @@ -67,19 +68,13 @@ func pragmaFlag(verb string) ir.PragmaFlag { return ir.Yeswritebarrierrec case "go:cgo_unsafe_args": return ir.CgoUnsafeArgs | ir.NoCheckPtr // implies NoCheckPtr (see #34968) + case "go:uintptrkeepalive": + return ir.UintptrKeepAlive case "go:uintptrescapes": - // For the next function declared in the file - // any uintptr arguments may be pointer values - // converted to uintptr. This directive - // ensures that the referenced allocated - // object, if any, is retained and not moved - // until the call completes, even though from - // the types alone it would appear that the - // object is no longer needed during the - // call. The conversion to uintptr must appear - // in the argument list. - // Used in syscall/dll_windows.go. - return ir.UintptrEscapes + // This directive extends //go:uintptrkeepalive by forcing + // uintptr arguments to escape to the heap, which makes stack + // growth safe. + return ir.UintptrEscapes | ir.UintptrKeepAlive // implies UintptrKeepAlive case "go:registerparams": // TODO(register args) remove after register abi is working return ir.RegisterParams case "go:notinheap": diff --git a/src/cmd/compile/internal/noder/noder.go b/src/cmd/compile/internal/noder/noder.go index cc5610acda..9a42b5afd1 100644 --- a/src/cmd/compile/internal/noder/noder.go +++ b/src/cmd/compile/internal/noder/noder.go @@ -340,6 +340,9 @@ func (p *noder) pragma(pos syntax.Pos, blankLine bool, text string, old syntax.P if !base.Flag.CompilingRuntime && flag&runtimePragmas != 0 { p.error(syntax.Error{Pos: pos, Msg: fmt.Sprintf("//%s only allowed in runtime", verb)}) } + if flag == ir.UintptrKeepAlive && !base.Flag.Std { + p.error(syntax.Error{Pos: pos, Msg: fmt.Sprintf("//%s is only allowed in the standard library", verb)}) + } if flag == 0 && !allowedStdPragmas[verb] && base.Flag.Std { p.error(syntax.Error{Pos: pos, Msg: fmt.Sprintf("//%s is not allowed in the standard library", verb)}) } diff --git a/src/cmd/compile/internal/noder/writer.go b/src/cmd/compile/internal/noder/writer.go index c5c346b784..0fb162d381 100644 --- a/src/cmd/compile/internal/noder/writer.go +++ b/src/cmd/compile/internal/noder/writer.go @@ -742,6 +742,22 @@ func (w *writer) funcExt(obj *types2.Func) { if pragma&ir.Noescape != 0 { w.p.errorf(decl, "can only use //go:noescape with external func implementations") } + if (pragma&ir.UintptrKeepAlive != 0 && pragma&ir.UintptrEscapes == 0) && pragma&ir.Nosplit == 0 { + // Stack growth can't handle uintptr arguments that may + // be pointers (as we don't know which are pointers + // when creating the stack map). Thus uintptrkeepalive + // functions (and all transitive callees) must be + // nosplit. + // + // N.B. uintptrescapes implies uintptrkeepalive but it + // is OK since the arguments must escape to the heap. + // + // TODO(prattmic): Add recursive nosplit check of callees. + // TODO(prattmic): Functions with no body (i.e., + // assembly) must also be nosplit, but we can't check + // that here. + w.p.errorf(decl, "go:uintptrkeepalive requires go:nosplit") + } } else { if base.Flag.Complete || decl.Name.Value == "init" { // Linknamed functions are allowed to have no body. Hopefully |
