From 6448f1b2e640b3521d00a26126e63d1ff12317cd Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Wed, 24 May 2023 16:33:35 +0800 Subject: [PATCH 01/14] cmd/cgo: add #cgo noescape/nocallback annotations When passing pointers of Go objects from Go to C, the cgo command generate _Cgo_use(pN) for the unsafe.Pointer type arguments, so that the Go compiler will escape these object to heap. Since the C function may callback to Go, then the Go stack might grow/shrink, that means the pointers that the C function have will be invalid. After adding the #cgo noescape/nocallback annotations for a C function, the cgo command won't generate _Cgo_use(pN), and the Go compiler won't force the object escape to heap, that means the C function won't callback to Go, if it do callback to Go, the Go process will crash. Fixes #56378 --- src/cmd/cgo/gcc.go | 23 ++++++++-- src/cmd/cgo/main.go | 42 ++++++++++++++----- src/cmd/cgo/out.go | 29 ++++++++++--- src/cmd/go/internal/modindex/build.go | 7 ++++ src/go/build/build.go | 7 ++++ src/runtime/cgo.go | 5 +++ src/runtime/cgocall.go | 4 ++ src/runtime/crash_cgo_test.go | 12 ++++++ src/runtime/runtime2.go | 1 + .../testdata/testprogcgo/CgoNoCallback.go | 40 ++++++++++++++++++ 10 files changed, 150 insertions(+), 20 deletions(-) create mode 100644 src/runtime/testdata/testprogcgo/CgoNoCallback.go diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index 78a44d33a247b0..92e9e5d5c2de81 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -24,6 +24,7 @@ import ( "math" "os" "os/exec" + "regexp" "strconv" "strings" "unicode" @@ -49,6 +50,8 @@ var nameToC = map[string]string{ var incomplete = "_cgopackage.Incomplete" +var cgoRx = regexp.MustCompile(`^#cgo\s+(nocallback|noescape)\s+(\S+)\s*$`) + // cname returns the C name to use for C.s. // The expansions are listed in nameToC and also // struct_foo becomes "struct foo", and similarly for @@ -73,18 +76,30 @@ func cname(s string) string { return s } -// DiscardCgoDirectives processes the import C preamble, and discards -// all #cgo CFLAGS and LDFLAGS directives, so they don't make their -// way into _cgo_export.h. -func (f *File) DiscardCgoDirectives() { +// ProcessCgoDirectives processes the import C preamble: +// 1. discards all #cgo CFLAGS and LDFLAGS directives, so they don't make their +// way into _cgo_export.h. +// 2. parse the nocallback and noescape directives. +func (f *File) ProcessCgoDirectives() { linesIn := strings.Split(f.Preamble, "\n") linesOut := make([]string, 0, len(linesIn)) + f.NoCallbacks = make(map[string]struct{}) + f.NoEscapes = make(map[string]struct{}) for _, line := range linesIn { l := strings.TrimSpace(line) if len(l) < 5 || l[:4] != "#cgo" || !unicode.IsSpace(rune(l[4])) { linesOut = append(linesOut, line) } else { linesOut = append(linesOut, "") + + match := cgoRx.FindStringSubmatch(line) + if match != nil { + if match[1] == "nocallback" { + f.NoCallbacks[match[2]] = struct{}{} + } else { + f.NoEscapes[match[2]] = struct{}{} + } + } } } f.Preamble = strings.Join(linesOut, "\n") diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go index 78020aedbe5aa6..d5b21d04aa1dcf 100644 --- a/src/cmd/cgo/main.go +++ b/src/cmd/cgo/main.go @@ -48,6 +48,8 @@ type Package struct { Preamble string // collected preamble for _cgo_export.h typedefs map[string]bool // type names that appear in the types of the objects we're interested in typedefList []typedefInfo + noCallbacks map[string]struct{} // C function names that with #cgo nocallback directive + noEscapes map[string]struct{} // C function names that with #cgo noescape directive } // A typedefInfo is an element on Package.typedefList: a typedef name @@ -59,16 +61,18 @@ type typedefInfo struct { // A File collects information about a single Go input file. type File struct { - AST *ast.File // parsed AST - Comments []*ast.CommentGroup // comments from file - Package string // Package name - Preamble string // C preamble (doc comment on import "C") - Ref []*Ref // all references to C.xxx in AST - Calls []*Call // all calls to C.xxx in AST - ExpFunc []*ExpFunc // exported functions for this file - Name map[string]*Name // map from Go name to Name - NamePos map[*Name]token.Pos // map from Name to position of the first reference - Edit *edit.Buffer + AST *ast.File // parsed AST + Comments []*ast.CommentGroup // comments from file + Package string // Package name + Preamble string // C preamble (doc comment on import "C") + Ref []*Ref // all references to C.xxx in AST + Calls []*Call // all calls to C.xxx in AST + ExpFunc []*ExpFunc // exported functions for this file + Name map[string]*Name // map from Go name to Name + NamePos map[*Name]token.Pos // map from Name to position of the first reference + Edit *edit.Buffer + NoCallbacks map[string]struct{} // C function names that with #cgo nocallback directive + NoEscapes map[string]struct{} // C function names that with #cgo noescape directive } func (f *File) offset(p token.Pos) int { @@ -374,7 +378,7 @@ func main() { f := new(File) f.Edit = edit.NewBuffer(b) f.ParseGo(input, b) - f.DiscardCgoDirectives() + f.ProcessCgoDirectives() fs[i] = f } @@ -487,6 +491,22 @@ func (p *Package) Record(f *File) { } } + // merge nocallback & noescape + if p.noCallbacks == nil { + p.noCallbacks = f.NoCallbacks + } else { + for k, v := range f.NoCallbacks { + p.noCallbacks[k] = v + } + } + if p.noEscapes == nil { + p.noEscapes = f.NoEscapes + } else { + for k, v := range f.NoEscapes { + p.noEscapes[k] = v + } + } + if f.ExpFunc != nil { p.ExpFunc = append(p.ExpFunc, f.ExpFunc...) p.Preamble += "\n" + f.Preamble diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go index b2933e2d82e80e..fb65a4f3cc2fa8 100644 --- a/src/cmd/cgo/out.go +++ b/src/cmd/cgo/out.go @@ -105,6 +105,8 @@ func (p *Package) writeDefs() { fmt.Fprintf(fgo2, "var _Cgo_always_false bool\n") fmt.Fprintf(fgo2, "//go:linkname _Cgo_use runtime.cgoUse\n") fmt.Fprintf(fgo2, "func _Cgo_use(interface{})\n") + fmt.Fprintf(fgo2, "//go:linkname _Cgo_no_callback runtime.cgoNoCallback\n") + fmt.Fprintf(fgo2, "func _Cgo_no_callback(bool)\n") } typedefNames := make([]string, 0, len(typedef)) @@ -612,6 +614,17 @@ func (p *Package) writeDefsFunc(fgo2 io.Writer, n *Name, callsMalloc *bool) { arg = "uintptr(unsafe.Pointer(&r1))" } + _, noCallback := p.noCallbacks[n.C] + _, noEscape := p.noEscapes[n.C] + noCgoCallback := false + if noEscape && noCallback { + noCgoCallback = true + } + if noCgoCallback { + // disable cgocallback, will check it in runtime. + fmt.Fprintf(fgo2, "\t_Cgo_no_callback(true)\n") + } + prefix := "" if n.AddError { prefix = "errno := " @@ -620,13 +633,19 @@ func (p *Package) writeDefsFunc(fgo2 io.Writer, n *Name, callsMalloc *bool) { if n.AddError { fmt.Fprintf(fgo2, "\tif errno != 0 { r2 = syscall.Errno(errno) }\n") } - fmt.Fprintf(fgo2, "\tif _Cgo_always_false {\n") - if d.Type.Params != nil { - for i := range d.Type.Params.List { - fmt.Fprintf(fgo2, "\t\t_Cgo_use(p%d)\n", i) + if noCgoCallback { + fmt.Fprintf(fgo2, "\t_Cgo_no_callback(false)\n") + } else { + // skip _Cgo_use when noescape & nocallback both exist, + // so that the compiler won't force to escape them to heap. + fmt.Fprintf(fgo2, "\tif _Cgo_always_false {\n") + if d.Type.Params != nil { + for i := range d.Type.Params.List { + fmt.Fprintf(fgo2, "\t\t_Cgo_use(p%d)\n", i) + } } + fmt.Fprintf(fgo2, "\t}\n") } - fmt.Fprintf(fgo2, "\t}\n") fmt.Fprintf(fgo2, "\treturn\n") fmt.Fprintf(fgo2, "}\n") } diff --git a/src/cmd/go/internal/modindex/build.go b/src/cmd/go/internal/modindex/build.go index b57f2f6368f0fe..72000aa40e7eb5 100644 --- a/src/cmd/go/internal/modindex/build.go +++ b/src/cmd/go/internal/modindex/build.go @@ -20,6 +20,7 @@ import ( "io" "io/fs" "path/filepath" + "regexp" "sort" "strings" "unicode" @@ -108,6 +109,8 @@ type Context struct { OpenFile func(path string) (io.ReadCloser, error) } +var cgoRx = regexp.MustCompile(`^#cgo\s+(?:nocallback|noescape)\s+(?:\S+)\s*$`) + // joinPath calls ctxt.JoinPath (if not nil) or else filepath.Join. func (ctxt *Context) joinPath(elem ...string) string { if f := ctxt.JoinPath; f != nil { @@ -622,6 +625,10 @@ func (ctxt *Context) saveCgo(filename string, di *build.Package, text string) er continue } + if cgoRx.FindStringSubmatch(line) != nil { + continue + } + // Split at colon. line, argstr, ok := strings.Cut(strings.TrimSpace(line[4:]), ":") if !ok { diff --git a/src/go/build/build.go b/src/go/build/build.go index dd6cdc903a21a8..e21984eae07f5b 100644 --- a/src/go/build/build.go +++ b/src/go/build/build.go @@ -23,6 +23,7 @@ import ( "os/exec" pathpkg "path" "path/filepath" + "regexp" "runtime" "sort" "strconv" @@ -113,6 +114,8 @@ type Context struct { OpenFile func(path string) (io.ReadCloser, error) } +var cgoRx = regexp.MustCompile(`^#cgo\s+(?:nocallback|noescape)\s+(?:\S+)\s*$`) + // joinPath calls ctxt.JoinPath (if not nil) or else filepath.Join. func (ctxt *Context) joinPath(elem ...string) string { if f := ctxt.JoinPath; f != nil { @@ -1687,6 +1690,10 @@ func (ctxt *Context) saveCgo(filename string, di *Package, cg *ast.CommentGroup) continue } + if cgoRx.FindStringSubmatch(line) != nil { + continue + } + // Split at colon. line, argstr, ok := strings.Cut(strings.TrimSpace(line[4:]), ":") if !ok { diff --git a/src/runtime/cgo.go b/src/runtime/cgo.go index 395303552c9bd1..eb1335f495afd6 100644 --- a/src/runtime/cgo.go +++ b/src/runtime/cgo.go @@ -61,3 +61,8 @@ func cgoUse(any) { throw("cgoUse should not be called") } var cgoAlwaysFalse bool var cgo_yield = &_cgo_yield + +func cgoNoCallback(v bool) { + g := getg() + g.nocgocallback = v +} diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go index f6e2f6381382b5..b1fa9b23068461 100644 --- a/src/runtime/cgocall.go +++ b/src/runtime/cgocall.go @@ -216,6 +216,10 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) { exit(2) } + if gp.nocgocallback { + throw("runtime: disable callback from C") + } + // The call from C is on gp.m's g0 stack, so we must ensure // that we stay on that M. We have to do this before calling // exitsyscall, since it would otherwise be free to move us to diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index e1851808f3194e..748e0ef4e10795 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -753,6 +753,18 @@ func TestNeedmDeadlock(t *testing.T) { } } +func TestCgoNoCallback(t *testing.T) { + switch runtime.GOOS { + case "plan9", "windows": + t.Skipf("no signals on %s", runtime.GOOS) + } + got := runTestProg(t, "testprogcgo", "CgoNoCallback") + want := "runtime: disable callback from C" + if !strings.Contains(got, want) { + t.Fatalf("did not see %q in output: %q", want, got) + } +} + func TestCgoTracebackGoroutineProfile(t *testing.T) { output := runTestProg(t, "testprogcgo", "GoroutineProfile") want := "OK\n" diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 2a02e1fb3bfe2e..2aa69057ea2efd 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -482,6 +482,7 @@ type g struct { raceignore int8 // ignore race detection events tracking bool // whether we're tracking this G for sched latency statistics + nocgocallback bool // whether disable callback from C trackingSeq uint8 // used to decide whether to track this G trackingStamp int64 // timestamp of when the G last started being tracked runnableTime int64 // the amount of time spent runnable, cleared when running, only used when tracking diff --git a/src/runtime/testdata/testprogcgo/CgoNoCallback.go b/src/runtime/testdata/testprogcgo/CgoNoCallback.go new file mode 100644 index 00000000000000..c6290c082f54f7 --- /dev/null +++ b/src/runtime/testdata/testprogcgo/CgoNoCallback.go @@ -0,0 +1,40 @@ +// Copyright 2020 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. + +//go:build !plan9 && !windows +// +build !plan9,!windows + +package main + +// #cgo noescape/nocallback annotations for a C function means it should not callback to Go. +// But it do callback to go in this test, Go should crash here. + +/* +#cgo nocallback runCShouldNotCallback +#cgo noescape runCShouldNotCallback + +extern void CallbackToGo(); + +static void runCShouldNotCallback() { + CallbackToGo(); +} +*/ +import "C" + +import ( + "fmt" +) + +func init() { + register("CgoNoCallback", CgoNoCallback) +} + +//export CallbackToGo +func CallbackToGo() { +} + +func CgoNoCallback() { + C.runCShouldNotCallback() + fmt.Println("OK") +} From e9e6a42d9f1a0e5b47cac43e20afc497959dea22 Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Tue, 30 May 2023 09:38:46 +0800 Subject: [PATCH 02/14] address comments. Signed-off-by: doujiang24 --- src/cmd/cgo/gcc.go | 23 ++++++------- src/cmd/cgo/main.go | 34 ++++++++----------- src/cmd/cgo/out.go | 4 +-- src/cmd/go/internal/modindex/build.go | 6 ++-- src/go/build/build.go | 6 ++-- src/runtime/cgocall.go | 8 ++--- src/runtime/crash_cgo_test.go | 6 +--- .../testdata/testprogcgo/CgoNoCallback.go | 3 -- 8 files changed, 35 insertions(+), 55 deletions(-) diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index 92e9e5d5c2de81..35bb4cd9cad5ce 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -24,7 +24,6 @@ import ( "math" "os" "os/exec" - "regexp" "strconv" "strings" "unicode" @@ -50,8 +49,6 @@ var nameToC = map[string]string{ var incomplete = "_cgopackage.Incomplete" -var cgoRx = regexp.MustCompile(`^#cgo\s+(nocallback|noescape)\s+(\S+)\s*$`) - // cname returns the C name to use for C.s. // The expansions are listed in nameToC and also // struct_foo becomes "struct foo", and similarly for @@ -83,21 +80,21 @@ func cname(s string) string { func (f *File) ProcessCgoDirectives() { linesIn := strings.Split(f.Preamble, "\n") linesOut := make([]string, 0, len(linesIn)) - f.NoCallbacks = make(map[string]struct{}) - f.NoEscapes = make(map[string]struct{}) + f.NoCallbacks = make(map[string]bool) + f.NoEscapes = make(map[string]bool) for _, line := range linesIn { l := strings.TrimSpace(line) if len(l) < 5 || l[:4] != "#cgo" || !unicode.IsSpace(rune(l[4])) { linesOut = append(linesOut, line) } else { - linesOut = append(linesOut, "") - - match := cgoRx.FindStringSubmatch(line) - if match != nil { - if match[1] == "nocallback" { - f.NoCallbacks[match[2]] = struct{}{} - } else { - f.NoEscapes[match[2]] = struct{}{} + // #cgo (nocallback|noescape) + if fields := strings.Fields(l); len(fields) == 3 { + directive := fields[1] + funcName := fields[2] + if directive == "nocallback" { + f.NoCallbacks[funcName] = true + } else if directive == "noescape" { + f.NoEscapes[funcName] = true } } } diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go index d5b21d04aa1dcf..34093d15f7f7b9 100644 --- a/src/cmd/cgo/main.go +++ b/src/cmd/cgo/main.go @@ -48,8 +48,8 @@ type Package struct { Preamble string // collected preamble for _cgo_export.h typedefs map[string]bool // type names that appear in the types of the objects we're interested in typedefList []typedefInfo - noCallbacks map[string]struct{} // C function names that with #cgo nocallback directive - noEscapes map[string]struct{} // C function names that with #cgo noescape directive + noCallbacks map[string]bool // C function names that with #cgo nocallback directive + noEscapes map[string]bool // C function names that with #cgo noescape directive } // A typedefInfo is an element on Package.typedefList: a typedef name @@ -70,9 +70,9 @@ type File struct { ExpFunc []*ExpFunc // exported functions for this file Name map[string]*Name // map from Go name to Name NamePos map[*Name]token.Pos // map from Name to position of the first reference + NoCallbacks map[string]bool // C function names that with #cgo nocallback directive + NoEscapes map[string]bool // C function names that with #cgo noescape directive Edit *edit.Buffer - NoCallbacks map[string]struct{} // C function names that with #cgo nocallback directive - NoEscapes map[string]struct{} // C function names that with #cgo noescape directive } func (f *File) offset(p token.Pos) int { @@ -454,10 +454,12 @@ func newPackage(args []string) *Package { os.Setenv("LC_ALL", "C") p := &Package{ - PtrSize: ptrSize, - IntSize: intSize, - CgoFlags: make(map[string][]string), - Written: make(map[string]bool), + PtrSize: ptrSize, + IntSize: intSize, + CgoFlags: make(map[string][]string), + Written: make(map[string]bool), + noCallbacks: make(map[string]bool), + noEscapes: make(map[string]bool), } p.addToFlag("CFLAGS", args) return p @@ -492,19 +494,11 @@ func (p *Package) Record(f *File) { } // merge nocallback & noescape - if p.noCallbacks == nil { - p.noCallbacks = f.NoCallbacks - } else { - for k, v := range f.NoCallbacks { - p.noCallbacks[k] = v - } + for k, v := range f.NoCallbacks { + p.noCallbacks[k] = v } - if p.noEscapes == nil { - p.noEscapes = f.NoEscapes - } else { - for k, v := range f.NoEscapes { - p.noEscapes[k] = v - } + for k, v := range f.NoEscapes { + p.noEscapes[k] = v } if f.ExpFunc != nil { diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go index fb65a4f3cc2fa8..561074dd37c9a2 100644 --- a/src/cmd/cgo/out.go +++ b/src/cmd/cgo/out.go @@ -105,9 +105,9 @@ func (p *Package) writeDefs() { fmt.Fprintf(fgo2, "var _Cgo_always_false bool\n") fmt.Fprintf(fgo2, "//go:linkname _Cgo_use runtime.cgoUse\n") fmt.Fprintf(fgo2, "func _Cgo_use(interface{})\n") - fmt.Fprintf(fgo2, "//go:linkname _Cgo_no_callback runtime.cgoNoCallback\n") - fmt.Fprintf(fgo2, "func _Cgo_no_callback(bool)\n") } + fmt.Fprintf(fgo2, "//go:linkname _Cgo_no_callback runtime.cgoNoCallback\n") + fmt.Fprintf(fgo2, "func _Cgo_no_callback(bool)\n") typedefNames := make([]string, 0, len(typedef)) for name := range typedef { diff --git a/src/cmd/go/internal/modindex/build.go b/src/cmd/go/internal/modindex/build.go index 72000aa40e7eb5..0b06373984190a 100644 --- a/src/cmd/go/internal/modindex/build.go +++ b/src/cmd/go/internal/modindex/build.go @@ -20,7 +20,6 @@ import ( "io" "io/fs" "path/filepath" - "regexp" "sort" "strings" "unicode" @@ -109,8 +108,6 @@ type Context struct { OpenFile func(path string) (io.ReadCloser, error) } -var cgoRx = regexp.MustCompile(`^#cgo\s+(?:nocallback|noescape)\s+(?:\S+)\s*$`) - // joinPath calls ctxt.JoinPath (if not nil) or else filepath.Join. func (ctxt *Context) joinPath(elem ...string) string { if f := ctxt.JoinPath; f != nil { @@ -625,7 +622,8 @@ func (ctxt *Context) saveCgo(filename string, di *build.Package, text string) er continue } - if cgoRx.FindStringSubmatch(line) != nil { + // #cgo (nocallback|noescape) + if fields := strings.Fields(line); len(fields) == 3 && (fields[1] == "nocallback" || fields[1] == "noescape") { continue } diff --git a/src/go/build/build.go b/src/go/build/build.go index e21984eae07f5b..f51713806119e1 100644 --- a/src/go/build/build.go +++ b/src/go/build/build.go @@ -23,7 +23,6 @@ import ( "os/exec" pathpkg "path" "path/filepath" - "regexp" "runtime" "sort" "strconv" @@ -114,8 +113,6 @@ type Context struct { OpenFile func(path string) (io.ReadCloser, error) } -var cgoRx = regexp.MustCompile(`^#cgo\s+(?:nocallback|noescape)\s+(?:\S+)\s*$`) - // joinPath calls ctxt.JoinPath (if not nil) or else filepath.Join. func (ctxt *Context) joinPath(elem ...string) string { if f := ctxt.JoinPath; f != nil { @@ -1690,7 +1687,8 @@ func (ctxt *Context) saveCgo(filename string, di *Package, cg *ast.CommentGroup) continue } - if cgoRx.FindStringSubmatch(line) != nil { + // #cgo (nocallback|noescape) + if fields := strings.Fields(line); len(fields) == 3 && (fields[1] == "nocallback" || fields[1] == "noescape") { continue } diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go index b1fa9b23068461..802d6f2084067b 100644 --- a/src/runtime/cgocall.go +++ b/src/runtime/cgocall.go @@ -216,10 +216,6 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) { exit(2) } - if gp.nocgocallback { - throw("runtime: disable callback from C") - } - // The call from C is on gp.m's g0 stack, so we must ensure // that we stay on that M. We have to do this before calling // exitsyscall, since it would otherwise be free to move us to @@ -246,6 +242,10 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) { osPreemptExtExit(gp.m) + if gp.nocgocallback { + panic("runtime: function marked with #cgo nocallback called back into Go") + } + cgocallbackg1(fn, frame, ctxt) // will call unlockOSThread // At this point unlockOSThread has been called. diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index 748e0ef4e10795..9954d397b75c8a 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -754,12 +754,8 @@ func TestNeedmDeadlock(t *testing.T) { } func TestCgoNoCallback(t *testing.T) { - switch runtime.GOOS { - case "plan9", "windows": - t.Skipf("no signals on %s", runtime.GOOS) - } got := runTestProg(t, "testprogcgo", "CgoNoCallback") - want := "runtime: disable callback from C" + want := "function marked with #cgo nocallback called back into Go" if !strings.Contains(got, want) { t.Fatalf("did not see %q in output: %q", want, got) } diff --git a/src/runtime/testdata/testprogcgo/CgoNoCallback.go b/src/runtime/testdata/testprogcgo/CgoNoCallback.go index c6290c082f54f7..5eb9b1905f15a4 100644 --- a/src/runtime/testdata/testprogcgo/CgoNoCallback.go +++ b/src/runtime/testdata/testprogcgo/CgoNoCallback.go @@ -2,9 +2,6 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build !plan9 && !windows -// +build !plan9,!windows - package main // #cgo noescape/nocallback annotations for a C function means it should not callback to Go. From 05b74c4539122cdf3b5bdd1e47da5ddf15c3a5dd Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Tue, 30 May 2023 14:29:47 +0800 Subject: [PATCH 03/14] update doc. --- src/cmd/cgo/doc.go | 19 +++++++++++++++++++ src/cmd/cgo/out.go | 6 +++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/cmd/cgo/doc.go b/src/cmd/cgo/doc.go index b1a288f57392aa..b2e835459711d9 100644 --- a/src/cmd/cgo/doc.go +++ b/src/cmd/cgo/doc.go @@ -420,6 +420,25 @@ passing uninitialized C memory to Go code if the Go code is going to store pointer values in it. Zero out the memory in C before passing it to Go. +When passing pointers of Go objects from Go to C, the Go compiler will +always escape these objects to heap, by default. +Since the C function may call back to Go, and then the Go stack might +grow/shrink, and the Go objects on stack will be moved to a new stack, +so that the Go compiler always escape those objects have been passed to C. + +After adding the #cgo noescape/nocallback annotations for a C function, +that means the C function won't call back to Go, +the Go compiler won't force those Go objects escape to heap, +to reduce GC overhead, for better performance. + +For example: + + // #cgo nocallback cFunctionName + // #cgo noescape cFunctionName + +When a C function marked with #cgo nocallback/noescape, but it does call +back to Go, the Go process will panic. + # Special cases A few special C types which would normally be represented by a pointer diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go index 561074dd37c9a2..5a394b404d9573 100644 --- a/src/cmd/cgo/out.go +++ b/src/cmd/cgo/out.go @@ -614,10 +614,10 @@ func (p *Package) writeDefsFunc(fgo2 io.Writer, n *Name, callsMalloc *bool) { arg = "uintptr(unsafe.Pointer(&r1))" } - _, noCallback := p.noCallbacks[n.C] - _, noEscape := p.noEscapes[n.C] noCgoCallback := false - if noEscape && noCallback { + noCallback, ok1 := p.noCallbacks[n.C] + noEscape, ok2 := p.noEscapes[n.C] + if ok1 && ok2 && noEscape && noCallback { noCgoCallback = true } if noCgoCallback { From 0a52887ef64bfdda4588bc9112bd42a86911eb0b Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Tue, 30 May 2023 15:28:37 +0800 Subject: [PATCH 04/14] minor bugfix. --- src/cmd/cgo/gcc.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index 35bb4cd9cad5ce..10c902adcec336 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -87,6 +87,8 @@ func (f *File) ProcessCgoDirectives() { if len(l) < 5 || l[:4] != "#cgo" || !unicode.IsSpace(rune(l[4])) { linesOut = append(linesOut, line) } else { + linesOut = append(linesOut, "") + // #cgo (nocallback|noescape) if fields := strings.Fields(l); len(fields) == 3 { directive := fields[1] From b0ebd477b71f20d4211db5fafdec2eb52d6d8426 Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Tue, 30 May 2023 18:13:07 +0800 Subject: [PATCH 05/14] remove extern. --- src/runtime/testdata/testprogcgo/CgoNoCallback.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/testdata/testprogcgo/CgoNoCallback.go b/src/runtime/testdata/testprogcgo/CgoNoCallback.go index 5eb9b1905f15a4..f68209e4326cc2 100644 --- a/src/runtime/testdata/testprogcgo/CgoNoCallback.go +++ b/src/runtime/testdata/testprogcgo/CgoNoCallback.go @@ -11,7 +11,7 @@ package main #cgo nocallback runCShouldNotCallback #cgo noescape runCShouldNotCallback -extern void CallbackToGo(); +void CallbackToGo(); static void runCShouldNotCallback() { CallbackToGo(); From 173c7f9f7207a6374fea76fa9c97fdd4771de0d7 Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Tue, 30 May 2023 19:23:02 +0800 Subject: [PATCH 06/14] move to a new C file. --- src/runtime/testdata/testprogcgo/CgoNoCallback.c | 9 +++++++++ src/runtime/testdata/testprogcgo/CgoNoCallback.go | 6 +----- 2 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 src/runtime/testdata/testprogcgo/CgoNoCallback.c diff --git a/src/runtime/testdata/testprogcgo/CgoNoCallback.c b/src/runtime/testdata/testprogcgo/CgoNoCallback.c new file mode 100644 index 00000000000000..465a48436186f1 --- /dev/null +++ b/src/runtime/testdata/testprogcgo/CgoNoCallback.c @@ -0,0 +1,9 @@ +// 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. + +#include "_cgo_export.h" + +void runCShouldNotCallback() { + CallbackToGo(); +} diff --git a/src/runtime/testdata/testprogcgo/CgoNoCallback.go b/src/runtime/testdata/testprogcgo/CgoNoCallback.go index f68209e4326cc2..5e8432341f26ed 100644 --- a/src/runtime/testdata/testprogcgo/CgoNoCallback.go +++ b/src/runtime/testdata/testprogcgo/CgoNoCallback.go @@ -11,11 +11,7 @@ package main #cgo nocallback runCShouldNotCallback #cgo noescape runCShouldNotCallback -void CallbackToGo(); - -static void runCShouldNotCallback() { - CallbackToGo(); -} +extern void runCShouldNotCallback(); */ import "C" From 34fd6b0cd2a895286bb27e69796e9d0fb4187677 Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Thu, 17 Aug 2023 19:43:00 +0800 Subject: [PATCH 07/14] address comments. Signed-off-by: doujiang24 --- src/cmd/cgo/doc.go | 29 ++++--- src/cmd/cgo/gcc.go | 4 +- src/cmd/cgo/main.go | 13 ++-- src/cmd/cgo/out.go | 17 ++-- src/runtime/cgo.go | 3 + src/runtime/crash_cgo_test.go | 8 ++ src/runtime/runtime2.go | 2 +- .../testdata/testprogcgo/CgoNoCallback.go | 5 +- .../testdata/testprogcgo/CgoNoEscape.go | 78 +++++++++++++++++++ 9 files changed, 123 insertions(+), 36 deletions(-) create mode 100644 src/runtime/testdata/testprogcgo/CgoNoEscape.go diff --git a/src/cmd/cgo/doc.go b/src/cmd/cgo/doc.go index b2e835459711d9..0e708fb3b44ac4 100644 --- a/src/cmd/cgo/doc.go +++ b/src/cmd/cgo/doc.go @@ -420,24 +420,29 @@ passing uninitialized C memory to Go code if the Go code is going to store pointer values in it. Zero out the memory in C before passing it to Go. -When passing pointers of Go objects from Go to C, the Go compiler will -always escape these objects to heap, by default. -Since the C function may call back to Go, and then the Go stack might -grow/shrink, and the Go objects on stack will be moved to a new stack, -so that the Go compiler always escape those objects have been passed to C. +# Optimizing calls of C code -After adding the #cgo noescape/nocallback annotations for a C function, -that means the C function won't call back to Go, -the Go compiler won't force those Go objects escape to heap, -to reduce GC overhead, for better performance. +When passing a Go pointer to a C function the compiler normally ensures +that the Go object lives on the heap. If the C function does not keep +a copy of the Go pointer, and never passes the Go pointer back to Go code, +then this is unnecessary. The #cgo noescape directive may be used to tell +the compiler that no Go pointers escape via the named C function. +If the noescape directive is used and the C function does not handle the +pointer safely, the program may crash or see memory corruption. For example: // #cgo nocallback cFunctionName - // #cgo noescape cFunctionName -When a C function marked with #cgo nocallback/noescape, but it does call -back to Go, the Go process will panic. +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 +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. + +For example: + + // #cgo noescape cFunctionName # Special cases diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go index 10c902adcec336..28dc2a9bf89981 100644 --- a/src/cmd/cgo/gcc.go +++ b/src/cmd/cgo/gcc.go @@ -74,8 +74,8 @@ func cname(s string) string { } // ProcessCgoDirectives processes the import C preamble: -// 1. discards all #cgo CFLAGS and LDFLAGS directives, so they don't make their -// way into _cgo_export.h. +// 1. discards all #cgo CFLAGS, LDFLAGS, nocallback and noescape directives, +// so they don't make their way into _cgo_export.h. // 2. parse the nocallback and noescape directives. func (f *File) ProcessCgoDirectives() { linesIn := strings.Split(f.Preamble, "\n") diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go index 34093d15f7f7b9..5b88c05e0f7f1a 100644 --- a/src/cmd/cgo/main.go +++ b/src/cmd/cgo/main.go @@ -18,6 +18,7 @@ import ( "go/token" "internal/buildcfg" "io" + "maps" "os" "path/filepath" "reflect" @@ -48,8 +49,8 @@ type Package struct { Preamble string // collected preamble for _cgo_export.h typedefs map[string]bool // type names that appear in the types of the objects we're interested in typedefList []typedefInfo - noCallbacks map[string]bool // C function names that with #cgo nocallback directive - noEscapes map[string]bool // C function names that with #cgo noescape directive + noCallbacks map[string]bool // C function names with #cgo nocallback directive + noEscapes map[string]bool // C function names with #cgo noescape directive } // A typedefInfo is an element on Package.typedefList: a typedef name @@ -494,12 +495,8 @@ func (p *Package) Record(f *File) { } // merge nocallback & noescape - for k, v := range f.NoCallbacks { - p.noCallbacks[k] = v - } - for k, v := range f.NoEscapes { - p.noEscapes[k] = v - } + maps.Copy(p.noCallbacks, f.NoCallbacks) + maps.Copy(p.noEscapes, f.NoEscapes) if f.ExpFunc != nil { p.ExpFunc = append(p.ExpFunc, f.ExpFunc...) diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go index 5a394b404d9573..243e658837bcd2 100644 --- a/src/cmd/cgo/out.go +++ b/src/cmd/cgo/out.go @@ -614,13 +614,8 @@ func (p *Package) writeDefsFunc(fgo2 io.Writer, n *Name, callsMalloc *bool) { arg = "uintptr(unsafe.Pointer(&r1))" } - noCgoCallback := false - noCallback, ok1 := p.noCallbacks[n.C] - noEscape, ok2 := p.noEscapes[n.C] - if ok1 && ok2 && noEscape && noCallback { - noCgoCallback = true - } - if noCgoCallback { + noCallback, _ := p.noCallbacks[n.C] + if noCallback { // disable cgocallback, will check it in runtime. fmt.Fprintf(fgo2, "\t_Cgo_no_callback(true)\n") } @@ -633,10 +628,12 @@ func (p *Package) writeDefsFunc(fgo2 io.Writer, n *Name, callsMalloc *bool) { if n.AddError { fmt.Fprintf(fgo2, "\tif errno != 0 { r2 = syscall.Errno(errno) }\n") } - if noCgoCallback { + if noCallback { fmt.Fprintf(fgo2, "\t_Cgo_no_callback(false)\n") - } else { - // skip _Cgo_use when noescape & nocallback both exist, + } + + if noEscape, _ := p.noEscapes[n.C]; !noEscape { + // skip _Cgo_use when noescape exist, // so that the compiler won't force to escape them to heap. fmt.Fprintf(fgo2, "\tif _Cgo_always_false {\n") if d.Type.Params != nil { diff --git a/src/runtime/cgo.go b/src/runtime/cgo.go index eb1335f495afd6..40c8c748d3e56e 100644 --- a/src/runtime/cgo.go +++ b/src/runtime/cgo.go @@ -64,5 +64,8 @@ var cgo_yield = &_cgo_yield func cgoNoCallback(v bool) { g := getg() + if g.nocgocallback && v { + panic("runtime: unexpected setting cgoNoCallback") + } g.nocgocallback = v } diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index 9954d397b75c8a..bdd1f63cb1c6ef 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -761,6 +761,14 @@ func TestCgoNoCallback(t *testing.T) { } } +func TestCgoNoEscape(t *testing.T) { + got := runTestProg(t, "testprogcgo", "CgoNoEscape") + want := "OK\n" + if got != want { + t.Fatalf("want %s, got %s\n", want, got) + } +} + func TestCgoTracebackGoroutineProfile(t *testing.T) { output := runTestProg(t, "testprogcgo", "GoroutineProfile") want := "OK\n" diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 2aa69057ea2efd..a207859f5a3626 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -481,8 +481,8 @@ type g struct { parkingOnChan atomic.Bool raceignore int8 // ignore race detection events - tracking bool // whether we're tracking this G for sched latency statistics nocgocallback bool // whether disable callback from C + tracking bool // whether we're tracking this G for sched latency statistics trackingSeq uint8 // used to decide whether to track this G trackingStamp int64 // timestamp of when the G last started being tracked runnableTime int64 // the amount of time spent runnable, cleared when running, only used when tracking diff --git a/src/runtime/testdata/testprogcgo/CgoNoCallback.go b/src/runtime/testdata/testprogcgo/CgoNoCallback.go index 5e8432341f26ed..8cbbfd1957d668 100644 --- a/src/runtime/testdata/testprogcgo/CgoNoCallback.go +++ b/src/runtime/testdata/testprogcgo/CgoNoCallback.go @@ -1,15 +1,14 @@ -// Copyright 2020 The Go Authors. All rights reserved. +// 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 main -// #cgo noescape/nocallback annotations for a C function means it should not callback to Go. +// #cgo nocallback annotations for a C function means it should not callback to Go. // But it do callback to go in this test, Go should crash here. /* #cgo nocallback runCShouldNotCallback -#cgo noescape runCShouldNotCallback extern void runCShouldNotCallback(); */ diff --git a/src/runtime/testdata/testprogcgo/CgoNoEscape.go b/src/runtime/testdata/testprogcgo/CgoNoEscape.go new file mode 100644 index 00000000000000..47d2befd717946 --- /dev/null +++ b/src/runtime/testdata/testprogcgo/CgoNoEscape.go @@ -0,0 +1,78 @@ +// 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 main + +// #cgo noescape annotations for a C function means its arguments won't escape to heap. + +/* +#cgo noescape runCWithNoEscape + +void runCWithNoEscape(void *p) { +} +void runCWithoutNoEscape(void *p) { +} +*/ +import "C" + +import ( + "fmt" + "runtime" + "runtime/debug" + "unsafe" +) + +var NUM uint64 = 100 + +func init() { + register("CgoNoEscape", CgoNoEscape) +} + +//go:noinline +func withNoEscape() { + var str string + C.runCWithNoEscape(unsafe.Pointer(&str)) +} + +//go:noinline +func withoutNoEscape() { + var str string + C.runCWithoutNoEscape(unsafe.Pointer(&str)) +} + +func CgoNoEscape() { + // make GC nearly stop + debug.SetGCPercent(1000) + + var stats runtime.MemStats + runtime.ReadMemStats(&stats) + preHeapObjects := stats.HeapObjects + + for i := uint64(0); i < NUM; i++ { + withNoEscape() + } + + runtime.ReadMemStats(&stats) + nowHeapObjects := stats.HeapObjects + + if nowHeapObjects-preHeapObjects >= NUM { + fmt.Printf("too much heap objects allocated, pre: %v, now: %v\n", preHeapObjects, nowHeapObjects) + } + + runtime.ReadMemStats(&stats) + preHeapObjects = stats.HeapObjects + + for i := uint64(0); i < NUM; i++ { + withoutNoEscape() + } + + runtime.ReadMemStats(&stats) + nowHeapObjects = stats.HeapObjects + + if nowHeapObjects-preHeapObjects < NUM { + fmt.Printf("too less too heap objects allocated, pre: %v, now: %v\n", preHeapObjects, nowHeapObjects) + } + + fmt.Println("OK") +} From 38aa1e930fc8e86e6295b55ec9fb2fb9935b32d0 Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Sun, 20 Aug 2023 19:25:57 +0800 Subject: [PATCH 08/14] add noescape for _cgoCheckPointer and _cgoCheckResult. Signed-off-by: doujiang24 --- src/cmd/cgo/out.go | 2 ++ src/runtime/testdata/testprogcgo/CgoNoEscape.go | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go index 243e658837bcd2..01841994e81183 100644 --- a/src/cmd/cgo/out.go +++ b/src/cmd/cgo/out.go @@ -1628,9 +1628,11 @@ const goProlog = ` func _cgo_runtime_cgocall(unsafe.Pointer, uintptr) int32 //go:linkname _cgoCheckPointer runtime.cgoCheckPointer +//go:noescape func _cgoCheckPointer(interface{}, interface{}) //go:linkname _cgoCheckResult runtime.cgoCheckResult +//go:noescape func _cgoCheckResult(interface{}) ` diff --git a/src/runtime/testdata/testprogcgo/CgoNoEscape.go b/src/runtime/testdata/testprogcgo/CgoNoEscape.go index 47d2befd717946..4d19a0592d5941 100644 --- a/src/runtime/testdata/testprogcgo/CgoNoEscape.go +++ b/src/runtime/testdata/testprogcgo/CgoNoEscape.go @@ -6,6 +6,12 @@ package main // #cgo noescape annotations for a C function means its arguments won't escape to heap. +// We assume that there won't be 100 new allocated heap objects in other places, +// i.e. runtime.ReadMemStats or other runtime background works. +// So, the tests are: +// 1. at least 100 new allocated heap objects after invoking withoutNoEscape 100 times. +// 2. less than 100 new allocated heap objects after invoking withoutNoEscape 100 times. + /* #cgo noescape runCWithNoEscape From 454a3544374a9057ba5bf44ece8c09eb52aae911 Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Sun, 20 Aug 2023 23:57:12 +0800 Subject: [PATCH 09/14] throw error when no match C function name. Signed-off-by: doujiang24 --- src/cmd/cgo/main.go | 19 +++++++++++++++ src/runtime/crash_cgo_test.go | 8 +++++++ .../testbadcgo/cgoNotMatchedCFunction.go | 24 +++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 src/runtime/testdata/testbadcgo/cgoNotMatchedCFunction.go diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go index 5b88c05e0f7f1a..7227d99ed2964e 100644 --- a/src/cmd/cgo/main.go +++ b/src/cmd/cgo/main.go @@ -418,6 +418,25 @@ func main() { p.writeOutput(f, input) } } + cFunctions := make(map[string]bool) + for _, key := range nameKeys(p.Name) { + n := p.Name[key] + if n.FuncType != nil { + cFunctions[n.C] = true + } + } + + for funcName, _ := range p.noEscapes { + if found, _ := cFunctions[funcName]; !found { + error_(token.NoPos, "#cgo noescape %s: no matched C function", funcName) + } + } + + for funcName, _ := range p.noCallbacks { + if found, _ := cFunctions[funcName]; !found { + error_(token.NoPos, "#cgo nocallback %s: no matched C function", funcName) + } + } if !*godefs { p.writeDefs() diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index bdd1f63cb1c6ef..eb80f2cc8574c3 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -769,6 +769,14 @@ func TestCgoNoEscape(t *testing.T) { } } +func TestCgoNoMatchedCFunction(t *testing.T) { + _, err := buildTestProg(t, "testbadcgo") + want := "no matched C function" + if !strings.Contains(err.Error(), want) { + t.Fatalf("did not see %q in error: %q", want, err.Error()) + } +} + func TestCgoTracebackGoroutineProfile(t *testing.T) { output := runTestProg(t, "testprogcgo", "GoroutineProfile") want := "OK\n" diff --git a/src/runtime/testdata/testbadcgo/cgoNotMatchedCFunction.go b/src/runtime/testdata/testbadcgo/cgoNotMatchedCFunction.go new file mode 100644 index 00000000000000..d7dc371441e0af --- /dev/null +++ b/src/runtime/testdata/testbadcgo/cgoNotMatchedCFunction.go @@ -0,0 +1,24 @@ +// 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 testbadcgo + +// no matched C function. + +/* +#cgo noescape noMatchedCFunction +*/ +import "C" + +import ( + "fmt" +) + +func init() { + register("CgoNoMatchedCFunction", CgoNotMatchedCFunction) +} + +func CgoNotMatchedCFunction() { + fmt.Println("OK") +} From b37037acd9d42edea6ea9c2d4506e601b60f1e9d Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Mon, 21 Aug 2023 11:05:29 +0800 Subject: [PATCH 10/14] rename files. Signed-off-by: doujiang24 --- .../{cgoNotMatchedCFunction.go => cgonotmatchedcfunction.go} | 0 .../testdata/testprogcgo/{CgoNoCallback.c => cgonocallback.c} | 0 .../testdata/testprogcgo/{CgoNoCallback.go => cgonocallback.go} | 0 .../testdata/testprogcgo/{CgoNoEscape.go => cgonoescape.go} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename src/runtime/testdata/testbadcgo/{cgoNotMatchedCFunction.go => cgonotmatchedcfunction.go} (100%) rename src/runtime/testdata/testprogcgo/{CgoNoCallback.c => cgonocallback.c} (100%) rename src/runtime/testdata/testprogcgo/{CgoNoCallback.go => cgonocallback.go} (100%) rename src/runtime/testdata/testprogcgo/{CgoNoEscape.go => cgonoescape.go} (100%) diff --git a/src/runtime/testdata/testbadcgo/cgoNotMatchedCFunction.go b/src/runtime/testdata/testbadcgo/cgonotmatchedcfunction.go similarity index 100% rename from src/runtime/testdata/testbadcgo/cgoNotMatchedCFunction.go rename to src/runtime/testdata/testbadcgo/cgonotmatchedcfunction.go diff --git a/src/runtime/testdata/testprogcgo/CgoNoCallback.c b/src/runtime/testdata/testprogcgo/cgonocallback.c similarity index 100% rename from src/runtime/testdata/testprogcgo/CgoNoCallback.c rename to src/runtime/testdata/testprogcgo/cgonocallback.c diff --git a/src/runtime/testdata/testprogcgo/CgoNoCallback.go b/src/runtime/testdata/testprogcgo/cgonocallback.go similarity index 100% rename from src/runtime/testdata/testprogcgo/CgoNoCallback.go rename to src/runtime/testdata/testprogcgo/cgonocallback.go diff --git a/src/runtime/testdata/testprogcgo/CgoNoEscape.go b/src/runtime/testdata/testprogcgo/cgonoescape.go similarity index 100% rename from src/runtime/testdata/testprogcgo/CgoNoEscape.go rename to src/runtime/testdata/testprogcgo/cgonoescape.go From f01351d0e0ce8995bc1c7f1c3f3ebea2dd11cb8d Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Tue, 22 Aug 2023 14:56:46 +0800 Subject: [PATCH 11/14] revert maps.Copy. Signed-off-by: doujiang24 --- src/cmd/cgo/main.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go index 7227d99ed2964e..7871866d086470 100644 --- a/src/cmd/cgo/main.go +++ b/src/cmd/cgo/main.go @@ -18,7 +18,6 @@ import ( "go/token" "internal/buildcfg" "io" - "maps" "os" "path/filepath" "reflect" @@ -514,8 +513,12 @@ func (p *Package) Record(f *File) { } // merge nocallback & noescape - maps.Copy(p.noCallbacks, f.NoCallbacks) - maps.Copy(p.noEscapes, f.NoEscapes) + for k, v := range f.NoCallbacks { + p.noCallbacks[k] = v + } + for k, v := range f.NoEscapes { + p.noEscapes[k] = v + } if f.ExpFunc != nil { p.ExpFunc = append(p.ExpFunc, f.ExpFunc...) From d5dd38d998b18660fe1d2480dbf3b011b6a7d51c Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Wed, 23 Aug 2023 10:16:34 +0800 Subject: [PATCH 12/14] address comments. Signed-off-by: doujiang24 --- src/cmd/cgo/main.go | 8 ++++---- src/cmd/cgo/out.go | 8 ++++---- src/runtime/crash_cgo_test.go | 4 ++-- .../testdata/testprogcgo/cgonoescape.go | 18 +++++++++--------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go index 7871866d086470..55f9cdc318b08e 100644 --- a/src/cmd/cgo/main.go +++ b/src/cmd/cgo/main.go @@ -425,14 +425,14 @@ func main() { } } - for funcName, _ := range p.noEscapes { - if found, _ := cFunctions[funcName]; !found { + for funcName := range p.noEscapes { + if _, found := cFunctions[funcName]; !found { error_(token.NoPos, "#cgo noescape %s: no matched C function", funcName) } } - for funcName, _ := range p.noCallbacks { - if found, _ := cFunctions[funcName]; !found { + for funcName := range p.noCallbacks { + if _, found := cFunctions[funcName]; !found { error_(token.NoPos, "#cgo nocallback %s: no matched C function", funcName) } } diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go index 01841994e81183..947c61b5c5970e 100644 --- a/src/cmd/cgo/out.go +++ b/src/cmd/cgo/out.go @@ -614,7 +614,7 @@ func (p *Package) writeDefsFunc(fgo2 io.Writer, n *Name, callsMalloc *bool) { arg = "uintptr(unsafe.Pointer(&r1))" } - noCallback, _ := p.noCallbacks[n.C] + noCallback := p.noCallbacks[n.C] if noCallback { // disable cgocallback, will check it in runtime. fmt.Fprintf(fgo2, "\t_Cgo_no_callback(true)\n") @@ -632,9 +632,9 @@ func (p *Package) writeDefsFunc(fgo2 io.Writer, n *Name, callsMalloc *bool) { fmt.Fprintf(fgo2, "\t_Cgo_no_callback(false)\n") } - if noEscape, _ := p.noEscapes[n.C]; !noEscape { - // skip _Cgo_use when noescape exist, - // so that the compiler won't force to escape them to heap. + // skip _Cgo_use when noescape exist, + // so that the compiler won't force to escape them to heap. + if !p.noEscapes[n.C] { fmt.Fprintf(fgo2, "\tif _Cgo_always_false {\n") if d.Type.Params != nil { for i := range d.Type.Params.List { diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index eb80f2cc8574c3..dc974138e292e8 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -757,7 +757,7 @@ func TestCgoNoCallback(t *testing.T) { got := runTestProg(t, "testprogcgo", "CgoNoCallback") want := "function marked with #cgo nocallback called back into Go" if !strings.Contains(got, want) { - t.Fatalf("did not see %q in output: %q", want, got) + t.Fatalf("did not see %q in output:\n%s", want, got) } } @@ -773,7 +773,7 @@ func TestCgoNoMatchedCFunction(t *testing.T) { _, err := buildTestProg(t, "testbadcgo") want := "no matched C function" if !strings.Contains(err.Error(), want) { - t.Fatalf("did not see %q in error: %q", want, err.Error()) + t.Fatalf("did not see %q in error:\n%s", want, err.Error()) } } diff --git a/src/runtime/testdata/testprogcgo/cgonoescape.go b/src/runtime/testdata/testprogcgo/cgonoescape.go index 4d19a0592d5941..056be44889b264 100644 --- a/src/runtime/testdata/testprogcgo/cgonoescape.go +++ b/src/runtime/testdata/testprogcgo/cgonoescape.go @@ -29,7 +29,7 @@ import ( "unsafe" ) -var NUM uint64 = 100 +const num = 100 func init() { register("CgoNoEscape", CgoNoEscape) @@ -48,36 +48,36 @@ func withoutNoEscape() { } func CgoNoEscape() { - // make GC nearly stop - debug.SetGCPercent(1000) + // make GC stop to see the heap objects allocated + debug.SetGCPercent(-1) var stats runtime.MemStats runtime.ReadMemStats(&stats) preHeapObjects := stats.HeapObjects - for i := uint64(0); i < NUM; i++ { + for i := 0; i < num; i++ { withNoEscape() } runtime.ReadMemStats(&stats) nowHeapObjects := stats.HeapObjects - if nowHeapObjects-preHeapObjects >= NUM { - fmt.Printf("too much heap objects allocated, pre: %v, now: %v\n", preHeapObjects, nowHeapObjects) + if nowHeapObjects-preHeapObjects >= num { + fmt.Printf("too many heap objects allocated, pre: %v, now: %v\n", preHeapObjects, nowHeapObjects) } runtime.ReadMemStats(&stats) preHeapObjects = stats.HeapObjects - for i := uint64(0); i < NUM; i++ { + for i := 0; i < num; i++ { withoutNoEscape() } runtime.ReadMemStats(&stats) nowHeapObjects = stats.HeapObjects - if nowHeapObjects-preHeapObjects < NUM { - fmt.Printf("too less too heap objects allocated, pre: %v, now: %v\n", preHeapObjects, nowHeapObjects) + if nowHeapObjects-preHeapObjects < num { + fmt.Printf("too few heap objects allocated, pre: %v, now: %v\n", preHeapObjects, nowHeapObjects) } fmt.Println("OK") From 6e4adbc07ae6de53280b65ec640262ed3d622e11 Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Thu, 24 Aug 2023 11:35:59 +0800 Subject: [PATCH 13/14] move test to another dir. Signed-off-by: doujiang24 --- .../cgo/internal/testerrors/errors_test.go | 28 +++++++++++++------ .../testdata/notmatchedcfunction.go} | 16 ++--------- src/runtime/crash_cgo_test.go | 8 ------ 3 files changed, 23 insertions(+), 29 deletions(-) rename src/{runtime/testdata/testbadcgo/cgonotmatchedcfunction.go => cmd/cgo/internal/testerrors/testdata/notmatchedcfunction.go} (52%) diff --git a/src/cmd/cgo/internal/testerrors/errors_test.go b/src/cmd/cgo/internal/testerrors/errors_test.go index 486530e186c610..fd522ba4745a83 100644 --- a/src/cmd/cgo/internal/testerrors/errors_test.go +++ b/src/cmd/cgo/internal/testerrors/errors_test.go @@ -39,16 +39,23 @@ func check(t *testing.T, file string) { continue } - _, frag, ok := bytes.Cut(line, []byte("ERROR HERE: ")) - if !ok { - continue + if _, frag, ok := bytes.Cut(line, []byte("ERROR HERE: ")); ok { + re, err := regexp.Compile(fmt.Sprintf(":%d:.*%s", i+1, frag)) + if err != nil { + t.Errorf("Invalid regexp after `ERROR HERE: `: %#q", frag) + continue + } + errors = append(errors, re) } - re, err := regexp.Compile(fmt.Sprintf(":%d:.*%s", i+1, frag)) - if err != nil { - t.Errorf("Invalid regexp after `ERROR HERE: `: %#q", frag) - continue + + if _, frag, ok := bytes.Cut(line, []byte("ERROR MESSAGE: ")); ok { + re, err := regexp.Compile(string(frag)) + if err != nil { + t.Errorf("Invalid regexp after `ERROR MESSAGE: `: %#q", frag) + continue + } + errors = append(errors, re) } - errors = append(errors, re) } if len(errors) == 0 { t.Fatalf("cannot find ERROR HERE") @@ -165,3 +172,8 @@ func TestMallocCrashesOnNil(t *testing.T) { t.Fatalf("succeeded unexpectedly") } } + +func TestNotMatchedCFunction(t *testing.T) { + file := "notmatchedcfunction.go" + check(t, file) +} diff --git a/src/runtime/testdata/testbadcgo/cgonotmatchedcfunction.go b/src/cmd/cgo/internal/testerrors/testdata/notmatchedcfunction.go similarity index 52% rename from src/runtime/testdata/testbadcgo/cgonotmatchedcfunction.go rename to src/cmd/cgo/internal/testerrors/testdata/notmatchedcfunction.go index d7dc371441e0af..46afeefcc0f65c 100644 --- a/src/runtime/testdata/testbadcgo/cgonotmatchedcfunction.go +++ b/src/cmd/cgo/internal/testerrors/testdata/notmatchedcfunction.go @@ -2,23 +2,13 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package testbadcgo - -// no matched C function. +package main /* +// ERROR MESSAGE: #cgo noescape noMatchedCFunction: no matched C function #cgo noescape noMatchedCFunction */ import "C" -import ( - "fmt" -) - -func init() { - register("CgoNoMatchedCFunction", CgoNotMatchedCFunction) -} - -func CgoNotMatchedCFunction() { - fmt.Println("OK") +func main() { } diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go index dc974138e292e8..88044caacf7442 100644 --- a/src/runtime/crash_cgo_test.go +++ b/src/runtime/crash_cgo_test.go @@ -769,14 +769,6 @@ func TestCgoNoEscape(t *testing.T) { } } -func TestCgoNoMatchedCFunction(t *testing.T) { - _, err := buildTestProg(t, "testbadcgo") - want := "no matched C function" - if !strings.Contains(err.Error(), want) { - t.Fatalf("did not see %q in error:\n%s", want, err.Error()) - } -} - func TestCgoTracebackGoroutineProfile(t *testing.T) { output := runTestProg(t, "testprogcgo", "GoroutineProfile") want := "OK\n" From f1a17b08b0590eca2670e404bbfedad5461df72f Mon Sep 17 00:00:00 2001 From: doujiang24 Date: Sat, 26 Aug 2023 01:01:55 +0800 Subject: [PATCH 14/14] fix doc. Signed-off-by: doujiang24 --- src/cmd/cgo/doc.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmd/cgo/doc.go b/src/cmd/cgo/doc.go index 0e708fb3b44ac4..894df2d836de9c 100644 --- a/src/cmd/cgo/doc.go +++ b/src/cmd/cgo/doc.go @@ -432,7 +432,7 @@ pointer safely, the program may crash or see memory corruption. For example: - // #cgo nocallback cFunctionName + // #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 @@ -442,7 +442,7 @@ Go code, the program will panic. For example: - // #cgo noescape cFunctionName + // #cgo nocallback cFunctionName # Special cases