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)
>         <autogenerated>: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)
>         <autogenerated>: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 <stdio.h>
> 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 <[email protected]>
Fixes #75856

Author: ariel-anieli

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   // expected regexp of error message
 }
 
 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: `.*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: `.*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,18 @@ func testOne(t *testing.T, pt ptrTest, exe, exe2 string) {
 	}
 
 	buf, err := runcmd(cgocheck)
+
+	// TODO: add a regexp on each ptrTest
+	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)
 		}

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,15 +628,15 @@ 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)
 		}
 		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)
+}