From 42fcc506d6a7681ef24ac36a5904b57bda4b15cd Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Fri, 16 Aug 2024 15:26:24 +0800 Subject: [PATCH] address comments. Signed-off-by: doujiang24 --- src/cmd/cgo/doc.go | 2 +- src/cmd/cgo/out.go | 4 ++-- src/runtime/cgo.go | 9 +++++---- src/runtime/testdata/testprogcgo/issue63739.go | 10 +++++----- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/cmd/cgo/doc.go b/src/cmd/cgo/doc.go index 35601be47c3c8..fc6e6d98b7500 100644 --- a/src/cmd/cgo/doc.go +++ b/src/cmd/cgo/doc.go @@ -436,7 +436,7 @@ For example: // #cgo noescape cFunctionName When a Go function calls a C function, it prepares for the C function to -call back to a Go function. the #cgo nocallback directive may be used to +call back to a Go function. The #cgo nocallback directive may be used to tell the compiler that these preparations are not necessary. If the nocallback directive is used and the C function does call back into Go code, the program will panic. diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go index 75b8d9245e422..05cd19251a0dd 100644 --- a/src/cmd/cgo/out.go +++ b/src/cmd/cgo/out.go @@ -644,9 +644,9 @@ func (p *Package) writeDefsFunc(fgo2 io.Writer, n *Name, callsMalloc *bool) { fmt.Fprintf(fgo2, "\t_Cgo_no_callback(false)\n") } - // use _Cgo_keepalive instead of _Cgo_use when noescape & nocallback exist, + // Use _Cgo_keepalive instead of _Cgo_use when noescape & nocallback exist, // so that the compiler won't force to escape them to heap. - // instead, make the compiler keep them alive by using _Cgo_keepalive. + // Instead, make the compiler keep them alive by using _Cgo_keepalive. touchFunc := "_Cgo_use" if p.noEscapes[n.C] && p.noCallbacks[n.C] { touchFunc = "_Cgo_keepalive" diff --git a/src/runtime/cgo.go b/src/runtime/cgo.go index 1cbece8bc1445..eca905bad9515 100644 --- a/src/runtime/cgo.go +++ b/src/runtime/cgo.go @@ -73,10 +73,11 @@ var cgoHasExtraM bool func cgoUse(any) { throw("cgoUse should not be called") } // cgoKeepAlive is called by cgo-generated code (using go:linkname to get at -// an unexported name). Unlike cgoUse The calls serve one purposes: -// 1) they keep the argument alive until the call site; the call is emitted after -// the end of the (presumed) use of the argument by C. -// cgoKeepAlive should not actually be called (see cgoAlwaysFalse). +// an unexported name). This call keeps its argument alive until the call site; +// cgo emits the call after the last possible use of the argument by C code. +// cgoKeepAlive is marked in the cgo-generated code as //go:noescape, so +// unlike cgoUse it does not force the argument to escape to the heap. +// This is used to implement the #cgo noescape directive. func cgoKeepAlive(any) { throw("cgoKeepAlive should not be called") } // cgoAlwaysFalse is a boolean value that is always false. diff --git a/src/runtime/testdata/testprogcgo/issue63739.go b/src/runtime/testdata/testprogcgo/issue63739.go index 74dbb044f09df..dbe37b6d0e732 100644 --- a/src/runtime/testdata/testprogcgo/issue63739.go +++ b/src/runtime/testdata/testprogcgo/issue63739.go @@ -4,11 +4,11 @@ package main -// This is for issue #56378. -// After we retiring _Cgo_use for parameters, the compiler will treat the -// parameters, start from the second, as non-alive. Then, they will be marked -// as scalar in stackmap, which means the pointer won't be copied correctly -// in copystack. +// This is for issue #63739. +// Ensure that parameters are kept alive until the end of the C call. If not, +// then a stack copy at just the right time while calling into C might think +// that any stack pointers are not alive and fail to update them, causing the C +// function to see the old, no longer correct, pointer values. /* int add_from_multiple_pointers(int *a, int *b, int *c) {