]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/covdata: relax mode clash policy for selected operations
authorThan McIntosh <thanm@google.com>
Tue, 27 Dec 2022 19:34:11 +0000 (14:34 -0500)
committerThan McIntosh <thanm@google.com>
Tue, 23 May 2023 11:36:37 +0000 (11:36 +0000)
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
src/cmd/covdata/merge.go
src/cmd/covdata/tool_test.go
src/internal/coverage/cmerge/merge.go
src/internal/coverage/cmerge/merge_test.go

index 62267170ce1d1295cd1ed46493c1faea603ae702..4b4671e9712a7ae782919aefcaf24fdbb3a60e91 100644 (file)
@@ -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{})
        }
index 225861dab59ff566b63177ca05b69f92f4b79b99..63e823d376a6522f4a1f6b43d61fb6a355c1a5ca 100644 (file)
@@ -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) {
index 42334eae9499be7b4924d7b72304ee198894e6b8..b6c246581294c6d4a6583cf866912da10c681a35 100644 (file)
@@ -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)
        }
 }
 
index c482b8bfa88eecb785f2e475b02cecbe08f9ac77..16fa1e8c387927e24d5d5dccafc931b6bc5e4eb7 100644 (file)
@@ -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.cmode = cmode
-       cm.cgran = cgran
        return nil
 }
 
index e45589f6b8bfb4f8b221984e12899d9ac32cef66..0e6112a62cd54000b7f587a8141e74ac2c9908e6 100644 (file)
@@ -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)
        }
 }