cmd/cover: handle gotos

If a labeled statement is the target of a goto, we must treat it as the
boundary of a new basic block, but only if it is not already the boundary
of a basic block such as a labeled for loop.

Fixes #16624

Now reports 100% coverage for the test in the issue.

Change-Id: If118bb6ff53a96c738e169d92c03cb3ce97bad0e
Reviewed-on: https://go-review.googlesource.com/30977
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

src/cmd/cover/cover.go

@@ -481,10 +481,35 @@ func (f *File) addCounters(pos, blockEnd token.Pos, list []ast.Stmt, extendToClo
 		var last int
 		end := blockEnd
 		for last = 0; last < len(list); last++ {
-			end = f.statementBoundary(list[last])
-			if f.endsBasicSourceBlock(list[last]) {
-				extendToClosingBrace = false // Block is broken up now.
+			stmt := list[last]
+			end = f.statementBoundary(stmt)
+			if f.endsBasicSourceBlock(stmt) {
+				// If it is a labeled statement, we need to place a counter between
+				// the label and its statement because it may be the target of a goto
+				// and thus start a basic block. That is, given
+				//	foo: stmt
+				// we need to create
+				//	foo: ; stmt
+				// and mark the label as a block-terminating statement.
+				// The result will then be
+				//	foo: COUNTER[n]++; stmt
+				// However, we can't do this if the labeled statement is already
+				// a control statement, such as a labeled for.
+				if label, isLabel := stmt.(*ast.LabeledStmt); isLabel && !f.isControl(label.Stmt) {
+					newLabel := *label
+					newLabel.Stmt = &ast.EmptyStmt{
+						Semicolon: label.Stmt.Pos(),
+						Implicit:  true,
+					}
+					end = label.Pos() // Previous block ends before the label.
+					list[last] = &newLabel
+					// Open a gap and drop in the old statement, now without a label.
+					list = append(list, nil)
+					copy(list[last+1:], list[last:])
+					list[last+1] = label.Stmt
+				}
 				last++
+				extendToClosingBrace = false // Block is broken up now.
 				break
 			}
 		}
@@ -603,7 +628,7 @@ func (f *File) endsBasicSourceBlock(s ast.Stmt) bool {
 	case *ast.IfStmt:
 		return true
 	case *ast.LabeledStmt:
-		return f.endsBasicSourceBlock(s.Stmt)
+		return true // A goto may branch here, starting a new basic block.
 	case *ast.RangeStmt:
 		return true
 	case *ast.SwitchStmt:
@@ -627,6 +652,16 @@ func (f *File) endsBasicSourceBlock(s ast.Stmt) bool {
 	return found
 }
 
+// isControl reports whether s is a control statement that, if labeled, cannot be
+// separated from its label.
+func (f *File) isControl(s ast.Stmt) bool {
+	switch s.(type) {
+	case *ast.ForStmt, *ast.RangeStmt, *ast.SwitchStmt, *ast.SelectStmt, *ast.TypeSwitchStmt:
+		return true
+	}
+	return false
+}
+
 // funcLitFinder implements the ast.Visitor pattern to find the location of any
 // function literal in a subtree.
 type funcLitFinder token.Pos

src/cmd/cover/testdata/test.go

@@ -25,6 +25,7 @@ func testAll() {
 	testPanic()
 	testEmptySwitches()
 	testFunctionLiteral()
+	testGoto()
 }
 
 // The indexes of the counters in testPanic are known to main.go
@@ -247,6 +248,24 @@ func testFunctionLiteral() {
 	}
 }
 
+func testGoto() {
+	for i := 0; i < 2; i++ {
+		if i == 0 {
+			goto Label
+		}
+		check(LINE, 1)
+	Label:
+		check(LINE, 2)
+	}
+	// Now test that we don't inject empty statements
+	// between a label and a loop.
+loop:
+	for {
+		check(LINE, 1)
+		break loop
+	}
+}
+
 // This comment shouldn't appear in generated go code.
 func haha() {
 	// Needed for cover to add counter increment here.