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,

     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: 76257c7dd7
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

@@ -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
+	name          string   // for reporting
-	c         string   // the cgo comment
+	c             string   // the cgo comment
-	c1        string   // cgo comment forced into non-export cgo file
+	c1            string   // cgo comment forced into non-export cgo file
-	imports   []string // a list of imports
+	imports       []string // a list of imports
-	support   string   // supporting functions
+	support       string   // supporting functions
-	body      string   // the body of the main function
+	body          string   // the body of the main function
-	extra     []extra  // extra files
+	extra         []extra  // extra files
-	fail      bool     // whether the test should fail
+	fail          bool     // whether the test should fail
-	expensive bool     // whether the test requires the expensive check
+	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)
 		}

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"
+type cgoErrorMsg int
-const cgoResultFail = "cgo result is unpinned Go pointer or points to unpinned Go pointer"
+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)
+}