From da07505f2b87b144347aa19597979d340b134b06 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 9 Apr 2024 09:02:58 +0200 Subject: pkg/cover: don't memorize all coverage points twice Currently we memorize all coverage points twice: as a slice and as a map. The map also contains __sanitizer_cov_trace_cmp PCs, but I think that's wrong, it should contain only __sanitizer_cov_trace_pc callbacks. We were careful to put as least pressure on the GC as possible by keeping all PCs as a dense allCoverPoints slice and subslicing it in all symbol/compilation unit objects. Don't duplicate coverage points in the map and just use the same slice we store for other purposes. --- pkg/cover/backend/backend.go | 14 +++++++------- pkg/cover/backend/dwarf.go | 18 +++++------------- pkg/cover/html.go | 4 ++-- pkg/cover/report.go | 10 +++++++--- 4 files changed, 21 insertions(+), 25 deletions(-) (limited to 'pkg') diff --git a/pkg/cover/backend/backend.go b/pkg/cover/backend/backend.go index a343dfe02..7ac0e8140 100644 --- a/pkg/cover/backend/backend.go +++ b/pkg/cover/backend/backend.go @@ -11,13 +11,13 @@ import ( ) type Impl struct { - Units []*CompileUnit - Symbols []*Symbol - Frames []Frame - Symbolize func(pcs map[*Module][]uint64) ([]Frame, error) - RestorePC func(pc uint32) uint64 - CallbackPoints map[uint64]bool - CoverageCallbackPoints []uint64 + Units []*CompileUnit + Symbols []*Symbol + Frames []Frame + Symbolize func(pcs map[*Module][]uint64) ([]Frame, error) + RestorePC func(pc uint32) uint64 + CallbackPoints []uint64 + PreciseCoverage bool } type Module struct { diff --git a/pkg/cover/backend/dwarf.go b/pkg/cover/backend/dwarf.go index b90bf0cc4..b13ba2ca8 100644 --- a/pkg/cover/backend/dwarf.go +++ b/pkg/cover/backend/dwarf.go @@ -154,7 +154,7 @@ func makeDWARFUnsafe(params *dwarfParams) (*Impl, error) { var allRanges []pcRange var allUnits []*CompileUnit var pcBase uint64 - var verifyCoverPoints = true + preciseCoverage := true for _, module := range modules { errc := make(chan error, 1) go func() { @@ -190,7 +190,7 @@ func makeDWARFUnsafe(params *dwarfParams) (*Impl, error) { allRanges = append(allRanges, ranges...) allUnits = append(allUnits, units...) if IsKcovBrokenInCompiler(params.getCompilerVersion(module.Path)) { - verifyCoverPoints = false + preciseCoverage = false } } @@ -225,23 +225,15 @@ func makeDWARFUnsafe(params *dwarfParams) (*Impl, error) { // On FreeBSD .text address in ELF is 0, but .text is actually mapped at 0xffffffff. pcBase = ^uint64(0) } - var allCoverPointsMap = make(map[uint64]bool) - if verifyCoverPoints { - for i := 0; i < 2; i++ { - for _, pc := range allCoverPoints[i] { - allCoverPointsMap[pc] = true - } - } - } impl := &Impl{ Units: allUnits, Symbols: allSymbols, Symbolize: func(pcs map[*Module][]uint64) ([]Frame, error) { return symbolize(target, objDir, srcDir, buildDir, splitBuildDelimiters, pcs) }, - RestorePC: makeRestorePC(params, pcBase), - CallbackPoints: allCoverPointsMap, - CoverageCallbackPoints: allCoverPoints[0], + RestorePC: makeRestorePC(params, pcBase), + CallbackPoints: allCoverPoints[0], + PreciseCoverage: preciseCoverage, } return impl, nil } diff --git a/pkg/cover/html.go b/pkg/cover/html.go index 6e266ebce..319a61ddd 100644 --- a/pkg/cover/html.go +++ b/pkg/cover/html.go @@ -225,8 +225,8 @@ type CoverageInfo struct { // DoCoverJSONL is a handler for "/cover?jsonl=1". func (rg *ReportGenerator) DoCoverJSONL(w io.Writer, params CoverHandlerParams) error { - if rg.CoverageCallbackPoints != nil { - if err := rg.symbolizePCs(rg.CoverageCallbackPoints); err != nil { + if rg.CallbackPoints != nil { + if err := rg.symbolizePCs(rg.CallbackPoints); err != nil { return fmt.Errorf("failed to symbolize PCs(): %w", err) } } diff --git a/pkg/cover/report.go b/pkg/cover/report.go index 4e7ec3001..7b291874b 100644 --- a/pkg/cover/report.go +++ b/pkg/cover/report.go @@ -93,14 +93,13 @@ func (rg *ReportGenerator) prepareFileMap(progs []Prog, debug bool) (fileMap, er } progPCs := make(map[uint64]map[int]bool) unmatchedProgPCs := make(map[uint64]bool) - verifyCallbackPoints := (len(rg.CallbackPoints) > 0) for i, prog := range progs { for _, pc := range prog.PCs { if progPCs[pc] == nil { progPCs[pc] = make(map[int]bool) } progPCs[pc][i] = true - if verifyCallbackPoints && !rg.CallbackPoints[pc] { + if rg.PreciseCoverage && !contains(rg.CallbackPoints, pc) { unmatchedProgPCs[pc] = true } } @@ -136,7 +135,7 @@ func (rg *ReportGenerator) prepareFileMap(progs []Prog, debug bool) (fileMap, er } // If the backend provided coverage callback locations for the binaries, use them to // verify data returned by kcov. - if verifyCallbackPoints && (len(unmatchedProgPCs) > 0) { + if len(unmatchedProgPCs) > 0 { return nil, coverageCallbackMismatch(debug, len(progPCs), unmatchedProgPCs) } for _, unit := range rg.Units { @@ -168,6 +167,11 @@ func (rg *ReportGenerator) prepareFileMap(progs []Prog, debug bool) (fileMap, er return files, nil } +func contains(pcs []uint64, pc uint64) bool { + idx := sort.Search(len(pcs), func(i int) bool { return pcs[i] >= pc }) + return idx < len(pcs) && pcs[idx] == pc +} + func coverageCallbackMismatch(debug bool, numPCs int, unmatchedProgPCs map[uint64]bool) error { debugStr := "" if debug { -- cgit mrf-deployment