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>

diff --git a/src/cmd/cover/cover.go b/src/cmd/cover/cover.go
index e48d811ead20eb26e81d389e4cc60e863e20622c..222737571fb72c7853d9dfaf11b003b02c4ce5c2 100644 (file)
--- a/src/cmd/cover/cover.go
+++ b/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

diff --git a/src/cmd/cover/testdata/test.go b/src/cmd/cover/testdata/test.go
index 095ce1d909af51a6dedd253f221cda4a9d4aed93..71cb1153313a958b723faa87af8bb52bccf768c2 100644 (file)
--- a/src/cmd/cover/testdata/test.go
+++ b/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.