From 78e12634b50c24181d11f5e53aa1eacc509e7c8d Mon Sep 17 00:00:00 2001 From: Ariel Otilibili Date: Wed, 22 Oct 2025 18:55:35 +0200 Subject: [PATCH] runtime/cgo: improve error messages after ptr panic This commit improves the error messages after panics due to the sharing of an unpinned Go pointer (or a pointer to an unpinned Go pointer) between Go and C. This occurs when it is: 1. returned from Go to C (through cgoCheckResult) 2. passed as argument to a C function (through cgoCheckPointer). An unpinned Go pointer refers to a memory location that might be moved or freed by the garbage collector. Therefore: - change the signature of cgoCheckArg (it does the real work behind cgoCheckResult and cgoCheckPointer) - change the signature of cgoCheckUnknownPointer (called by cgoCheckArg for checking unexpected pointers) - introduce cgoFormatErr (it formats an error message with the caller name) - update the cgo pointer tests (add new tests, and a field errTextRegexp to the struct ptrTest) - remove an unused assignment in TestPointerChecks. 1. cgoCheckResult When an unpinned Go pointer (or a pointer to an unpinned Go pointer) is returned from Go to C, > package main > > import ( > "C" > ) > > //export foo > func foo() map[int]int { > return map[int]int{0: 1,} > } This error shows up at runtime: > panic: runtime error: cgo result is unpinned Go pointer or points to unpinned Go pointer > > goroutine 17 [running, locked to thread]: > panic({0x78ac36001d40?, 0xc000194000?}) > /usr/lib/go/src/runtime/panic.go:802 +0x168 > runtime.cgoCheckArg(0x78ac36002f40, 0xc000190000, 0xd8?, 0x0, {0x78ac35fdda62, 0x42}) > /usr/lib/go/src/runtime/cgocall.go:628 +0x4c5 > runtime.cgoCheckResult({0x78ac36002f40, 0xc000190000}) > /usr/lib/go/src/runtime/cgocall.go:795 +0x4b > _cgoexp_1c29bd2c0b96_foo(0x7fffc16d87b0) > _cgo_gotypes.go:46 +0x6f > runtime.cgocallbackg1(0x78ac35fd4d20, 0x7fffc16d87b0, 0x0) > /usr/lib/go/src/runtime/cgocall.go:446 +0x289 > runtime.cgocallbackg(0x78ac35fd4d20, 0x7fffc16d87b0, 0x0) > /usr/lib/go/src/runtime/cgocall.go:350 +0x132 > runtime.cgocallbackg(0x78ac35fd4d20, 0x7fffc16d87b0, 0x0) > :1 +0x2b > runtime.cgocallback(0x0, 0x0, 0x0) > /usr/lib/go/src/runtime/asm_amd64.s:1082 +0xcd > runtime.goexit({}) > /usr/lib/go/src/runtime/asm_amd64.s:1693 +0x1 _cgoexp_1c29bd2c0b96_foo is the faulty caller; it is not obvious to find it in the stack trace. Moreover the error does say which kind of pointer caused the panic; for instance, a Go map. Retrieve caller name and pointer kind; use them in the error message: > panic: runtime error: result of cgo function foo is unpinned Go map or points to unpinned Go map > > goroutine 17 [running, locked to thread]: > panic({0x76f2fd69b3c0?, 0x1fba8c79c000?}) > /mnt/go/src/runtime/panic.go:877 +0x16f > runtime.cgoCheckArg(0x76f2fd69c5c0, 0x1fba8c790000, 0x0?, 0x0, 0x1) > /mnt/go/src/runtime/cgocall.go:631 +0x499 > runtime.cgoCheckResult({0x76f2fd69c5c0, 0x1fba8c790000}) > /mnt/go/src/runtime/cgocall.go:798 +0x45 > _cgoexp_1c29bd2c0b96_foo(0x7ffd0803b8c0) > _cgo_gotypes.go:46 +0x6f > runtime.cgocallbackg1(0x76f2fd667680, 0x7ffd0803b8c0, 0x0) > /mnt/go/src/runtime/cgocall.go:446 +0x289 > runtime.cgocallbackg(0x76f2fd667680, 0x7ffd0803b8c0, 0x0) > /mnt/go/src/runtime/cgocall.go:350 +0x132 > runtime.cgocallbackg(0x76f2fd667680, 0x7ffd0803b8c0, 0x0) > :1 +0x2b > runtime.cgocallback(0x0, 0x0, 0x0) > /mnt/go/src/runtime/asm_amd64.s:1101 +0xcd > runtime.goexit({}) > /mnt/go/src/runtime/asm_amd64.s:1712 +0x1 2. cgoCheckPointer When an unpinned Go pointer (or a pointer to an unpinned Go pointer) is passed to a C function, > package main > > /* > #include > void foo(void *bar) {} > */ > import "C" > import "unsafe" > > func main() { > m := map[int]int{0: 1,} > C.foo(unsafe.Pointer(&m)) > } This error shows up at runtime: > panic: runtime error: cgo argument has Go pointer to unpinned Go pointer > > goroutine 1 [running]: > main.main.func1(...) > /mnt/chexit/cgomap.go:12 > main.main() > /mnt/chexit/cgomap.go:12 +0x91 > exit status 2 Retrieve callee name and pointer kind; use them in the error message. > panic: runtime error: cgo argument of function main.main.func1 has Go pointer to unpinned Go map > > goroutine 1 [running]: > main.main.func1(...) > /mnt/chexit/cgomap.go:12 > main.main() > /mnt/chexit/cgomap.go:12 +0x9b > exit status 2 Link: https://pkg.go.dev/cmd/cgo#hdr-Passing_pointers Suggested-by: Ian Lance Taylor Fixes #75856 --- src/cmd/cgo/internal/testerrors/ptr_test.go | 50 ++++++++--- src/runtime/cgocall.go | 96 ++++++++++++++++++--- 2 files changed, 122 insertions(+), 24 deletions(-) diff --git a/src/cmd/cgo/internal/testerrors/ptr_test.go b/src/cmd/cgo/internal/testerrors/ptr_test.go index beba0d26ac1818..3eb555ec840b73 100644 --- a/src/cmd/cgo/internal/testerrors/ptr_test.go +++ b/src/cmd/cgo/internal/testerrors/ptr_test.go @@ -14,6 +14,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "slices" "strings" "sync/atomic" @@ -24,15 +25,16 @@ var tmp = flag.String("tmp", "", "use `dir` for temporary files and do not clean // ptrTest is the tests without the boilerplate. type ptrTest struct { - name string // for reporting - c string // the cgo comment - c1 string // cgo comment forced into non-export cgo file - imports []string // a list of imports - support string // supporting functions - body string // the body of the main function - extra []extra // extra files - fail bool // whether the test should fail - expensive bool // whether the test requires the expensive check + name string // for reporting + c string // the cgo comment + c1 string // cgo comment forced into non-export cgo file + imports []string // a list of imports + support string // supporting functions + body string // the body of the main function + extra []extra // extra files + fail bool // whether the test should fail + expensive bool // whether the test requires the expensive check + errTextRegexp string // error text regexp; if empty, use the pattern `.*unpinned Go.*` } type extra struct { @@ -489,6 +491,27 @@ var ptrTests = []ptrTest{ body: `i := 0; a := &[2]unsafe.Pointer{nil, unsafe.Pointer(&i)}; C.f45(&a[0])`, fail: true, }, + { + // Passing a Go map as argument to C. + name: "argmap", + c: `void f46(void* p) {}`, + imports: []string{"unsafe"}, + body: `m := map[int]int{0: 1,}; C.f46(unsafe.Pointer(&m))`, + fail: true, + errTextRegexp: `.*cgo argument of function .*argmap.*has Go pointer to unpinned Go map.*`, + }, + { + // Returning a Go map to C. + name: "retmap", + c: `extern void f47();`, + support: `//export GoMap47 + func GoMap47() map[int]int { return map[int]int{0: 1,} }`, + body: `C.f47()`, + c1: `extern void* GoMap47(); + void f47() { GoMap47(); }`, + fail: true, + errTextRegexp: `.*cgo function GoMap47 is unpinned Go map or points to unpinned Go map.*`, + }, } func TestPointerChecks(t *testing.T) { @@ -519,7 +542,6 @@ func TestPointerChecks(t *testing.T) { // after testOne finishes. var pending int32 for _, pt := range ptrTests { - pt := pt t.Run(pt.name, func(t *testing.T) { atomic.AddInt32(&pending, +1) defer func() { @@ -690,11 +712,17 @@ func testOne(t *testing.T, pt ptrTest, exe, exe2 string) { } buf, err := runcmd(cgocheck) + + var pattern string = pt.errTextRegexp + if pt.errTextRegexp == "" { + pattern = `.*unpinned Go.*` + } + if pt.fail { if err == nil { t.Logf("%s", buf) t.Fatalf("did not fail as expected") - } else if !bytes.Contains(buf, []byte("Go pointer")) { + } else if ok, _ := regexp.Match(pattern, buf); !ok { t.Logf("%s", buf) t.Fatalf("did not print expected error (failed with %v)", err) } diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go index 18e1dc8bafc4aa..3147f337aade01 100644 --- a/src/runtime/cgocall.go +++ b/src/runtime/cgocall.go @@ -591,15 +591,18 @@ func cgoCheckPointer(ptr any, arg any) { cgoCheckArg(t, ep.data, !t.IsDirectIface(), top, cgoCheckPointerFail) } -const cgoCheckPointerFail = "cgo argument has Go pointer to unpinned Go pointer" -const cgoResultFail = "cgo result is unpinned Go pointer or points to unpinned Go pointer" +type cgoErrorMsg int +const ( + cgoCheckPointerFail cgoErrorMsg = iota + cgoResultFail +) // cgoCheckArg is the real work of cgoCheckPointer. The argument p // is either a pointer to the value (of type t), or the value itself, // depending on indir. The top parameter is whether we are at the top // level, where Go pointers are allowed. Go pointers to pinned objects are // allowed as long as they don't reference other unpinned pointers. -func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) { +func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg cgoErrorMsg) { if !t.Pointers() || p == nil { // If the type has no pointers there is nothing to do. return @@ -625,7 +628,7 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) { // These types contain internal pointers that will // always be allocated in the Go heap. It's never OK // to pass them to C. - panic(errorString(msg)) + panic(cgoFormatErr(msg, t.Kind())) case abi.Func: if indir { p = *(*unsafe.Pointer)(p) @@ -633,7 +636,7 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) { if !cgoIsGoPointer(p) { return } - panic(errorString(msg)) + panic(cgoFormatErr(msg, t.Kind())) case abi.Interface: it := *(**_type)(p) if it == nil { @@ -643,14 +646,14 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) { // constant. A type not known at compile time will be // in the heap and will not be OK. if inheap(uintptr(unsafe.Pointer(it))) { - panic(errorString(msg)) + panic(cgoFormatErr(msg, t.Kind())) } p = *(*unsafe.Pointer)(add(p, goarch.PtrSize)) if !cgoIsGoPointer(p) { return } if !top && !isPinned(p) { - panic(errorString(msg)) + panic(cgoFormatErr(msg, t.Kind())) } cgoCheckArg(it, p, !it.IsDirectIface(), false, msg) case abi.Slice: @@ -661,7 +664,7 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) { return } if !top && !isPinned(p) { - panic(errorString(msg)) + panic(cgoFormatErr(msg, t.Kind())) } if !st.Elem.Pointers() { return @@ -676,7 +679,7 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) { return } if !top && !isPinned(ss.str) { - panic(errorString(msg)) + panic(cgoFormatErr(msg, t.Kind())) } case abi.Struct: st := (*structtype)(unsafe.Pointer(t)) @@ -705,7 +708,7 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) { return } if !top && !isPinned(p) { - panic(errorString(msg)) + panic(cgoFormatErr(msg, t.Kind())) } cgoCheckUnknownPointer(p, msg) @@ -716,7 +719,7 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) { // memory. It checks whether that Go memory contains any other // pointer into unpinned Go memory. If it does, we panic. // The return values are unused but useful to see in panic tracebacks. -func cgoCheckUnknownPointer(p unsafe.Pointer, msg string) (base, i uintptr) { +func cgoCheckUnknownPointer(p unsafe.Pointer, msg cgoErrorMsg) (base, i uintptr) { if inheap(uintptr(p)) { b, span, _ := findObject(uintptr(p), 0, 0) base = b @@ -731,7 +734,7 @@ func cgoCheckUnknownPointer(p unsafe.Pointer, msg string) (base, i uintptr) { } pp := *(*unsafe.Pointer)(unsafe.Pointer(addr)) if cgoIsGoPointer(pp) && !isPinned(pp) { - panic(errorString(msg)) + panic(cgoFormatErr(msg, abi.Pointer)) } } return @@ -741,7 +744,7 @@ func cgoCheckUnknownPointer(p unsafe.Pointer, msg string) (base, i uintptr) { if cgoInRange(p, datap.data, datap.edata) || cgoInRange(p, datap.bss, datap.ebss) { // We have no way to know the size of the object. // We have to assume that it might contain a pointer. - panic(errorString(msg)) + panic(cgoFormatErr(msg, abi.Pointer)) } // In the text or noptr sections, we know that the // pointer does not point to a Go pointer. @@ -794,3 +797,70 @@ func cgoCheckResult(val any) { t := ep._type cgoCheckArg(t, ep.data, !t.IsDirectIface(), false, cgoResultFail) } + +// cgoFormatErr is called to format an error message with the caller name. +func cgoFormatErr(error cgoErrorMsg, kind abi.Kind) errorString { + var msg, kindname string + var cgoFunction string = "unknown" + var offset int + + // We expect one of these abi.Kind from cgoCheckArg + switch kind { + case abi.Chan: + kindname = "channel" + case abi.Func: + kindname = "function" + case abi.Interface: + kindname = "interface" + case abi.Map: + kindname = "map" + case abi.Pointer: + kindname = "pointer" + case abi.Slice: + kindname = "slice" + case abi.String: + kindname = "string" + case abi.Struct: + kindname = "struct" + case abi.UnsafePointer: + kindname = "unsafe pointer" + default: + kindname = "pointer" + } + + // The cgo function name might need an offset to be obtained + if error == cgoResultFail { + offset = 21 + } + + // Relatively to cgoFormatErr, this is the stack frame: + // 0. cgoFormatErr + // 1. cgoCheckArg or cgoCheckUnknownPointer + // 2. cgoCheckPointer or cgoCheckResult + // 3. cgo function + pc, _, _, ok := Caller(3) + if ok { + function := FuncForPC(pc) + + if function != nil { + // Expected format of cgo function name: + // - caller: _cgoexp_3c910ddb72c4_foo + // - callee: Module.Function.funcX + if offset > len(function.Name()) { + cgoFunction = function.Name() + } else { + cgoFunction = function.Name()[offset:] + } + } + } + + if error == cgoResultFail { + msg = "result of cgo function " + cgoFunction + msg += " is unpinned Go " + kindname + " or points to unpinned Go " + kindname + } else if error == cgoCheckPointerFail { + msg = "cgo argument of function " + cgoFunction + msg += " has Go pointer to unpinned Go " + kindname + } + + return errorString(msg) +}