From: Than McIntosh Date: Tue, 27 Dec 2022 19:34:11 +0000 (-0500) Subject: cmd/covdata: relax mode clash policy for selected operations X-Git-Tag: go1.21rc1~363 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=a371fa5e7042ee5d07b18045aefa8e9c4bfd4efa;p=gostls13.git 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 Reviewed-by: David Chase Run-TryBot: Than McIntosh --- diff --git a/src/cmd/covdata/dump.go b/src/cmd/covdata/dump.go index 62267170ce..4b4671e971 100644 --- a/src/cmd/covdata/dump.go +++ b/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{}) } diff --git a/src/cmd/covdata/merge.go b/src/cmd/covdata/merge.go index 225861dab5..63e823d376 100644 --- a/src/cmd/covdata/merge.go +++ b/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) { diff --git a/src/cmd/covdata/tool_test.go b/src/cmd/covdata/tool_test.go index 42334eae94..b6c2465812 100644 --- a/src/cmd/covdata/tool_test.go +++ b/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) } } diff --git a/src/internal/coverage/cmerge/merge.go b/src/internal/coverage/cmerge/merge.go index c482b8bfa8..16fa1e8c38 100644 --- a/src/internal/coverage/cmerge/merge.go +++ b/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.cmode = cmode - cm.cgran = cgran return nil } diff --git a/src/internal/coverage/cmerge/merge_test.go b/src/internal/coverage/cmerge/merge_test.go index e45589f6b8..0e6112a62c 100644 --- a/src/internal/coverage/cmerge/merge_test.go +++ b/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) } }