Skip to content

Commit fe7c70a

Browse files
committed
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) - change the failure pattern in cgo pointer tests. 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" > > func main(){ > } > > //export foo > func foo(bar *C.char) string { > return C.GoString(bar) > } 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({0x7448b71c85c0?, 0xc00001e050?}) > /usr/lib/go/src/runtime/panic.go:802 +0x168 > runtime.cgoCheckArg(0x7448b71c5f20, 0xc000066e50, 0x0?, 0x0, {0x7448b71a4a62, 0x42}) > /usr/lib/go/src/runtime/cgocall.go:679 +0x35b > runtime.cgoCheckResult({0x7448b71c5f20, 0xc000066e50}) > /usr/lib/go/src/runtime/cgocall.go:795 +0x4b > _cgoexp_62fc19d078a9_foo(0x7ffc8f6ca510) > _cgo_gotypes.go:65 +0x5d > runtime.cgocallbackg1(0x7448b719b6c0, 0x7ffc8f6ca510, 0x0) > /usr/lib/go/src/runtime/cgocall.go:446 +0x289 > runtime.cgocallbackg(0x7448b719b6c0, 0x7ffc8f6ca510, 0x0) > /usr/lib/go/src/runtime/cgocall.go:350 +0x132 > runtime.cgocallbackg(0x7448b719b6c0, 0x7ffc8f6ca510, 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_62fc19d078a9_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. Retrieve caller name and pointer kind; use them in the error message: > panic: runtime error: result of cgo function foo is unpinned Go string or points to unpinned Go string > > goroutine 17 [running, locked to thread]: > panic({0x743542d35cc0?, 0x3515a4ae0040?}) > /mnt/go/src/runtime/panic.go:877 +0x16f > runtime.cgoCheckArg(0x743542d33560, 0x3515a4b3ae50, 0x3?, 0x0, 0x1) > /mnt/go/src/runtime/cgocall.go:682 +0x325 > runtime.cgoCheckResult({0x743542d33560, 0x3515a4b3ae50}) > /mnt/go/src/runtime/cgocall.go:798 +0x45 > _cgoexp_62fc19d078a9_foo(0x7ffee111c660) > _cgo_gotypes.go:65 +0x5d > runtime.cgocallbackg1(0x743542d030e0, 0x7ffee111c660, 0x0) > /mnt/go/src/runtime/cgocall.go:446 +0x289 > runtime.cgocallbackg(0x743542d030e0, 0x7ffee111c660, 0x0) > /mnt/go/src/runtime/cgocall.go:350 +0x132 > runtime.cgocallbackg(0x743542d030e0, 0x7ffee111c660, 0x0) > <autogenerated>:1 +0x2b > runtime.cgocallback(0x0, 0x0, 0x0) > /mnt/go/src/runtime/asm_amd64.s:1098 +0xcd > runtime.goexit({}) > /mnt/go/src/runtime/asm_amd64.s:1709 +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 print_ptr(void *p) { > printf("Pointer: %p\n", p); > } > */ > import "C" > import "unsafe" > > func main() { > x := 10 > p := &x // p is a Go pointer > C.print_ptr(unsafe.Pointer(&p)) // passing Go pointer to Go pointer > } 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/go/src/test.go:15 > main.main() > /mnt/go/src/test.go:15 +0x79 > 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 pointer > > goroutine 1 [running]: > main.main.func1(...) > /mnt/go/src/test.go:15 > main.main() > /mnt/go/src/test.go:15 +0x88 > exit status 2 Link: https://pkg.go.dev/cmd/cgo#hdr-Passing_pointers Suggested-by: Ian Lance Taylor <iant@golang.org> Fixes #75856
1 parent 864acdd commit fe7c70a

File tree

2 files changed

+80
-14
lines changed

2 files changed

+80
-14
lines changed

src/cmd/cgo/internal/testerrors/ptr_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,7 @@ func testOne(t *testing.T, pt ptrTest, exe, exe2 string) {
694694
if err == nil {
695695
t.Logf("%s", buf)
696696
t.Fatalf("did not fail as expected")
697-
} else if !bytes.Contains(buf, []byte("Go pointer")) {
697+
} else if !bytes.Contains(buf, []byte("unpinned Go")) {
698698
t.Logf("%s", buf)
699699
t.Fatalf("did not print expected error (failed with %v)", err)
700700
}

src/runtime/cgocall.go

Lines changed: 79 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -591,15 +591,18 @@ func cgoCheckPointer(ptr any, arg any) {
591591
cgoCheckArg(t, ep.data, !t.IsDirectIface(), top, cgoCheckPointerFail)
592592
}
593593

594-
const cgoCheckPointerFail = "cgo argument has Go pointer to unpinned Go pointer"
595-
const cgoResultFail = "cgo result is unpinned Go pointer or points to unpinned Go pointer"
594+
type cgoErrorMsg int
595+
const (
596+
cgoCheckPointerFail cgoErrorMsg = iota
597+
cgoResultFail
598+
)
596599

597600
// cgoCheckArg is the real work of cgoCheckPointer and cgoCheckResult.
598601
// The argument p is either a pointer to the value (of type t), or the value
599602
// itself, depending on indir. The top parameter is whether we are at the top
600603
// level, where Go pointers are allowed. Go pointers to pinned objects are
601604
// allowed as long as they don't reference other unpinned pointers.
602-
func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) {
605+
func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg cgoErrorMsg) {
603606
if !t.Pointers() || p == nil {
604607
// If the type has no pointers there is nothing to do.
605608
return
@@ -625,15 +628,15 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) {
625628
// These types contain internal pointers that will
626629
// always be allocated in the Go heap. It's never OK
627630
// to pass them to C.
628-
panic(errorString(msg))
631+
panic(cgoFormatErr(msg, t.Kind()))
629632
case abi.Func:
630633
if indir {
631634
p = *(*unsafe.Pointer)(p)
632635
}
633636
if !cgoIsGoPointer(p) {
634637
return
635638
}
636-
panic(errorString(msg))
639+
panic(cgoFormatErr(msg, t.Kind()))
637640
case abi.Interface:
638641
it := *(**_type)(p)
639642
if it == nil {
@@ -643,14 +646,14 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) {
643646
// constant. A type not known at compile time will be
644647
// in the heap and will not be OK.
645648
if inheap(uintptr(unsafe.Pointer(it))) {
646-
panic(errorString(msg))
649+
panic(cgoFormatErr(msg, t.Kind()))
647650
}
648651
p = *(*unsafe.Pointer)(add(p, goarch.PtrSize))
649652
if !cgoIsGoPointer(p) {
650653
return
651654
}
652655
if !top && !isPinned(p) {
653-
panic(errorString(msg))
656+
panic(cgoFormatErr(msg, t.Kind()))
654657
}
655658
cgoCheckArg(it, p, !it.IsDirectIface(), false, msg)
656659
case abi.Slice:
@@ -661,7 +664,7 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) {
661664
return
662665
}
663666
if !top && !isPinned(p) {
664-
panic(errorString(msg))
667+
panic(cgoFormatErr(msg, t.Kind()))
665668
}
666669
if !st.Elem.Pointers() {
667670
return
@@ -676,7 +679,7 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) {
676679
return
677680
}
678681
if !top && !isPinned(ss.str) {
679-
panic(errorString(msg))
682+
panic(cgoFormatErr(msg, t.Kind()))
680683
}
681684
case abi.Struct:
682685
st := (*structtype)(unsafe.Pointer(t))
@@ -705,7 +708,7 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) {
705708
return
706709
}
707710
if !top && !isPinned(p) {
708-
panic(errorString(msg))
711+
panic(cgoFormatErr(msg, t.Kind()))
709712
}
710713

711714
cgoCheckUnknownPointer(p, msg)
@@ -716,7 +719,7 @@ func cgoCheckArg(t *_type, p unsafe.Pointer, indir, top bool, msg string) {
716719
// memory. It checks whether that Go memory contains any other
717720
// pointer into unpinned Go memory. If it does, we panic.
718721
// The return values are unused but useful to see in panic tracebacks.
719-
func cgoCheckUnknownPointer(p unsafe.Pointer, msg string) (base, i uintptr) {
722+
func cgoCheckUnknownPointer(p unsafe.Pointer, msg cgoErrorMsg) (base, i uintptr) {
720723
if inheap(uintptr(p)) {
721724
b, span, _ := findObject(uintptr(p), 0, 0)
722725
base = b
@@ -731,7 +734,7 @@ func cgoCheckUnknownPointer(p unsafe.Pointer, msg string) (base, i uintptr) {
731734
}
732735
pp := *(*unsafe.Pointer)(unsafe.Pointer(addr))
733736
if cgoIsGoPointer(pp) && !isPinned(pp) {
734-
panic(errorString(msg))
737+
panic(cgoFormatErr(msg, abi.Pointer))
735738
}
736739
}
737740
return
@@ -741,7 +744,7 @@ func cgoCheckUnknownPointer(p unsafe.Pointer, msg string) (base, i uintptr) {
741744
if cgoInRange(p, datap.data, datap.edata) || cgoInRange(p, datap.bss, datap.ebss) {
742745
// We have no way to know the size of the object.
743746
// We have to assume that it might contain a pointer.
744-
panic(errorString(msg))
747+
panic(cgoFormatErr(msg, abi.Pointer))
745748
}
746749
// In the text or noptr sections, we know that the
747750
// pointer does not point to a Go pointer.
@@ -794,3 +797,66 @@ func cgoCheckResult(val any) {
794797
t := ep._type
795798
cgoCheckArg(t, ep.data, !t.IsDirectIface(), false, cgoResultFail)
796799
}
800+
801+
// cgoFormatErr is called to format an error message with the caller name.
802+
func cgoFormatErr(error cgoErrorMsg, kind abi.Kind) errorString {
803+
var msg, kindname string
804+
var callerName string = "unknown"
805+
var offset int
806+
807+
// We expect one of these abi.Kind from cgoCheckArg
808+
switch kind {
809+
case abi.Chan:
810+
kindname = "channel"
811+
case abi.Func:
812+
kindname = "function"
813+
case abi.Interface:
814+
kindname = "interface"
815+
case abi.Map:
816+
kindname = "map"
817+
case abi.Pointer:
818+
kindname = "pointer"
819+
case abi.Slice:
820+
kindname = "slice"
821+
case abi.String:
822+
kindname = "string"
823+
case abi.Struct:
824+
kindname = "struct"
825+
case abi.UnsafePointer:
826+
kindname = "unsafe pointer"
827+
default:
828+
kindname = "pointer"
829+
}
830+
831+
// The cgo name might need an offset to be obtained
832+
if error == cgoResultFail {
833+
offset = 21
834+
}
835+
836+
// Relatively to cgoFormatErr, this is the stack frame:
837+
// 0. cgoFormatErr
838+
// 1. cgoCheckArg or cgoCheckUnknownPointer
839+
// 2. cgoCheckPointer or cgoCheckResult
840+
// 3. cgoCaller or cgoCallee
841+
pc, _, _, ok := Caller(3)
842+
if ok {
843+
function := FuncForPC(pc)
844+
845+
if function != nil {
846+
// Expected format:
847+
// - caller name: _cgoexp_3c910ddb72c4_foo
848+
// - callee name: GoRoutineName.ModuleName.funcX
849+
callerName = function.Name()[offset:]
850+
}
851+
}
852+
853+
if error == cgoResultFail {
854+
msg = "result of cgo function " + callerName
855+
msg += " is unpinned Go " + kindname + " or points to unpinned Go " + kindname
856+
} else if error == cgoCheckPointerFail {
857+
msg = "cgo argument of function " + callerName
858+
msg += " has Go pointer to unpinned Go " + kindname
859+
}
860+
861+
return errorString(msg)
862+
}

0 commit comments

Comments
 (0)