From 032678e0fb0a5e0471a6555b758ca4d960249013 Mon Sep 17 00:00:00 2001
From: Austin Clements <austin@google.com>
Date: Thu, 9 Nov 2017 17:55:45 -0500
Subject: runtime: don't elide wrapper functions that call panic or at TOS

CL 45412 started hiding autogenerated wrapper functions from call
stacks so that call stack semantics better matched language semantics.
This is based on the theory that the wrapper function will call the
"real" function and all the programmer knows about is the real
function.

However, this theory breaks down in two cases:

1. If the wrapper is at the top of the stack, then it didn't call
   anything. This can happen, for example, if the "stack" was actually
   synthesized by the user.

2. If the wrapper panics, for example by calling panicwrap or by
   dereferencing a nil pointer, then it didn't call the wrapped
   function and the user needs to see what panicked, even if we can't
   attribute it nicely.

This commit modifies the traceback logic to include the wrapper
function in both of these cases.

Fixes #22231.

Change-Id: I6e4339a652f73038bd8331884320f0b8edd86eb1
Reviewed-on: https://go-review.googlesource.com/76770
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
---
 src/runtime/stack_test.go | 78 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

(limited to 'src/runtime/stack_test.go')

diff --git a/src/runtime/stack_test.go b/src/runtime/stack_test.go
index 8e7c7d47a8..cb0e08256b 100644
--- a/src/runtime/stack_test.go
+++ b/src/runtime/stack_test.go
@@ -7,6 +7,7 @@ package runtime_test
 import (
 	"bytes"
 	"fmt"
+	"reflect"
 	. "runtime"
 	"strings"
 	"sync"
@@ -650,6 +651,8 @@ func (s structWithMethod) stack() string {
 	return string(buf[:Stack(buf, false)])
 }
 
+func (s structWithMethod) nop() {}
+
 func TestStackWrapperCaller(t *testing.T) {
 	var d structWithMethod
 	// Force the compiler to construct a wrapper method.
@@ -687,6 +690,81 @@ func TestStackWrapperStack(t *testing.T) {
 	}
 }
 
+type I interface {
+	M()
+}
+
+func TestStackWrapperStackPanic(t *testing.T) {
+	t.Run("sigpanic", func(t *testing.T) {
+		// nil calls to interface methods cause a sigpanic.
+		testStackWrapperPanic(t, func() { I.M(nil) }, "runtime_test.I.M")
+	})
+	t.Run("panicwrap", func(t *testing.T) {
+		// Nil calls to value method wrappers call panicwrap.
+		wrapper := (*structWithMethod).nop
+		testStackWrapperPanic(t, func() { wrapper(nil) }, "runtime_test.(*structWithMethod).nop")
+	})
+}
+
+func testStackWrapperPanic(t *testing.T, cb func(), expect string) {
+	// Test that the stack trace from a panicking wrapper includes
+	// the wrapper, even though elide these when they don't panic.
+	t.Run("CallersFrames", func(t *testing.T) {
+		defer func() {
+			err := recover()
+			if err == nil {
+				t.Fatalf("expected panic")
+			}
+			pcs := make([]uintptr, 10)
+			n := Callers(0, pcs)
+			frames := CallersFrames(pcs[:n])
+			for {
+				frame, more := frames.Next()
+				t.Log(frame.Function)
+				if frame.Function == expect {
+					return
+				}
+				if !more {
+					break
+				}
+			}
+			t.Fatalf("panicking wrapper %s missing from stack trace", expect)
+		}()
+		cb()
+	})
+	t.Run("Stack", func(t *testing.T) {
+		defer func() {
+			err := recover()
+			if err == nil {
+				t.Fatalf("expected panic")
+			}
+			buf := make([]byte, 4<<10)
+			stk := string(buf[:Stack(buf, false)])
+			if !strings.Contains(stk, "\n"+expect) {
+				t.Fatalf("panicking wrapper %s missing from stack trace:\n%s", expect, stk)
+			}
+		}()
+		cb()
+	})
+}
+
+func TestCallersFromWrapper(t *testing.T) {
+	// Test that invoking CallersFrames on a stack where the first
+	// PC is an autogenerated wrapper keeps the wrapper in the
+	// trace. Normally we elide these, assuming that the wrapper
+	// calls the thing you actually wanted to see, but in this
+	// case we need to keep it.
+	pc := reflect.ValueOf(I.M).Pointer()
+	frames := CallersFrames([]uintptr{pc})
+	frame, more := frames.Next()
+	if frame.Function != "runtime_test.I.M" {
+		t.Fatalf("want function %s, got %s", "runtime_test.I.M", frame.Function)
+	}
+	if more {
+		t.Fatalf("want 1 frame, got > 1")
+	}
+}
+
 func TestTracebackSystemstack(t *testing.T) {
 	if GOARCH == "ppc64" || GOARCH == "ppc64le" {
 		t.Skip("systemstack tail call not implemented on ppc64x")
-- 
cgit v1.3-5-g9baa