From 3392690e404b6ba5022825d33259bc2e9e89eb53 Mon Sep 17 00:00:00 2001 From: Alexander Potapenko Date: Wed, 10 Jan 2024 16:17:08 +0100 Subject: pkg/cover: ensure that all PCs returned by kcov have matching callbacks In the case some modules' addresses are off, certain kernel addresses returned by kcov may not have corresponding coverage callbacks in the .ko files. Keep an additional map in the backend to verify those addresses and report an error if that is the case. Also adjust text expectations in pkg/cover/report_test.go, as inexact coverage will result in an error now. --- pkg/cover/backend/backend.go | 11 ++++++----- pkg/cover/backend/dwarf.go | 11 ++++++++++- pkg/cover/report.go | 15 +++++++++++++++ pkg/cover/report_test.go | 32 +++++++++++++++++++++++++++----- 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/pkg/cover/backend/backend.go b/pkg/cover/backend/backend.go index 3bd95a6fa..dc3fd7f0c 100644 --- a/pkg/cover/backend/backend.go +++ b/pkg/cover/backend/backend.go @@ -11,11 +11,12 @@ import ( ) type Impl struct { - Units []*CompileUnit - Symbols []*Symbol - Frames []Frame - Symbolize func(pcs map[*Module][]uint64) ([]Frame, error) - RestorePC func(pc uint32) uint64 + Units []*CompileUnit + Symbols []*Symbol + Frames []Frame + Symbolize func(pcs map[*Module][]uint64) ([]Frame, error) + RestorePC func(pc uint32) uint64 + CoverPoints map[uint64]bool } type Module struct { diff --git a/pkg/cover/backend/dwarf.go b/pkg/cover/backend/dwarf.go index 1a8217cb1..9f36cdfa6 100644 --- a/pkg/cover/backend/dwarf.go +++ b/pkg/cover/backend/dwarf.go @@ -187,13 +187,22 @@ 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) + for i := 0; i < 2; i++ { + for _, pc := range allCoverPoints[i] { + if pc != 0 { + allCoverPointsMap[pc] = true + } + } + } impl := &Impl{ Units: allUnits, Symbols: allSymbols, Symbolize: func(pcs map[*Module][]uint64) ([]Frame, error) { return symbolize(target, objDir, srcDir, buildDir, pcs) }, - RestorePC: makeRestorePC(params, pcBase), + RestorePC: makeRestorePC(params, pcBase), + CoverPoints: allCoverPointsMap, } return impl, nil } diff --git a/pkg/cover/report.go b/pkg/cover/report.go index d150fcab7..2fffbab14 100644 --- a/pkg/cover/report.go +++ b/pkg/cover/report.go @@ -88,12 +88,18 @@ func (rg *ReportGenerator) prepareFileMap(progs []Prog) (map[string]*file, error } } progPCs := make(map[uint64]map[int]bool) + allProgPCs := make(map[uint64]bool) + matchedProgPCs := make(map[uint64]bool) for i, prog := range progs { for _, pc := range prog.PCs { if progPCs[pc] == nil { progPCs[pc] = make(map[int]bool) } progPCs[pc][i] = true + allProgPCs[pc] = true + if rg.CoverPoints[pc] { + matchedProgPCs[pc] = true + } } } matchedPC := false @@ -123,6 +129,15 @@ func (rg *ReportGenerator) prepareFileMap(progs []Prog) (map[string]*file, error if !matchedPC { return nil, fmt.Errorf("coverage doesn't match any coverage callbacks") } + if len(allProgPCs) != len(matchedProgPCs) { + for mismatch := range allProgPCs { + if !matchedProgPCs[mismatch] { + return nil, fmt.Errorf("%d out of %d PCs returned by kcov do not have matching "+ + "coverage callbacks. Check the discoverModules() code, %v", + len(allProgPCs)-len(matchedProgPCs), len(allProgPCs), mismatch) + } + } + } for _, unit := range rg.Units { f := files[unit.Name] for _, pc := range unit.PCs { diff --git a/pkg/cover/report_test.go b/pkg/cover/report_test.go index 64baf3ee2..2c07cb107 100644 --- a/pkg/cover/report_test.go +++ b/pkg/cover/report_test.go @@ -11,6 +11,7 @@ package cover import ( "bytes" "encoding/csv" + "fmt" "os" "path/filepath" "reflect" @@ -36,8 +37,10 @@ type Test struct { Progs []Prog DebugInfo bool AddCover bool - Result string - Supports func(target *targets.Target) bool + AddBadPc bool + // Inexact coverage generated by AddCover=true may override empty Result. + Result string + Supports func(target *targets.Target) bool } func TestReportGenerator(t *testing.T) { @@ -73,6 +76,14 @@ func TestReportGenerator(t *testing.T) { CFlags: []string{"-fsanitize-coverage=trace-pc"}, DebugInfo: true, }, + { + Name: "mismatch-pcs", + AddCover: true, + AddBadPc: true, + CFlags: []string{"-fsanitize-coverage=trace-pc"}, + DebugInfo: true, + Result: `.* do not have matching coverage callbacks`, + }, { Name: "good-pie", AddCover: true, @@ -132,7 +143,7 @@ func TestReportGenerator(t *testing.T) { } func testReportGenerator(t *testing.T, target *targets.Target, test Test) { - rep, csv, err := generateReport(t, target, test) + rep, csv, err := generateReport(t, target, &test) if err != nil { if test.Result == "" { t.Fatalf("expected no error, but got:\n%v", err) @@ -177,7 +188,7 @@ void* aslr_base() { return NULL; } void __sanitizer_cov_trace_pc() { printf("%llu", (long long)(__builtin_return_address(0) - aslr_base())); } ` -func buildTestBinary(t *testing.T, target *targets.Target, test Test, dir string) string { +func buildTestBinary(t *testing.T, target *targets.Target, test *Test, dir string) string { kcovSrc := filepath.Join(dir, "kcov.c") kcovObj := filepath.Join(dir, "kcov.o") if err := osutil.WriteFile(kcovSrc, []byte(kcovCode)); err != nil { @@ -257,7 +268,7 @@ func buildTestBinary(t *testing.T, target *targets.Target, test Test, dir string return bin } -func generateReport(t *testing.T, target *targets.Target, test Test) ([]byte, []byte, error) { +func generateReport(t *testing.T, target *targets.Target, test *Test) ([]byte, []byte, error) { dir := t.TempDir() bin := buildTestBinary(t, target, test, dir) cfg := &mgrconfig.Config{ @@ -292,6 +303,7 @@ func generateReport(t *testing.T, target *targets.Target, test Test) ([]byte, [] } if test.AddCover { var pcs []uint64 + Inexact := false // Sanitizers crash when installing signal handlers with static libc. const sanitizerOptions = "handle_segv=0:handle_sigbus=0:handle_sigfpe=0" cmd := osutil.Command(bin) @@ -322,6 +334,16 @@ func generateReport(t *testing.T, target *targets.Target, test Test) ([]byte, [] pcs = append(pcs, main.Addr+uint64(off)) } t.Logf("using inexact coverage range 0x%x-0x%x", main.Addr, main.Addr+uint64(main.Size)) + Inexact = true + } + // All PCs are required to have corresponding coverage callbacks. If the expected result is empty, + // override it with an error message. + if Inexact && test.Result == "" { + test.Result = fmt.Sprintf("%d out of %d PCs returned by kcov do not have matching coverage callbacks", + len(pcs)-1, len(pcs)) + } + if test.AddBadPc { + pcs = append(pcs, 0xdeadbeef) } progs = append(progs, Prog{Data: "main", PCs: pcs}) } -- cgit mrf-deployment