Skip to content

Commit aca5ab4

Browse files
committed
{runtime,cmd}/cgo: improve error messages after panic
This commit improves the error messages returned by cgoCheckResult and cgoCheckPointer after pointer panics. Therefore: - change the signature of cgoCheckArg (it does the real work behind them) - change the signature of cgoCheckUnknownPointer - introduce cgoFormatErr (it formats an error message with the caller name) - update pointer tests. 1. cgoCheckResult When an unpinned Go pointer is returned to a C function, this error shows up: > panic: runtime error: cgo result is unpinned Go pointer or points to unpinned Go pointer > > goroutine 17 [running, locked to thread]: > panic({0x78fa15d1e5c0?, 0xc00001e050?}) > /usr/lib/go/src/runtime/panic.go:802 +0x168 > runtime.cgoCheckArg(0x78fa15d1bf20, 0xc000066e50, 0x0?, 0x0, {0x78fa15cfaa62, 0x42}) > /usr/lib/go/src/runtime/cgocall.go:679 +0x35b > runtime.cgoCheckResult({0x78fa15d1bf20, 0xc000066e50}) > /usr/lib/go/src/runtime/cgocall.go:795 +0x4b > _cgoexp_1ff3739d54a6_foo(0x7fff7f7b0220) > _cgo_gotypes.go:65 +0x5d > runtime.cgocallbackg1(0x78fa15cf16c0, 0x7fff7f7b0220, 0x0) > /usr/lib/go/src/runtime/cgocall.go:446 +0x289 > runtime.cgocallbackg(0x78fa15cf16c0, 0x7fff7f7b0220, 0x0) > /usr/lib/go/src/runtime/cgocall.go:350 +0x132 > runtime.cgocallbackg(0x78fa15cf16c0, 0x7fff7f7b0220, 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_1ff3739d54a6_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({0x7feae6c50640?, 0x26fcc9896000?}) > /mnt/go/src/runtime/panic.go:877 +0x16f > runtime.cgoCheckArg(0x7feae6c4dee0, 0x26fcc9774e50, 0x0?, 0x0, {0x26fcc9894000, 0x52}) > /mnt/go/src/runtime/cgocall.go:678 +0x35b > runtime.cgoCheckResult({0x7feae6c4dee0, 0x26fcc9774e50}) > /mnt/go/src/runtime/cgocall.go:797 +0x85 > _cgoexp_9ed4cd31b2e5_foo(0x7ffcc89baf10) > _cgo_gotypes.go:65 +0x5d > runtime.cgocallbackg1(0x7feae6c1d340, 0x7ffcc89baf10, 0x0) > /mnt/go/src/runtime/cgocall.go:446 +0x289 > runtime.cgocallbackg(0x7feae6c1d340, 0x7ffcc89baf10, 0x0) > /mnt/go/src/runtime/cgocall.go:350 +0x132 > runtime.cgocallbackg(0x7feae6c1d340, 0x7ffcc89baf10, 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 is passed to a C function, this error shows up: > 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 Cc: Keith Randall <khr@golang.org> Cc: Sean Liao <sean@liao.dev> Suggested-by: Ian Lance Taylor <iant@golang.org> GH: #75856
1 parent 54fd72f commit aca5ab4

File tree

2 files changed

+82
-14
lines changed

2 files changed

+82
-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: 81 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,68 @@ 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+
pcs := make([]uintptr, 10)
804+
var msg, kindname string
805+
var callerName string = " "
806+
807+
// Built from a truncated copy of the abi slice
808+
// https://go.dev/src/internal/abi/type.go
809+
var kindNames = map[abi.Kind]string{
810+
abi.Chan: "channel",
811+
abi.Func: "function",
812+
abi.Interface: "interface",
813+
abi.Map: "map",
814+
abi.Pointer: "pointer",
815+
abi.Slice: "slice",
816+
abi.String: "string",
817+
abi.Struct: "struct",
818+
abi.UnsafePointer: "unsafe pointer",
819+
}
820+
821+
// kindnames match the switch case statement in cgoCheckArg
822+
kindname, ok := kindNames[kind]
823+
if !ok {
824+
throw("can't happen")
825+
}
826+
827+
n := Callers(0, pcs)
828+
pcs = pcs[:n]
829+
830+
frames := CallersFrames(pcs)
831+
832+
for {
833+
frame, more := frames.Next()
834+
835+
// The caller name should be within the top ten frames.
836+
if !more {
837+
throw("can't happen")
838+
break
839+
}
840+
841+
// Expected format of cgo caller name: _cgoexp_3c910ddb72c4_foo
842+
if error == cgoResultFail && frame.Function[0:8] == "_cgoexp_" {
843+
callerName += frame.Function[21:] + " "
844+
break
845+
}
846+
847+
// Expected format of cgo callee name: GoRoutineName.ModuleName.funcX
848+
// It's the first function on the stack out of the runtime package
849+
if error == cgoCheckPointerFail && frame.Function[0:8] != "runtime." {
850+
callerName += frame.Function + " "
851+
break
852+
}
853+
}
854+
855+
if error == cgoResultFail {
856+
msg = "result of cgo function" + callerName
857+
msg += "is unpinned Go " + kindname + " or points to unpinned Go " + kindname
858+
} else if error == cgoCheckPointerFail {
859+
msg = "cgo argument of function" + callerName
860+
msg += "has Go pointer to unpinned Go " + kindname
861+
}
862+
863+
return errorString(msg)
864+
}

0 commit comments

Comments
 (0)