cmd/covdata: relax mode clash policy for selected operations

Relax the policy on counter mode clashes in certain cases for "go tool
covdata" operations. Specifically, when generating 'percent',
'pkglist' or 'func' reports, we only care about whether a given
statement is executed, thus counter mode clashes are irrelevant; there
is no need to report clashes for these ops.

Example:

  $ go build -covermode=count -o myprog.count.exe myprog
  $ go build -covermode=set -o myprog.set.exe myprog
  $ GOCOVERDIR=dir1 ./myprog.count.exe
  ...
  $ GOCOVERDIR=dir2 ./myprog.set.exe
  ...
  $ go tool covdata percent i=dir1,dir2
  error: counter mode clash while reading meta-data file dir2/covmeta.1a0cd0c8ccab07d3179f0ac3dd98159a: previous file had count, new file has set
  $

With this patch the command above will "do the right thing" and work
properly, and in addition merges using the "-pcombine" flag will also
operate with relaxed rules. Note that textfmt operations still require
inputs with consistent coverage modes.

Change-Id: I01e97530d9780943c99b399d03d4cfff05aafd8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/495440
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>

src/cmd/covdata/dump.go

@@ -38,6 +38,14 @@ func makeDumpOp(cmd string) covOperation {
 		cmd: cmd,
 		cm:  &cmerge.Merger{},
 	}
+	// For these modes (percent, pkglist, func, etc), use a relaxed
+	// policy when it comes to counter mode clashes. For a percent
+	// report, for example, we only care whether a given line is
+	// executed at least once, so it's ok to (effectively) merge
+	// together runs derived from different counter modes.
+	if d.cmd == percentMode || d.cmd == funcMode || d.cmd == pkglistMode {
+		d.cm.SetModeMergePolicy(cmerge.ModeMergeRelaxed)
+	}
 	if d.cmd == pkglistMode {
 		d.pkgpaths = make(map[string]struct{})
 	}

src/cmd/covdata/merge.go

@@ -11,6 +11,7 @@ import (
 	"flag"
 	"fmt"
 	"internal/coverage"
+	"internal/coverage/cmerge"
 	"internal/coverage/decodecounter"
 	"internal/coverage/decodemeta"
 	"internal/coverage/pods"
@@ -60,6 +61,7 @@ func (m *mstate) Setup() {
 	if *outdirflag == "" {
 		m.Usage("select output directory with '-o' option")
 	}
+	m.mm.SetModeMergePolicy(cmerge.ModeMergeRelaxed)
 }
 
 func (m *mstate) BeginPod(p pods.Pod) {

src/cmd/covdata/tool_test.go

@@ -808,7 +808,7 @@ func testCounterClash(t *testing.T, s state) {
 
 	// Try to merge covdata0 (from prog1.go -countermode=set) with
 	// covdata1 (from prog1.go -countermode=atomic"). This should
-	// produce a counter mode clash error.
+	// work properly, but result in multiple meta-data files.
 	ins := fmt.Sprintf("-i=%s,%s", s.outdirs[0], s.outdirs[3])
 	out := fmt.Sprintf("-o=%s", ccoutdir)
 	args := append([]string{}, "merge", ins, out, "-pcombine")
@@ -818,13 +818,27 @@ func testCounterClash(t *testing.T, s state) {
 	cmd := testenv.Command(t, s.tool, args...)
 	b, err := cmd.CombinedOutput()
 	t.Logf("%% output: %s\n", string(b))
+	if err != nil {
+		t.Fatalf("clash merge failed: %v", err)
+	}
+
+	// Ask for a textual report from the two dirs. Here we have
+	// to report the mode clash.
+	out = "-o=" + filepath.Join(ccoutdir, "file.txt")
+	args = append([]string{}, "textfmt", ins, out)
+	if debugtrace {
+		t.Logf("clash textfmt command is %s %v\n", s.tool, args)
+	}
+	cmd = testenv.Command(t, s.tool, args...)
+	b, err = cmd.CombinedOutput()
+	t.Logf("%% output: %s\n", string(b))
 	if err == nil {
-		t.Fatalf("clash merge passed unexpectedly")
+		t.Fatalf("expected mode clash")
 	}
 	got := string(b)
 	want := "counter mode clash while reading meta-data"
 	if !strings.Contains(got, want) {
-		t.Errorf("counter clash merge: wanted %s got %s", want, got)
+		t.Errorf("counter clash textfmt: wanted %s got %s", want, got)
 	}
 }
 

src/internal/coverage/cmerge/merge.go

@@ -13,6 +13,13 @@ import (
 	"math"
 )
 
+type ModeMergePolicy uint8
+
+const (
+	ModeMergeStrict ModeMergePolicy = iota
+	ModeMergeRelaxed
+)
+
 // Merger provides state and methods to help manage the process of
 // merging together coverage counter data for a given function, for
 // tools that need to implicitly merge counter as they read multiple
@@ -20,9 +27,14 @@ import (
 type Merger struct {
 	cmode    coverage.CounterMode
 	cgran    coverage.CounterGranularity
+	policy   ModeMergePolicy
 	overflow bool
 }
 
+func (cm *Merger) SetModeMergePolicy(policy ModeMergePolicy) {
+	cm.policy = policy
+}
+
 // MergeCounters takes the counter values in 'src' and merges them
 // into 'dst' according to the correct counter mode.
 func (m *Merger) MergeCounters(dst, src []uint32) (error, bool) {
@@ -72,20 +84,31 @@ func SaturatingAdd(dst, src uint32) (uint32, bool) {
 // SetModeAndGranularity records the counter mode and granularity for
 // the current merge. In the specific case of merging across coverage
 // data files from different binaries, where we're combining data from
-// more than one meta-data file, we need to check for mode/granularity
-// clashes.
+// more than one meta-data file, we need to check for and resolve
+// mode/granularity clashes.
 func (cm *Merger) SetModeAndGranularity(mdf string, cmode coverage.CounterMode, cgran coverage.CounterGranularity) error {
-	// Collect counter mode and granularity so as to detect clashes.
-	if cm.cmode != coverage.CtrModeInvalid {
-		if cm.cmode != cmode {
-			return fmt.Errorf("counter mode clash while reading meta-data file %s: previous file had %s, new file has %s", mdf, cm.cmode.String(), cmode.String())
-		}
+	if cm.cmode == coverage.CtrModeInvalid {
+		// Set merger mode based on what we're seeing here.
+		cm.cmode = cmode
+		cm.cgran = cgran
+	} else {
+		// Granularity clashes are always errors.
 		if cm.cgran != cgran {
 			return fmt.Errorf("counter granularity clash while reading meta-data file %s: previous file had %s, new file has %s", mdf, cm.cgran.String(), cgran.String())
 		}
+		// Mode clashes are treated as errors if we're using the
+		// default strict policy.
+		if cm.cmode != cmode {
+			if cm.policy == ModeMergeStrict {
+				return fmt.Errorf("counter mode clash while reading meta-data file %s: previous file had %s, new file has %s", mdf, cm.cmode.String(), cmode.String())
 			}
+			// In the case of a relaxed mode merge policy, upgrade
+			// mode if needed.
+			if cm.cmode < cmode {
 				cm.cmode = cmode
-	cm.cgran = cgran
+			}
+		}
+	}
 	return nil
 }
 

src/internal/coverage/cmerge/merge_test.go

@@ -15,11 +15,11 @@ func TestClash(t *testing.T) {
 	m := &cmerge.Merger{}
 	err := m.SetModeAndGranularity("mdf1.data", coverage.CtrModeSet, coverage.CtrGranularityPerBlock)
 	if err != nil {
-		t.Fatalf("unexpected clash")
+		t.Fatalf("unexpected clash: %v", err)
 	}
 	err = m.SetModeAndGranularity("mdf1.data", coverage.CtrModeSet, coverage.CtrGranularityPerBlock)
 	if err != nil {
-		t.Fatalf("unexpected clash")
+		t.Fatalf("unexpected clash: %v", err)
 	}
 	err = m.SetModeAndGranularity("mdf1.data", coverage.CtrModeCount, coverage.CtrGranularityPerBlock)
 	if err == nil {
@@ -29,10 +29,23 @@ func TestClash(t *testing.T) {
 	if err == nil {
 		t.Fatalf("expected granularity clash, not found")
 	}
+	m.SetModeMergePolicy(cmerge.ModeMergeRelaxed)
+	err = m.SetModeAndGranularity("mdf1.data", coverage.CtrModeCount, coverage.CtrGranularityPerBlock)
+	if err != nil {
+		t.Fatalf("unexpected clash: %v", err)
+	}
+	err = m.SetModeAndGranularity("mdf1.data", coverage.CtrModeSet, coverage.CtrGranularityPerBlock)
+	if err != nil {
+		t.Fatalf("unexpected clash: %v", err)
+	}
+	err = m.SetModeAndGranularity("mdf1.data", coverage.CtrModeAtomic, coverage.CtrGranularityPerBlock)
+	if err != nil {
+		t.Fatalf("unexpected clash: %v", err)
+	}
 	m.ResetModeAndGranularity()
 	err = m.SetModeAndGranularity("mdf1.data", coverage.CtrModeCount, coverage.CtrGranularityPerFunc)
 	if err != nil {
-		t.Fatalf("unexpected clash after reset")
+		t.Fatalf("unexpected clash after reset: %v", err)
 	}
 }