From 4bd78cef058ec8782ed0a8b4f2596f4748dbb575 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 14 Nov 2017 10:04:22 +0100 Subject: pkg/report, pkg/repro, syz-manager: name crash attributes consistently We currently have several names for crash attributes, which is disturbing. E.g. crash title is called "Title" or "Desc". Name them consistently. Title - single line bug identity. Report - whole crash text. Log - whole fuzzer/kernel output. --- pkg/report/freebsd.go | 12 ++++++------ pkg/report/linux.go | 30 +++++++++++++++--------------- pkg/report/linux_test.go | 24 ++++++++++++------------ pkg/report/report.go | 14 +++++++------- pkg/report/report_test.go | 20 ++++++++++---------- pkg/repro/repro.go | 26 +++++++++++++------------- 6 files changed, 63 insertions(+), 63 deletions(-) (limited to 'pkg') diff --git a/pkg/report/freebsd.go b/pkg/report/freebsd.go index 70ffd2249..544ee68c0 100644 --- a/pkg/report/freebsd.go +++ b/pkg/report/freebsd.go @@ -50,10 +50,10 @@ func (ctx *freebsd) Parse(output []byte) *Report { } if oops == nil { oops = oops1 - rep.Start = pos - rep.Desc = string(output[pos+match : next]) + rep.StartPos = pos + rep.Title = string(output[pos+match : next]) } - rep.End = next + rep.EndPos = next } // Console output is indistinguishable from fuzzer output, // so we just collect everything after the oops. @@ -62,15 +62,15 @@ func (ctx *freebsd) Parse(output []byte) *Report { if lineEnd != 0 && output[lineEnd-1] == '\r' { lineEnd-- } - rep.Text = append(rep.Text, output[pos:lineEnd]...) - rep.Text = append(rep.Text, '\n') + rep.Report = append(rep.Report, output[pos:lineEnd]...) + rep.Report = append(rep.Report, '\n') } pos = next + 1 } if oops == nil { return nil } - rep.Desc = extractDescription(output[rep.Start:], oops) + rep.Title = extractDescription(output[rep.StartPos:], oops) return rep } diff --git a/pkg/report/linux.go b/pkg/report/linux.go index 8a137d9b6..3c7596eb5 100644 --- a/pkg/report/linux.go +++ b/pkg/report/linux.go @@ -95,10 +95,10 @@ func (ctx *linux) Parse(output []byte) *Report { } if oops == nil { oops = oops1 - rep.Start = pos - rep.Desc = string(output[pos+match : next]) + rep.StartPos = pos + rep.Title = string(output[pos+match : next]) } - rep.End = next + rep.EndPos = next } if ctx.consoleOutputRe.Match(output[pos:next]) && (!ctx.questionableRe.Match(output[pos:next]) || @@ -117,8 +117,8 @@ func (ctx *linux) Parse(output []byte) *Report { // Prepend 5 lines preceding start of the report, // they can contain additional info related to the report. for _, prefix := range textPrefix { - rep.Text = append(rep.Text, prefix...) - rep.Text = append(rep.Text, '\n') + rep.Report = append(rep.Report, prefix...) + rep.Report = append(rep.Report, '\n') } textPrefix = nil textLines++ @@ -140,8 +140,8 @@ func (ctx *linux) Parse(output []byte) *Report { skipLine = true } if !skipLine { - rep.Text = append(rep.Text, ln...) - rep.Text = append(rep.Text, '\n') + rep.Report = append(rep.Report, ln...) + rep.Report = append(rep.Report, '\n') } } } @@ -150,22 +150,22 @@ func (ctx *linux) Parse(output []byte) *Report { if oops == nil { return nil } - rep.Desc = extractDescription(output[rep.Start:], oops) + rep.Title = extractDescription(output[rep.StartPos:], oops) // Executor PIDs are not interesting. - rep.Desc = executorRe.ReplaceAllLiteralString(rep.Desc, "syz-executor") + rep.Title = executorRe.ReplaceAllLiteralString(rep.Title, "syz-executor") // Replace that everything looks like an address with "ADDR", // addresses in descriptions can't be good regardless of the oops regexps. - rep.Desc = addrRe.ReplaceAllLiteralString(rep.Desc, "ADDR") + rep.Title = addrRe.ReplaceAllLiteralString(rep.Title, "ADDR") // Replace that everything looks like a decimal number with "NUM". - rep.Desc = decNumRe.ReplaceAllLiteralString(rep.Desc, "NUM") + rep.Title = decNumRe.ReplaceAllLiteralString(rep.Title, "NUM") // Replace that everything looks like a file line number with "LINE". - rep.Desc = lineNumRe.ReplaceAllLiteralString(rep.Desc, ":LINE") + rep.Title = lineNumRe.ReplaceAllLiteralString(rep.Title, ":LINE") // Replace all raw references to runctions (e.g. "ip6_fragment+0x1052/0x2d80") // with just function name ("ip6_fragment"). Offsets and sizes are not stable. - rep.Desc = funcRe.ReplaceAllString(rep.Desc, "$1") + rep.Title = funcRe.ReplaceAllString(rep.Title, "$1") // CPU numbers are not interesting. - rep.Desc = cpuRe.ReplaceAllLiteralString(rep.Desc, "CPU") - rep.Corrupted = ctx.isCorrupted(rep.Desc, string(rep.Text)) + rep.Title = cpuRe.ReplaceAllLiteralString(rep.Title, "CPU") + rep.Corrupted = ctx.isCorrupted(rep.Title, string(rep.Report)) return rep } diff --git a/pkg/report/linux_test.go b/pkg/report/linux_test.go index e957ad40e..ffc6d04b7 100644 --- a/pkg/report/linux_test.go +++ b/pkg/report/linux_test.go @@ -1253,29 +1253,29 @@ func TestLinuxIgnores(t *testing.T) { if !reporter.ContainsCrash([]byte(log)) { t.Fatalf("no crash") } - if rep := reporter.Parse([]byte(log)); rep.Desc != "BUG: bug1" { - t.Fatalf("want `BUG: bug1`, found `%v`", rep.Desc) + if rep := reporter.Parse([]byte(log)); rep.Title != "BUG: bug1" { + t.Fatalf("want `BUG: bug1`, found `%v`", rep.Title) } if !reporter1.ContainsCrash([]byte(log)) { t.Fatalf("no crash") } - if rep := reporter1.Parse([]byte(log)); rep.Desc != "BUG: bug1" { - t.Fatalf("want `BUG: bug1`, found `%v`", rep.Desc) + if rep := reporter1.Parse([]byte(log)); rep.Title != "BUG: bug1" { + t.Fatalf("want `BUG: bug1`, found `%v`", rep.Title) } if !reporter2.ContainsCrash([]byte(log)) { t.Fatalf("no crash") } - if rep := reporter2.Parse([]byte(log)); rep.Desc != "BUG: bug2" { - t.Fatalf("want `BUG: bug2`, found `%v`", rep.Desc) + if rep := reporter2.Parse([]byte(log)); rep.Title != "BUG: bug2" { + t.Fatalf("want `BUG: bug2`, found `%v`", rep.Title) } if reporter3.ContainsCrash([]byte(log)) { t.Fatalf("found crash, should be ignored") } if rep := reporter3.Parse([]byte(log)); rep != nil { - t.Fatalf("found `%v`, should be ignored", rep.Desc) + t.Fatalf("found `%v`, should be ignored", rep.Title) } } @@ -1328,11 +1328,11 @@ Read of size 4 by task syz-executor2/5764 t.Fatal(err) } for log, text0 := range tests { - if rep := reporter.Parse([]byte(log)); string(rep.Text) != text0 { + if rep := reporter.Parse([]byte(log)); string(rep.Report) != text0 { t.Logf("log:\n%s", log) t.Logf("want text:\n%s", text0) - t.Logf("got text:\n%s", rep.Text) - t.Fatalf("bad text, desc: '%v'", rep.Desc) + t.Logf("got text:\n%s", rep.Report) + t.Fatalf("bad text, desc: '%v'", rep.Title) } } } @@ -1497,9 +1497,9 @@ func TestLinuxParseReport(t *testing.T) { for i, test := range parseReportTests { t.Run(fmt.Sprint(i), func(t *testing.T) { rep := reporter.Parse([]byte(test.in)) - if test.out != string(rep.Text) { + if test.out != string(rep.Report) { t.Logf("expect:\n%v", test.out) - t.Logf("got:\n%v", string(rep.Text)) + t.Logf("got:\n%v", string(rep.Report)) t.Fail() } }) diff --git a/pkg/report/report.go b/pkg/report/report.go index efc1623bd..403f7bc76 100644 --- a/pkg/report/report.go +++ b/pkg/report/report.go @@ -30,13 +30,13 @@ type Reporter interface { } type Report struct { - // Desc contains a representative description of the first oops. - Desc string - // Text contains whole oops text. - Text []byte - // Start and End denote region of output with oops message(s). - Start int - End int + // Title contains a representative description of the first oops. + Title string + // Report contains whole oops text. + Report []byte + // StartPos/EndPos denote region of output with oops message(s). + StartPos int + EndPos int // Corrupted indicates whether the report is truncated of corrupted in some other way. Corrupted bool } diff --git a/pkg/report/report_test.go b/pkg/report/report_test.go index 6802d0772..98a7725ac 100644 --- a/pkg/report/report_test.go +++ b/pkg/report/report_test.go @@ -69,28 +69,28 @@ func testParse(t *testing.T, os string, tests []ParseTest) { if !expectCrash && containsCrash { t.Fatalf("ContainsCrash found unexpected crash:\n%v", test.Log) } - if rep != nil && rep.Desc == "" { + if rep != nil && rep.Title == "" { t.Fatalf("found crash, but title is empty '%v' in:\n%v", test.Desc, test.Log) } - desc, corrupted := "", false + title, corrupted := "", false if rep != nil { - desc = rep.Desc + title = rep.Title corrupted = rep.Corrupted } - if desc == "" && test.Desc != "" { + if title == "" && test.Desc != "" { t.Fatalf("did not find crash message '%v' in:\n%v", test.Desc, test.Log) } - if desc != "" && test.Desc == "" { - t.Fatalf("found bogus crash message '%v' in:\n%v", desc, test.Log) + if title != "" && test.Desc == "" { + t.Fatalf("found bogus crash message '%v' in:\n%v", title, test.Log) } - if desc != test.Desc { - t.Fatalf("extracted bad crash message:\n%+q\nwant:\n%+q", desc, test.Desc) + if title != test.Desc { + t.Fatalf("extracted bad crash message:\n%+q\nwant:\n%+q", title, test.Desc) } if corrupted && !test.Corrupted { - t.Fatalf("incorrectly marked report as corrupted: '%v'\n%v", desc, test.Log) + t.Fatalf("incorrectly marked report as corrupted: '%v'\n%v", title, test.Log) } if !corrupted && test.Corrupted { - t.Fatalf("failed to mark report as corrupted: '%v'\n%v", desc, test.Log) + t.Fatalf("failed to mark report as corrupted: '%v'\n%v", title, test.Log) } } } diff --git a/pkg/repro/repro.go b/pkg/repro/repro.go index 5575d7768..11f0cccaf 100644 --- a/pkg/repro/repro.go +++ b/pkg/repro/repro.go @@ -35,9 +35,9 @@ type Result struct { Opts csource.Options CRepro bool Stats Stats - // Description, log and report of the final crash that we reproduced. + // Title, Log and Report of the final crash that we reproduced. // Can be different from what we started reproducing. - Desc string + Title string Log []byte Report []byte } @@ -45,11 +45,11 @@ type Result struct { type context struct { cfg *mgrconfig.Config reporter report.Reporter - crashDesc string + crashTitle string instances chan *instance bootRequests chan int stats Stats - desc string + title string log []byte report []byte } @@ -75,16 +75,16 @@ func Run(crashLog []byte, cfg *mgrconfig.Config, reporter report.Reporter, vmPoo return nil, fmt.Errorf("crash log does not contain any programs") } crashStart := len(crashLog) // assuming VM hanged - crashDesc := "hang" + crashTitle := "hang" if rep := reporter.Parse(crashLog); rep != nil { - crashStart = rep.Start - crashDesc = rep.Desc + crashStart = rep.StartPos + crashTitle = rep.Title } ctx := &context{ cfg: cfg, reporter: reporter, - crashDesc: crashDesc, + crashTitle: crashTitle, instances: make(chan *instance, len(vmIndexes)), bootRequests: make(chan int, len(vmIndexes)), } @@ -153,7 +153,7 @@ func Run(crashLog []byte, cfg *mgrconfig.Config, reporter report.Reporter, vmPoo if res != nil { ctx.reproLog(3, "repro crashed as:\n%s", string(ctx.report)) res.Stats = ctx.stats - res.Desc = ctx.desc + res.Title = ctx.title res.Log = ctx.log res.Report = ctx.report } @@ -595,15 +595,15 @@ func (ctx *context) testImpl(inst *vm.Instance, command string, duration time.Du if err != nil { return false, fmt.Errorf("failed to run command in VM: %v", err) } - desc, report, output, crashed, _ := vm.MonitorExecution(outc, errc, ctx.reporter) + title, report, output, crashed, _ := vm.MonitorExecution(outc, errc, ctx.reporter) if !crashed { ctx.reproLog(2, "program did not crash") return false, nil } - ctx.desc = desc + ctx.title = title ctx.log = output ctx.report = report - ctx.reproLog(2, "program crashed: %v", desc) + ctx.reproLog(2, "program crashed: %v", title) return true, nil } @@ -613,7 +613,7 @@ func (ctx *context) returnInstance(inst *instance) { } func (ctx *context) reproLog(level int, format string, args ...interface{}) { - prefix := fmt.Sprintf("reproducing crash '%v': ", ctx.crashDesc) + prefix := fmt.Sprintf("reproducing crash '%v': ", ctx.crashTitle) Logf(level, prefix+format, args...) ctx.stats.Log = append(ctx.stats.Log, []byte(fmt.Sprintf(format, args...)+"\n")...) } -- cgit mrf-deployment