testing: use function names to identify helpers

Previously, helpers were identified by entry PC, but this breaks if the
helper is inlined (as in notHelperCallingHelper). Instead, identify
helpers by function name (with package path). Now TestTBHelper and
TestTBHelperParallel pass with -l=4.

To keep the code unified, this change makes it so that the runner
is also identified by function name instead of entry PC.

Change-Id: I1b1987fc49d114e69d075fab56aeeacd5294982b
Reviewed-on: https://go-review.googlesource.com/41257
Run-TryBot: David Lazar <lazard@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>

1 files changed, 26 insertions, 25 deletions

diff --git a/src/testing/testing.go b/src/testing/testing.go
index f95f4ec4a6..99e9af43b6 100644
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -273,20 +273,20 @@ var (
 // common holds the elements common between T and B and
 // captures common methods such as Errorf.
 type common struct {
-	mu      sync.RWMutex         // guards this group of fields
-	output  []byte               // Output generated by test or benchmark.
-	w       io.Writer            // For flushToParent.
-	ran     bool                 // Test or benchmark (or one of its subtests) was executed.
-	failed  bool                 // Test or benchmark has failed.
-	skipped bool                 // Test of benchmark has been skipped.
-	done    bool                 // Test is finished and all subtests have completed.
-	helpers map[uintptr]struct{} // functions to be skipped when writing file/line info
+	mu      sync.RWMutex        // guards this group of fields
+	output  []byte              // Output generated by test or benchmark.
+	w       io.Writer           // For flushToParent.
+	ran     bool                // Test or benchmark (or one of its subtests) was executed.
+	failed  bool                // Test or benchmark has failed.
+	skipped bool                // Test of benchmark has been skipped.
+	done    bool                // Test is finished and all subtests have completed.
+	helpers map[string]struct{} // functions to be skipped when writing file/line info
 
-	chatty     bool    // A copy of the chatty flag.
-	finished   bool    // Test function has completed.
-	hasSub     int32   // written atomically
-	raceErrors int     // number of races detected during test
-	runner     uintptr // entry pc of tRunner running the test
+	chatty     bool   // A copy of the chatty flag.
+	finished   bool   // Test function has completed.
+	hasSub     int32  // written atomically
+	raceErrors int    // number of races detected during test
+	runner     string // function name of tRunner running the test
 
 	parent   *common
 	level    int       // Nesting depth of test or benchmark.
@@ -336,14 +336,14 @@ func (c *common) frameSkip(skip int) int {
 	more := true
 	for i := 0; more; i++ {
 		frame, more = frames.Next()
-		if frame.Entry == c.runner {
+		if frame.Function == c.runner {
 			// We've gone up all the way to the tRunner calling
 			// the test function (so the user must have
 			// called tb.Helper from inside that test function).
 			// Only skip up to the test function itself.
 			return skip + i - 1
 		}
-		if _, ok := c.helpers[frame.Entry]; !ok {
+		if _, ok := c.helpers[frame.Function]; !ok {
 			// Found a frame that wasn't inside a helper function.
 			return skip + i
 		}
@@ -634,22 +634,23 @@ func (c *common) Helper() {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	if c.helpers == nil {
-		c.helpers = make(map[uintptr]struct{})
+		c.helpers = make(map[string]struct{})
 	}
-	c.helpers[callerEntry(1)] = struct{}{}
+	c.helpers[callerName(1)] = struct{}{}
 }
 
-// callerEntry gives the entry pc for the caller after skip frames
-// (where 0 means the current function).
-func callerEntry(skip int) uintptr {
-	var pc [1]uintptr
-	n := runtime.Callers(skip+2, pc[:]) // skip + runtime.Callers + callerEntry
+// callerName gives the function name (qualified with a package path)
+// for the caller after skip frames (where 0 means the current function).
+func callerName(skip int) string {
+	// Make room for the skip PC.
+	var pc [2]uintptr
+	n := runtime.Callers(skip+2, pc[:]) // skip + runtime.Callers + callerName
 	if n == 0 {
 		panic("testing: zero callers found")
 	}
-	frames := runtime.CallersFrames(pc[:])
+	frames := runtime.CallersFrames(pc[:n])
 	frame, _ := frames.Next()
-	return frame.Entry
+	return frame.Function
 }
 
 // Parallel signals that this test is to be run in parallel with (and only with)
@@ -686,7 +687,7 @@ type InternalTest struct {
 }
 
 func tRunner(t *T, fn func(t *T)) {
-	t.runner = callerEntry(0)
+	t.runner = callerName(0)
 
 	// When this goroutine is done, either because fn(t)
 	// returned normally or because a test failure triggered