Switch to runtime.CallersFrames (#183)

Fixes #160
Fixes #107

Signed-off-by: Dave Cheney <dave@cheney.net>

stack.go

@@ -9,32 +9,26 @@ import (
 )
 
 // Frame represents a program counter inside a stack frame.
-type Frame uintptr
+type Frame runtime.Frame
 
 // pc returns the program counter for this frame;
 // multiple frames may have the same PC value.
-func (f Frame) pc() uintptr { return uintptr(f) - 1 }
+func (f Frame) pc() uintptr { return runtime.Frame(f).PC }
 
 // file returns the full path to the file that contains the
 // function for this Frame's pc.
 func (f Frame) file() string {
-	fn := runtime.FuncForPC(f.pc())
+	file := runtime.Frame(f).File
-	if fn == nil {
+	if file == "" {
 		return "unknown"
 	}
-	file, _ := fn.FileLine(f.pc())
 	return file
 }
 
 // line returns the line number of source code of the
 // function for this Frame's pc.
 func (f Frame) line() int {
-	fn := runtime.FuncForPC(f.pc())
+	return runtime.Frame(f).Line
-	if fn == nil {
-		return 0
-	}
-	_, line := fn.FileLine(f.pc())
-	return line
 }
 
 // Format formats the frame according to the fmt.Formatter interface.
@@ -54,12 +48,11 @@ func (f Frame) Format(s fmt.State, verb rune) {
 	case 's':
 		switch {
 		case s.Flag('+'):
-			pc := f.pc()
+			fn := runtime.Frame(f).Func
-			fn := runtime.FuncForPC(pc)
 			if fn == nil {
 				io.WriteString(s, "unknown")
 			} else {
-				file, _ := fn.FileLine(pc)
+				file := runtime.Frame(f).File
 				fmt.Fprintf(s, "%s\n\t%s", fn.Name(), file)
 			}
 		default:
@@ -114,20 +107,29 @@ func (s *stack) Format(st fmt.State, verb rune) {
 	case 'v':
 		switch {
 		case st.Flag('+'):
-			for _, pc := range *s {
+			frames := runtime.CallersFrames(*s)
-				f := Frame(pc)
+			for {
-				fmt.Fprintf(st, "\n%+v", f)
+				frame, more := frames.Next()
+				fmt.Fprintf(st, "\n%+v", Frame(frame))
+				if !more {
+					break
+				}
 			}
 		}
 	}
 }
 
 func (s *stack) StackTrace() StackTrace {
-	f := make([]Frame, len(*s))
+	var st []Frame
-	for i := 0; i < len(f); i++ {
+	frames := runtime.CallersFrames(*s)
-		f[i] = Frame((*s)[i])
+	for {
+		frame, more := frames.Next()
+		st = append(st, Frame(frame))
+		if !more {
+			break
 		}
-	return f
+	}
+	return st
 }
 
 func callers() *stack {

stack_test.go

@@ -6,51 +6,18 @@ import (
 	"testing"
 )
 
-var initpc, _, _, _ = runtime.Caller(0)
+var initpc = caller()
-
-func TestFrameLine(t *testing.T) {
-	var tests = []struct {
-		Frame
-		want int
-	}{{
-		Frame(initpc),
-		9,
-	}, {
-		func() Frame {
-			var pc, _, _, _ = runtime.Caller(0)
-			return Frame(pc)
-		}(),
-		20,
-	}, {
-		func() Frame {
-			var pc, _, _, _ = runtime.Caller(1)
-			return Frame(pc)
-		}(),
-		28,
-	}, {
-		Frame(0), // invalid PC
-		0,
-	}}
-
-	for _, tt := range tests {
-		got := tt.Frame.line()
-		want := tt.want
-		if want != got {
-			t.Errorf("Frame(%v): want: %v, got: %v", uintptr(tt.Frame), want, got)
-		}
-	}
-}
 
 type X struct{}
 
+//go:noinline
 func (x X) val() Frame {
-	var pc, _, _, _ = runtime.Caller(0)
+	return caller()
-	return Frame(pc)
 }
 
+//go:noinline
 func (x *X) ptr() Frame {
-	var pc, _, _, _ = runtime.Caller(0)
+	return caller()
-	return Frame(pc)
 }
 
 func TestFrameFormat(t *testing.T) {
@@ -59,32 +26,32 @@ func TestFrameFormat(t *testing.T) {
 		format string
 		want   string
 	}{{
-		Frame(initpc),
+		initpc,
 		"%s",
 		"stack_test.go",
 	}, {
-		Frame(initpc),
+		initpc,
 		"%+s",
 		"github.com/pkg/errors.init\n" +
 			"\t.+/github.com/pkg/errors/stack_test.go",
 	}, {
-		Frame(0),
+		Frame{},
 		"%s",
 		"unknown",
 	}, {
-		Frame(0),
+		Frame{},
 		"%+s",
 		"unknown",
 	}, {
-		Frame(initpc),
+		initpc,
 		"%d",
 		"9",
 	}, {
-		Frame(0),
+		Frame{},
 		"%d",
 		"0",
 	}, {
-		Frame(initpc),
+		initpc,
 		"%n",
 		"init",
 	}, {
@@ -102,20 +69,20 @@ func TestFrameFormat(t *testing.T) {
 		"%n",
 		"X.val",
 	}, {
-		Frame(0),
+		Frame{},
 		"%n",
 		"",
 	}, {
-		Frame(initpc),
+		initpc,
 		"%v",
 		"stack_test.go:9",
 	}, {
-		Frame(initpc),
+		initpc,
 		"%+v",
 		"github.com/pkg/errors.init\n" +
 			"\t.+/github.com/pkg/errors/stack_test.go:9",
 	}, {
-		Frame(0),
+		Frame{},
 		"%v",
 		"unknown:0",
 	}}
@@ -153,24 +120,24 @@ func TestStackTrace(t *testing.T) {
 	}{{
 		New("ooh"), []string{
 			"github.com/pkg/errors.TestStackTrace\n" +
-				"\t.+/github.com/pkg/errors/stack_test.go:154",
+				"\t.+/github.com/pkg/errors/stack_test.go:121",
 		},
 	}, {
 		Wrap(New("ooh"), "ahh"), []string{
 			"github.com/pkg/errors.TestStackTrace\n" +
-				"\t.+/github.com/pkg/errors/stack_test.go:159", // this is the stack of Wrap, not New
+				"\t.+/github.com/pkg/errors/stack_test.go:126", // this is the stack of Wrap, not New
 		},
 	}, {
 		Cause(Wrap(New("ooh"), "ahh")), []string{
 			"github.com/pkg/errors.TestStackTrace\n" +
-				"\t.+/github.com/pkg/errors/stack_test.go:164", // this is the stack of New
+				"\t.+/github.com/pkg/errors/stack_test.go:131", // this is the stack of New
 		},
 	}, {
-		func() error { return New("ooh") }(), []string{
+		func() error { noinline(); return New("ooh") }(), []string{
 			`github.com/pkg/errors.(func·009|TestStackTrace.func1)` +
-				"\n\t.+/github.com/pkg/errors/stack_test.go:169", // this is the stack of New
+				"\n\t.+/github.com/pkg/errors/stack_test.go:136", // this is the stack of New
 			"github.com/pkg/errors.TestStackTrace\n" +
-				"\t.+/github.com/pkg/errors/stack_test.go:169", // this is the stack of New's caller
+				"\t.+/github.com/pkg/errors/stack_test.go:136", // this is the stack of New's caller
 		},
 	}, {
 		Cause(func() error {
@@ -179,11 +146,11 @@ func TestStackTrace(t *testing.T) {
 			}()
 		}()), []string{
 			`github.com/pkg/errors.(func·010|TestStackTrace.func2.1)` +
-				"\n\t.+/github.com/pkg/errors/stack_test.go:178", // this is the stack of Errorf
+				"\n\t.+/github.com/pkg/errors/stack_test.go:145", // this is the stack of Errorf
 			`github.com/pkg/errors.(func·011|TestStackTrace.func2)` +
-				"\n\t.+/github.com/pkg/errors/stack_test.go:179", // this is the stack of Errorf's caller
+				"\n\t.+/github.com/pkg/errors/stack_test.go:146", // this is the stack of Errorf's caller
 			"github.com/pkg/errors.TestStackTrace\n" +
-				"\t.+/github.com/pkg/errors/stack_test.go:180", // this is the stack of Errorf's caller's caller
+				"\t.+/github.com/pkg/errors/stack_test.go:147", // this is the stack of Errorf's caller's caller
 		},
 	}}
 	for i, tt := range tests {
@@ -253,22 +220,35 @@ func TestStackTraceFormat(t *testing.T) {
 	}, {
 		stackTrace()[:2],
 		"%v",
-		`\[stack_test.go:207 stack_test.go:254\]`,
+		`\[stack_test.go:174 stack_test.go:221\]`,
 	}, {
 		stackTrace()[:2],
 		"%+v",
 		"\n" +
 			"github.com/pkg/errors.stackTrace\n" +
-			"\t.+/github.com/pkg/errors/stack_test.go:207\n" +
+			"\t.+/github.com/pkg/errors/stack_test.go:174\n" +
 			"github.com/pkg/errors.TestStackTraceFormat\n" +
-			"\t.+/github.com/pkg/errors/stack_test.go:258",
+			"\t.+/github.com/pkg/errors/stack_test.go:225",
 	}, {
 		stackTrace()[:2],
 		"%#v",
-		`\[\]errors.Frame{stack_test.go:207, stack_test.go:266}`,
+		`\[\]errors.Frame{stack_test.go:174, stack_test.go:233}`,
 	}}
 
 	for i, tt := range tests {
 		testFormatRegexp(t, i, tt.StackTrace, tt.format, tt.want)
 	}
 }
+
+// a version of runtime.Caller that returns a Frame, not a uintptr.
+func caller() Frame {
+	var pcs [3]uintptr
+	n := runtime.Callers(2, pcs[:])
+	frames := runtime.CallersFrames(pcs[:n])
+	frame, _ := frames.Next()
+	return Frame(frame)
+}
+
+//go:noinline
+// noinline prevents the caller being inlined
+func noinline() {}