Skip to content

Commit ff271cd

Browse files
doujiang24gopherbot
authored andcommitted
cmd/cgo: enable #cgo noescape/nocallback
In Go 1.22 we added code to the go/build package to ignore #cgo noescape and nocallback directives. That permits us to enable these directives in Go 1.24. Also, this fixed a Bug in CL 497837: After 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. Fixes #56378. Fixes #63739. Change-Id: I46e773240f8a467c3c4ba201dc5b4ee473cf6e3e GitHub-Last-Rev: 42fcc50 GitHub-Pull-Request: #66879 Reviewed-on: https://go-review.googlesource.com/c/go/+/579955 Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 96ef6ab commit ff271cd

File tree

11 files changed

+125
-20
lines changed

11 files changed

+125
-20
lines changed

src/cmd/cgo/doc.go

+24
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,30 @@ passing uninitialized C memory to Go code if the Go code is going to
425425
store pointer values in it. Zero out the memory in C before passing it
426426
to Go.
427427
428+
# Optimizing calls of C code
429+
430+
When passing a Go pointer to a C function the compiler normally ensures
431+
that the Go object lives on the heap. If the C function does not keep
432+
a copy of the Go pointer, and never passes the Go pointer back to Go code,
433+
then this is unnecessary. The #cgo noescape directive may be used to tell
434+
the compiler that no Go pointers escape via the named C function.
435+
If the noescape directive is used and the C function does not handle the
436+
pointer safely, the program may crash or see memory corruption.
437+
438+
For example:
439+
440+
// #cgo noescape cFunctionName
441+
442+
When a Go function calls a C function, it prepares for the C function to
443+
call back to a Go function. The #cgo nocallback directive may be used to
444+
tell the compiler that these preparations are not necessary.
445+
If the nocallback directive is used and the C function does call back into
446+
Go code, the program will panic.
447+
448+
For example:
449+
450+
// #cgo nocallback cFunctionName
451+
428452
# Special cases
429453
430454
A few special C types which would normally be represented by a pointer

src/cmd/cgo/gcc.go

-2
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,8 @@ func (f *File) ProcessCgoDirectives() {
9494
directive := fields[1]
9595
funcName := fields[2]
9696
if directive == "nocallback" {
97-
fatalf("#cgo nocallback disabled until Go 1.23")
9897
f.NoCallbacks[funcName] = true
9998
} else if directive == "noescape" {
100-
fatalf("#cgo noescape disabled until Go 1.23")
10199
f.NoEscapes[funcName] = true
102100
}
103101
}

src/cmd/cgo/internal/test/test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ int add(int x, int y) {
117117
118118
// escape vs noescape
119119
120-
// TODO(#56378): enable in Go 1.23:
121-
// #cgo noescape handleGoStringPointerNoescape
120+
#cgo noescape handleGoStringPointerNoescape
121+
#cgo nocallback handleGoStringPointerNoescape
122122
void handleGoStringPointerNoescape(void *s) {}
123123
124124
void handleGoStringPointerEscape(void *s) {}

src/cmd/cgo/internal/testerrors/testdata/notmatchedcfunction.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@
55
package main
66

77
/*
8-
// TODO(#56378): change back to "#cgo noescape noMatchedCFunction: no matched C function" in Go 1.23
9-
// ERROR MESSAGE: #cgo noescape disabled until Go 1.23
8+
// ERROR MESSAGE: #cgo noescape noMatchedCFunction: no matched C function
109
#cgo noescape noMatchedCFunction
1110
*/
1211
import "C"

src/cmd/cgo/out.go

+14-8
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ func (p *Package) writeDefs() {
104104
fmt.Fprintf(fgo2, "var _Cgo_always_false bool\n")
105105
fmt.Fprintf(fgo2, "//go:linkname _Cgo_use runtime.cgoUse\n")
106106
fmt.Fprintf(fgo2, "func _Cgo_use(interface{})\n")
107+
fmt.Fprintf(fgo2, "//go:linkname _Cgo_keepalive runtime.cgoKeepAlive\n")
108+
fmt.Fprintf(fgo2, "//go:noescape\n")
109+
fmt.Fprintf(fgo2, "func _Cgo_keepalive(interface{})\n")
107110
}
108111
fmt.Fprintf(fgo2, "//go:linkname _Cgo_no_callback runtime.cgoNoCallback\n")
109112
fmt.Fprintf(fgo2, "func _Cgo_no_callback(bool)\n")
@@ -639,17 +642,20 @@ func (p *Package) writeDefsFunc(fgo2 io.Writer, n *Name, callsMalloc *bool) {
639642
fmt.Fprintf(fgo2, "\t_Cgo_no_callback(false)\n")
640643
}
641644

642-
// skip _Cgo_use when noescape exist,
645+
// Use _Cgo_keepalive instead of _Cgo_use when noescape & nocallback exist,
643646
// so that the compiler won't force to escape them to heap.
644-
if !p.noEscapes[n.C] {
645-
fmt.Fprintf(fgo2, "\tif _Cgo_always_false {\n")
646-
if d.Type.Params != nil {
647-
for i := range d.Type.Params.List {
648-
fmt.Fprintf(fgo2, "\t\t_Cgo_use(p%d)\n", i)
649-
}
647+
// Instead, make the compiler keep them alive by using _Cgo_keepalive.
648+
touchFunc := "_Cgo_use"
649+
if p.noEscapes[n.C] && p.noCallbacks[n.C] {
650+
touchFunc = "_Cgo_keepalive"
651+
}
652+
fmt.Fprintf(fgo2, "\tif _Cgo_always_false {\n")
653+
if d.Type.Params != nil {
654+
for _, name := range paramnames {
655+
fmt.Fprintf(fgo2, "\t\t%s(%s)\n", touchFunc, name)
650656
}
651-
fmt.Fprintf(fgo2, "\t}\n")
652657
}
658+
fmt.Fprintf(fgo2, "\t}\n")
653659
fmt.Fprintf(fgo2, "\treturn\n")
654660
fmt.Fprintf(fgo2, "}\n")
655661
}

src/runtime/cgo.go

+11-2
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,20 @@ var cgoHasExtraM bool
7272
// cgoUse should not actually be called (see cgoAlwaysFalse).
7373
func cgoUse(any) { throw("cgoUse should not be called") }
7474

75+
// cgoKeepAlive is called by cgo-generated code (using go:linkname to get at
76+
// an unexported name). This call keeps its argument alive until the call site;
77+
// cgo emits the call after the last possible use of the argument by C code.
78+
// cgoKeepAlive is marked in the cgo-generated code as //go:noescape, so
79+
// unlike cgoUse it does not force the argument to escape to the heap.
80+
// This is used to implement the #cgo noescape directive.
81+
func cgoKeepAlive(any) { throw("cgoKeepAlive should not be called") }
82+
7583
// cgoAlwaysFalse is a boolean value that is always false.
76-
// The cgo-generated code says if cgoAlwaysFalse { cgoUse(p) }.
84+
// The cgo-generated code says if cgoAlwaysFalse { cgoUse(p) },
85+
// or if cgoAlwaysFalse { cgoKeepAlive(p) }.
7786
// The compiler cannot see that cgoAlwaysFalse is always false,
7887
// so it emits the test and keeps the call, giving the desired
79-
// escape analysis result. The test is cheaper than the call.
88+
// escape/alive analysis result. The test is cheaper than the call.
8089
var cgoAlwaysFalse bool
8190

8291
var cgo_yield = &_cgo_yield

src/runtime/crash_cgo_test.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,6 @@ func TestNeedmDeadlock(t *testing.T) {
750750
}
751751

752752
func TestCgoNoCallback(t *testing.T) {
753-
t.Skip("TODO(#56378): enable in Go 1.23")
754753
got := runTestProg(t, "testprogcgo", "CgoNoCallback")
755754
want := "function marked with #cgo nocallback called back into Go"
756755
if !strings.Contains(got, want) {
@@ -759,14 +758,22 @@ func TestCgoNoCallback(t *testing.T) {
759758
}
760759

761760
func TestCgoNoEscape(t *testing.T) {
762-
t.Skip("TODO(#56378): enable in Go 1.23")
763761
got := runTestProg(t, "testprogcgo", "CgoNoEscape")
764762
want := "OK\n"
765763
if got != want {
766764
t.Fatalf("want %s, got %s\n", want, got)
767765
}
768766
}
769767

768+
// Issue #63739.
769+
func TestCgoEscapeWithMultiplePointers(t *testing.T) {
770+
got := runTestProg(t, "testprogcgo", "CgoEscapeWithMultiplePointers")
771+
want := "OK\n"
772+
if got != want {
773+
t.Fatalf("output is %s; want %s", got, want)
774+
}
775+
}
776+
770777
func TestCgoTracebackGoroutineProfile(t *testing.T) {
771778
output := runTestProg(t, "testprogcgo", "GoroutineProfile")
772779
want := "OK\n"

src/runtime/linkname.go

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import _ "unsafe"
1313
//go:linkname _cgo_panic_internal
1414
//go:linkname cgoAlwaysFalse
1515
//go:linkname cgoUse
16+
//go:linkname cgoKeepAlive
1617
//go:linkname cgoCheckPointer
1718
//go:linkname cgoCheckResult
1819
//go:linkname cgoNoCallback

src/runtime/testdata/testprogcgo/cgonocallback.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ package main
88
// But it do callback to go in this test, Go should crash here.
99

1010
/*
11-
// TODO(#56378): #cgo nocallback runCShouldNotCallback
11+
#cgo nocallback runCShouldNotCallback
12+
1213
extern void runCShouldNotCallback();
1314
*/
1415
import "C"

src/runtime/testdata/testprogcgo/cgonoescape.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ package main
1313
// 2. less than 100 new allocated heap objects after invoking withoutNoEscape 100 times.
1414

1515
/*
16-
// TODO(#56378): #cgo noescape runCWithNoEscape
16+
#cgo noescape runCWithNoEscape
17+
#cgo nocallback runCWithNoEscape
1718
1819
void runCWithNoEscape(void *p) {
1920
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Copyright 2020 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
// This is for issue #63739.
8+
// Ensure that parameters are kept alive until the end of the C call. If not,
9+
// then a stack copy at just the right time while calling into C might think
10+
// that any stack pointers are not alive and fail to update them, causing the C
11+
// function to see the old, no longer correct, pointer values.
12+
13+
/*
14+
int add_from_multiple_pointers(int *a, int *b, int *c) {
15+
*a = *a + 1;
16+
*b = *b + 1;
17+
*c = *c + 1;
18+
return *a + *b + *c;
19+
}
20+
#cgo noescape add_from_multiple_pointers
21+
#cgo nocallback add_from_multiple_pointers
22+
*/
23+
import "C"
24+
25+
import (
26+
"fmt"
27+
)
28+
29+
const (
30+
maxStack = 1024
31+
)
32+
33+
func init() {
34+
register("CgoEscapeWithMultiplePointers", CgoEscapeWithMultiplePointers)
35+
}
36+
37+
func CgoEscapeWithMultiplePointers() {
38+
stackGrow(maxStack)
39+
fmt.Println("OK")
40+
}
41+
42+
//go:noinline
43+
func testCWithMultiplePointers() {
44+
var a C.int = 1
45+
var b C.int = 2
46+
var c C.int = 3
47+
v := C.add_from_multiple_pointers(&a, &b, &c)
48+
if v != 9 || a != 2 || b != 3 || c != 4 {
49+
fmt.Printf("%d + %d + %d != %d\n", a, b, c, v)
50+
}
51+
}
52+
53+
func stackGrow(n int) {
54+
if n == 0 {
55+
return
56+
}
57+
testCWithMultiplePointers()
58+
stackGrow(n - 1)
59+
}

0 commit comments

Comments
 (0)