From e66542d78f1cf0c783877440cd239a11fb73fb15 Mon Sep 17 00:00:00 2001 From: Alexander Potapenko Date: Mon, 12 Feb 2024 10:53:25 +0100 Subject: [PATCH] pkg/cover, syz-manager: introduce the /cover?debug=1 parameter Debugging coverage point validation warnings may require looking at specific addresses, which are not printed anywhere. Add a URL parameter that can be passed to prepareFileMap() to print a more meaningful error message. Also factor out the error message code from prepareFileMap() to reduce its cyclomatic complexity. --- pkg/cover/html.go | 38 +++++++++++++++++++++--------------- pkg/cover/report.go | 19 ++++++++++++++---- pkg/cover/report_test.go | 11 +++++++---- syz-manager/html.go | 11 ++++++++++- tools/syz-cover/syz-cover.go | 11 ++++++++--- 5 files changed, 62 insertions(+), 28 deletions(-) diff --git a/pkg/cover/html.go b/pkg/cover/html.go index bbfc11536ed9..8cb9e7511f38 100644 --- a/pkg/cover/html.go +++ b/pkg/cover/html.go @@ -24,9 +24,15 @@ import ( "github.com/google/syzkaller/pkg/mgrconfig" ) -func (rg *ReportGenerator) DoHTML(w io.Writer, progs []Prog, coverFilter map[uint32]uint32) error { - progs = fixUpPCs(rg.target.Arch, progs, coverFilter) - files, err := rg.prepareFileMap(progs) +type CoverHandlerParams struct { + Progs []Prog + CoverFilter map[uint32]uint32 + Debug bool +} + +func (rg *ReportGenerator) DoHTML(w io.Writer, params CoverHandlerParams) error { + var progs = fixUpPCs(rg.target.Arch, params.Progs, params.CoverFilter) + files, err := rg.prepareFileMap(progs, params.Debug) if err != nil { return err } @@ -127,9 +133,9 @@ type lineCoverExport struct { Both []int `json:",omitempty"` } -func (rg *ReportGenerator) DoLineJSON(w io.Writer, progs []Prog, coverFilter map[uint32]uint32) error { - progs = fixUpPCs(rg.target.Arch, progs, coverFilter) - files, err := rg.prepareFileMap(progs) +func (rg *ReportGenerator) DoLineJSON(w io.Writer, params CoverHandlerParams) error { + var progs = fixUpPCs(rg.target.Arch, params.Progs, params.CoverFilter) + files, err := rg.prepareFileMap(progs, params.Debug) if err != nil { return err } @@ -291,7 +297,7 @@ var csvFilesHeader = []string{ } func (rg *ReportGenerator) convertToStats(progs []Prog) ([]fileStats, error) { - files, err := rg.prepareFileMap(progs) + files, err := rg.prepareFileMap(progs, false) if err != nil { return nil, err } @@ -341,8 +347,8 @@ func (rg *ReportGenerator) convertToStats(progs []Prog) ([]fileStats, error) { return data, nil } -func (rg *ReportGenerator) DoCSVFiles(w io.Writer, progs []Prog, coverFilter map[uint32]uint32) error { - progs = fixUpPCs(rg.target.Arch, progs, coverFilter) +func (rg *ReportGenerator) DoCSVFiles(w io.Writer, params CoverHandlerParams) error { + var progs = fixUpPCs(rg.target.Arch, params.Progs, params.CoverFilter) data, err := rg.convertToStats(progs) if err != nil { return err @@ -441,8 +447,8 @@ func groupCoverByFilePrefixes(datas []fileStats, subsystems []mgrconfig.Subsyste return d } -func (rg *ReportGenerator) DoHTMLTable(w io.Writer, progs []Prog, coverFilter map[uint32]uint32) error { - progs = fixUpPCs(rg.target.Arch, progs, coverFilter) +func (rg *ReportGenerator) DoHTMLTable(w io.Writer, params CoverHandlerParams) error { + var progs = fixUpPCs(rg.target.Arch, params.Progs, params.CoverFilter) data, err := rg.convertToStats(progs) if err != nil { return err @@ -517,8 +523,8 @@ func groupCoverByModule(datas []fileStats) map[string]map[string]string { return d } -func (rg *ReportGenerator) DoModuleCover(w io.Writer, progs []Prog, coverFilter map[uint32]uint32) error { - progs = fixUpPCs(rg.target.Arch, progs, coverFilter) +func (rg *ReportGenerator) DoModuleCover(w io.Writer, params CoverHandlerParams) error { + var progs = fixUpPCs(rg.target.Arch, params.Progs, params.CoverFilter) data, err := rg.convertToStats(progs) if err != nil { return err @@ -537,9 +543,9 @@ var csvHeader = []string{ "Total PCs", } -func (rg *ReportGenerator) DoCSV(w io.Writer, progs []Prog, coverFilter map[uint32]uint32) error { - progs = fixUpPCs(rg.target.Arch, progs, coverFilter) - files, err := rg.prepareFileMap(progs) +func (rg *ReportGenerator) DoCSV(w io.Writer, params CoverHandlerParams) error { + var progs = fixUpPCs(rg.target.Arch, params.Progs, params.CoverFilter) + files, err := rg.prepareFileMap(progs, params.Debug) if err != nil { return err } diff --git a/pkg/cover/report.go b/pkg/cover/report.go index 6bf7e0a6ffd9..9d03811add9e 100644 --- a/pkg/cover/report.go +++ b/pkg/cover/report.go @@ -74,7 +74,20 @@ type line struct { progIndex int // example program index that covers this line } -func (rg *ReportGenerator) prepareFileMap(progs []Prog) (map[string]*file, error) { +func coverageCallbackMismatch(debug bool, numPCs int, unmatchedProgPCs map[uint64]bool) error { + debugStr := "" + if debug { + debugStr += "\n\nUnmatched PCs:\n" + for pc := range unmatchedProgPCs { + debugStr += fmt.Sprintf("%x\n", pc) + } + } + // nolint: lll + return fmt.Errorf("%d out of %d PCs returned by kcov do not have matching coverage callbacks. Check the discoverModules() code.%s", + len(unmatchedProgPCs), numPCs, debugStr) +} + +func (rg *ReportGenerator) prepareFileMap(progs []Prog, debug bool) (map[string]*file, error) { if err := rg.lazySymbolize(progs); err != nil { return nil, err } @@ -131,9 +144,7 @@ func (rg *ReportGenerator) prepareFileMap(progs []Prog) (map[string]*file, error // If the backend provided coverage callback locations for the binaries, use them to // verify data returned by kcov. if verifyCoverPoints && (len(unmatchedProgPCs) > 0) { - return nil, fmt.Errorf("%d out of %d PCs returned by kcov do not have matching "+ - "coverage callbacks. Check the discoverModules() code", - len(unmatchedProgPCs), len(progPCs)) + return nil, coverageCallbackMismatch(debug, len(progPCs), unmatchedProgPCs) } for _, unit := range rg.Units { f := files[unit.Name] diff --git a/pkg/cover/report_test.go b/pkg/cover/report_test.go index c6c9d28f19d4..3d31841cc367 100644 --- a/pkg/cover/report_test.go +++ b/pkg/cover/report_test.go @@ -365,20 +365,23 @@ func generateReport(t *testing.T, target *targets.Target, test *Test) ([]byte, [ progs = append(progs, Prog{Data: "main", PCs: pcs}) } html := new(bytes.Buffer) - if err := rg.DoHTML(html, progs, nil); err != nil { + params := CoverHandlerParams{ + Progs: progs, + } + if err := rg.DoHTML(html, params); err != nil { return nil, nil, err } htmlTable := new(bytes.Buffer) - if err := rg.DoHTMLTable(htmlTable, progs, nil); err != nil { + if err := rg.DoHTMLTable(htmlTable, params); err != nil { return nil, nil, err } _ = htmlTable csv := new(bytes.Buffer) - if err := rg.DoCSV(csv, progs, nil); err != nil { + if err := rg.DoCSV(csv, params); err != nil { return nil, nil, err } csvFiles := new(bytes.Buffer) - if err := rg.DoCSVFiles(csvFiles, progs, nil); err != nil { + if err := rg.DoCSVFiles(csvFiles, params); err != nil { return nil, nil, err } _ = csvFiles diff --git a/syz-manager/html.go b/syz-manager/html.go index 52086ca77e24..1c82daa74a3f 100644 --- a/syz-manager/html.go +++ b/syz-manager/html.go @@ -349,6 +349,7 @@ func (mgr *Manager) httpCoverCover(w http.ResponseWriter, r *http.Request, funcF } do := rg.DoHTML + if funcFlag == DoHTMLTable { do = rg.DoHTMLTable } else if funcFlag == DoModuleCover { @@ -359,7 +360,15 @@ func (mgr *Manager) httpCoverCover(w http.ResponseWriter, r *http.Request, funcF do = rg.DoCSVFiles } - if err := do(w, progs, coverFilter); err != nil { + debug := r.FormValue("debug") != "" + + params := cover.CoverHandlerParams{ + Progs: progs, + CoverFilter: coverFilter, + Debug: debug, + } + + if err := do(w, params); err != nil { http.Error(w, fmt.Sprintf("failed to generate coverage profile: %v", err), http.StatusInternalServerError) return } diff --git a/tools/syz-cover/syz-cover.go b/tools/syz-cover/syz-cover.go index 40a77c72ca27..4023f3e5ba3a 100644 --- a/tools/syz-cover/syz-cover.go +++ b/tools/syz-cover/syz-cover.go @@ -77,8 +77,13 @@ func main() { } progs := []cover.Prog{{PCs: pcs}} buf := new(bytes.Buffer) + params := cover.CoverHandlerParams{ + Progs: progs, + CoverFilter: nil, + Debug: false, + } if *flagExportCSV != "" { - if err := rg.DoCSV(buf, progs, nil); err != nil { + if err := rg.DoCSV(buf, params); err != nil { tool.Fail(err) } if err := osutil.WriteFile(*flagExportCSV, buf.Bytes()); err != nil { @@ -87,7 +92,7 @@ func main() { return } if *flagExportLineJSON != "" { - if err := rg.DoLineJSON(buf, progs, nil); err != nil { + if err := rg.DoLineJSON(buf, params); err != nil { tool.Fail(err) } if err := osutil.WriteFile(*flagExportLineJSON, buf.Bytes()); err != nil { @@ -95,7 +100,7 @@ func main() { } return } - if err := rg.DoHTML(buf, progs, nil); err != nil { + if err := rg.DoHTML(buf, params); err != nil { tool.Fail(err) } if *flagExportHTML != "" {