diff --git a/pkg/sql/colexecerror/error.go b/pkg/sql/colexecerror/error.go index 9d137ea0e929..4edc08b6c0ff 100644 --- a/pkg/sql/colexecerror/error.go +++ b/pkg/sql/colexecerror/error.go @@ -16,7 +16,11 @@ import ( "github.com/gogo/protobuf/proto" ) -const panicLineSubstring = "runtime/panic.go" +const ( + panicLineSubstring = "runtime/panic.go" + runtimePanicFileSubstring = "runtime" + runtimePanicFunctionSubstring = "runtime." +) // CatchVectorizedRuntimeError executes operation, catches a runtime error if // it is coming from the vectorized engine, and returns it. If an error not @@ -78,9 +82,13 @@ func CatchVectorizedRuntimeError(operation func()) (retErr error) { // panic frame. var panicLineFound bool var panicEmittedFrom string - // We should be able to find it within 3 program counters, starting with the - // caller of this deferred function (2 above the runtime.Callers frame). - pc := make([]uintptr, 3) + // Usually, we'll find the offending frame within 3 program counters, + // starting with the caller of this deferred function (2 above the + // runtime.Callers frame). However, we also want to catch panics + // originating in the Go runtime with the runtime frames being returned + // by CallersFrames, so we allow for 5 more program counters for that + // case (e.g. invalid interface conversions use 2 counters). + pc := make([]uintptr, 8) n := runtime.Callers(2, pc) if n >= 1 { frames := runtime.CallersFrames(pc[:n]) @@ -90,6 +98,13 @@ func CatchVectorizedRuntimeError(operation func()) (retErr error) { if strings.Contains(frame.File, panicLineSubstring) { panicLineFound = true } else if panicLineFound { + if strings.HasPrefix(frame.Function, runtimePanicFunctionSubstring) && + strings.Contains(frame.File, runtimePanicFileSubstring) { + // This frame is from the Go runtime, so we simply + // ignore it to get to the offending one within the + // CRDB. + continue + } panicEmittedFrom = frame.Function break } diff --git a/pkg/sql/colexecerror/error_test.go b/pkg/sql/colexecerror/error_test.go index 6654f794c9d4..152bd1ae3693 100644 --- a/pkg/sql/colexecerror/error_test.go +++ b/pkg/sql/colexecerror/error_test.go @@ -79,6 +79,40 @@ func TestNonCatchablePanicIsNotCaught(t *testing.T) { }) } +type testInterface interface { + foo() +} + +type testImpl1 struct{} + +var _ testInterface = &testImpl1{} + +func (t testImpl1) foo() {} + +type testImpl2 struct{} + +var _ testInterface = &testImpl2{} + +func (t testImpl2) foo() {} + +// TestRuntimePanicIsCaught verifies that if a runtime panic occurs in the +// safe-to-catch package (which this test package is), then it is converted into +// an internal error (#133167). +func TestRuntimePanicIsCaught(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + // Use the release-build panic-catching behavior instead of the + // crdb_test-build behavior. + defer colexecerror.ProductionBehaviorForTests()() + + require.Error(t, colexecerror.CatchVectorizedRuntimeError(func() { + // Attempt an invalid interface conversion. + var o testInterface = &testImpl1{} + _ = o.(*testImpl2) + })) +} + // BenchmarkCatchVectorizedRuntimeError measures the time for // CatchVectorizedRuntimeError to catch and process an error. func BenchmarkCatchVectorizedRuntimeError(b *testing.B) {