From 76257c7dd7c52a3d88234d43ec7f22bda81edcdd Mon Sep 17 00:00:00 2001 From: Ariel Otilibili Date: Tue, 4 Nov 2025 19:22:54 +0100 Subject: [PATCH] runtime/cgo: improve error messages after pointer panic This CL 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 is called by cgoCheckArg and cgoCheckUnknownPointer to format panic error messages) - update the cgo pointer tests (add new tests, and a field errTextRegexp to the struct ptrTest) - remove a loop variable in TestPointerChecks (similar to CL 711640). 1. cgoCheckResult When an unpinned Go pointer (or a pointer to an unpinned Go pointer) is returned from Go to C, $ cat -n parse.go 1 package main 2 3 import ( 4 "C" 5 ) 6 7 //export foo 8 func foo() map[int]int { 9 return map[int]int{0: 1,} 10 } 11 12 func main() { 13 } $ cat -n parse.py 1 import os 2 from cffi import FFI 3 4 cffi = FFI() 5 cffi.cdef("char* foo();") 6 dll = cffi.dlopen(os.path.abspath("parse.so")) 7 8 dll.foo() 9 cffi.dlclose(dll) $ go build -buildmode=c-shared -o parse.so parse.go $ python parse.py 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({0x7bf1b12ce460?, 0x3c4f1131c000?}) /mnt/go/src/runtime/panic.go:871 +0x16f runtime.cgoCheckArg(0x7bf1b12cf660, 0x3c4f11318000, 0xd8?, 0x0, {0x7bf1b12a341c, 0x42}) /mnt/go/src/runtime/cgocall.go:628 +0x4c5 runtime.cgoCheckResult({0x7bf1b12cf660, 0x3c4f11318000}) /mnt/go/src/runtime/cgocall.go:795 +0x4b _cgoexp_1c29bd2c0b96_foo(0x7ffd49d02380) /mnt/tmp/parse.go:8 +0x6f runtime.cgocallbackg1(0x7bf1b1299520, 0x7ffd49d02380, 0x0) /mnt/go/src/runtime/cgocall.go:446 +0x289 runtime.cgocallbackg(0x7bf1b1299520, 0x7ffd49d02380, 0x0) /mnt/go/src/runtime/cgocall.go:350 +0x132 runtime.cgocallbackg(0x7bf1b1299520, 0x7ffd49d02380, 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 foo is the faulty Go function; it is not obvious to find it in the stack trace: the stack trace mentions its cgo wrapper (_cgoexp_1c29bd2c0b96_foo). Moreover the error does not say which kind of pointer caused the panic; for instance, a Go map. Retrieve name and file/line of the Go function, as well as the kind of pointer; use them in the error message: panic: runtime error: /mnt/tmp/parse.go:8: result of Go function foo called from cgo is unpinned Go map or points to unpinned Go map goroutine 17 [running, locked to thread]: panic({0x785a83b15cc0?, 0x7ead553a050?}) /mnt/go/src/runtime/panic.go:871 +0x16f runtime.cgoCheckArg(0x785a83b16ec0, 0x7ead55c4060, 0x0?, 0x0, 0x1) /mnt/go/src/runtime/cgocall.go:631 +0x499 runtime.cgoCheckResult({0x785a83b16ec0, 0x7ead55c4060}) /mnt/go/src/runtime/cgocall.go:798 +0x45 _cgoexp_1c29bd2c0b96_foo(0x7ffef3c75230) /mnt/tmp/parse.go:8 +0x6f runtime.cgocallbackg1(0x785a83ae1d80, 0x7ffef3c75230, 0x0) /mnt/go/src/runtime/cgocall.go:446 +0x289 runtime.cgocallbackg(0x785a83ae1d80, 0x7ffef3c75230, 0x0) /mnt/go/src/runtime/cgocall.go:350 +0x132 runtime.cgocallbackg(0x785a83ae1d80, 0x7ffef3c75230, 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 a pointer to an unpinned Go pointer is passed to a C function, 1 package main 2 3 /* 4 #include 5 void foo(void *bar) {} 6 */ 7 import "C" 8 import "unsafe" 9 10 func main() { 11 m := map[int]int{0: 1,} 12 C.foo(unsafe.Pointer(&m)) 13 } 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/tmp/cgomap.go:12 main.main() /mnt/tmp/cgomap.go:12 +0x91 exit status 2 Retrieve kind of pointer; use it in the error message. panic: runtime error: argument of cgo function has Go pointer to unpinned Go map goroutine 1 [running]: main.main.func1(...) /mnt/tmp/cgomap.go:12 main.main() /mnt/tmp/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 | 98 ++++++++++++++++++--- 2 files changed, 124 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..bc1cc1c6e08ed8 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: `.*argument of cgo function 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: `.*result of Go function GoMap47 called from cgo 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 9b9a47b87eab75..a53fd6da340190 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 and cgoCheckResult. // 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,72 @@ func cgoCheckResult(val any) { t := ep._type cgoCheckArg(t, ep.data, !t.IsDirectIface(), false, cgoResultFail) } + +// cgoFormatErr is called by cgoCheckArg and cgoCheckUnknownPointer +// to format panic error messages. +func cgoFormatErr(error cgoErrorMsg, kind abi.Kind) errorString { + var msg, kindname string + var cgoFunction string = "unknown" + var offset int + var buf [20]byte + + // 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, path, line, ok := Caller(3) + if ok && error == cgoResultFail { + function := FuncForPC(pc) + + if function != nil { + // Expected format of cgo function name: + // - caller: _cgoexp_3c910ddb72c4_foo + if offset > len(function.Name()) { + cgoFunction = function.Name() + } else { + cgoFunction = function.Name()[offset:] + } + } + } + + switch error { + case cgoResultFail: + msg = path + ":" + string(itoa(buf[:], uint64(line))) + msg += ": result of Go function " + cgoFunction + " called from cgo" + msg += " is unpinned Go " + kindname + " or points to unpinned Go " + kindname + case cgoCheckPointerFail: + msg += "argument of cgo function has Go pointer to unpinned Go " + kindname + } + + return errorString(msg) +}