]> Cypherpunks repositories - gostls13.git/commitdiff
runtime/cgo: improve error messages after pointer panic
authorAriel Otilibili <otilibil@eurecom.fr>
Wed, 5 Nov 2025 21:00:52 +0000 (21:00 +0000)
committert hepudds <thepudds1460@gmail.com>
Fri, 7 Nov 2025 20:55:16 +0000 (12:55 -0800)
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,

     1  package main
     2
     3  /*
     4     #include <stdio.h>
     5
     6     extern void* GoFoo();
     7
     8     static void CFoo() { GoFoo();}
     9   */
    10  import (
    11          "C"
    12  )
    13
    14  //export GoFoo
    15  func GoFoo() map[int]int {
    16          return map[int]int{0: 1,}
    17  }
    18
    19  func main() {
    20          C.CFoo();
    21  }

This error shows up at runtime:

    panic: runtime error: cgo result is unpinned Go pointer or points to unpinned Go pointer

    goroutine 1 [running]:
    main._Cfunc_CFoo()
            _cgo_gotypes.go:46 +0x3a
    main.main()
            /mnt/tmp/parse.go:20 +0xf
    exit status 2

GoFoo is the faulty Go function; it is not mentioned in the error message.

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:15: result of Go function GoFoo called from cgo is unpinned Go map or points to unpinned Go map

    goroutine 1 [running]:
    main._Cfunc_CFoo()
            _cgo_gotypes.go:46 +0x3a
    main.main()
            /mnt/tmp/parse.go:20 +0xf
    exit status 2

2. cgoCheckPointer

When a pointer to an unpinned Go pointer is passed to a C function,

     1  package main
     2
     3  /*
     4  #include <stdio.h>
     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 <iant@golang.org>
Fixes #75856

Change-Id: Ia72f01df016feeae0cddb2558ced51a1b07e4486
GitHub-Last-Rev: 76257c7dd7c52a3d88234d43ec7f22bda81edcdd
GitHub-Pull-Request: golang/go#75894
Reviewed-on: https://go-review.googlesource.com/c/go/+/711801
Reviewed-by: Funda Secgin <fundasecgin73@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
src/cmd/cgo/internal/testerrors/ptr_test.go
src/runtime/cgocall.go

index beba0d26ac1818d6a7a93da7dca505fe272b2742..bc1cc1c6e08ed82448e51682b4acb579a391f59a 100644 (file)
@@ -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)
                }
index 9b9a47b87eab75d516d6d5bc9736a3d685f012c4..a53fd6da340190793840944b71fc320095a0e465 100644 (file)
@@ -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)
+}