Skip to content

Commit 034cc44

Browse files
committed
pkg/report: get only the thread local reports
Syzkaller is currently able to parse sequential reports from the kernel log and accumulate the statistics per report. You can see these reports in syzkaller only (not syzbot dashboard). The problems: 1. These reports may be interleaving if are coming from the parallel contexts (threads) 2. Getting reports from one thread instead of all threads will hopefully give a higher signal/noise ratio. This PR improves sequential reports parsing and is not expected to affect "first report" extraction.
1 parent bc54aa9 commit 034cc44

File tree

6 files changed

+144
-219
lines changed

6 files changed

+144
-219
lines changed

pkg/report/linux.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,9 @@ func (ctx *linux) Parse(output []byte) *Report {
165165
}
166166
for questionable := false; ; questionable = true {
167167
rep := &Report{
168-
Output: output,
169-
StartPos: startPos,
168+
Output: output,
169+
StartPos: startPos,
170+
ContextID: context,
170171
}
171172
endPos, reportEnd, report, prefix := ctx.findReport(output, oops, startPos, context, questionable)
172173
rep.EndPos = endPos

pkg/report/report.go

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ type Report struct {
7272
MachineInfo []byte
7373
// If the crash happened in the context of the syz-executor process, Executor will hold more info.
7474
Executor *ExecutorInfo
75+
// On Linux systems ContextID may be the ThreadID(enabled by CONFIG_PRINTK_CALLER)
76+
// or alternatively CpuID.
77+
ContextID string
78+
7579
// reportPrefixLen is length of additional prefix lines that we added before actual crash report.
7680
reportPrefixLen int
7781
// symbolized is set if the report is symbolized. It prevents double symbolization.
@@ -278,16 +282,46 @@ func IsSuppressed(reporter *Reporter, output []byte) bool {
278282
}
279283

280284
// ParseAll returns all successive reports in output.
281-
func ParseAll(reporter *Reporter, output []byte) (reports []*Report) {
282-
skipPos := 0
285+
func ParseAll(reporter *Reporter, output []byte, startFrom int) []*Report {
286+
skipPos := startFrom
287+
var res []*Report
288+
var scanFrom []int
283289
for {
284290
rep := reporter.ParseFrom(output, skipPos)
285291
if rep == nil {
286-
return
292+
break
287293
}
288-
reports = append(reports, rep)
294+
res = append(res, rep)
295+
scanFrom = append(scanFrom, skipPos)
289296
skipPos = rep.SkipPos
290297
}
298+
return fixReports(reporter, res, scanFrom)
299+
}
300+
301+
// fixReports truncates the report where possible.
302+
// Some reports last till the end of the output. If we have a few sequential reports, they intersect.
303+
// The idea is to cut the log into the chunks and generate the shorter but still valid(!corrupted) reports.
304+
func fixReports(reporter *Reporter, reports []*Report, skipPos []int) []*Report {
305+
nextContextReportPos := map[string]int{}
306+
for i := len(reports) - 1; i >= 0; i-- {
307+
rep := reports[i]
308+
if rep.Corrupted {
309+
continue
310+
}
311+
nextReportPos := nextContextReportPos[rep.ContextID]
312+
nextContextReportPos[rep.ContextID] = rep.StartPos
313+
if nextReportPos == 0 {
314+
continue
315+
}
316+
if nextReportPos < rep.EndPos {
317+
shorterReport := reporter.ParseFrom(rep.Output[:nextReportPos], skipPos[i])
318+
if !shorterReport.Corrupted {
319+
reports[i] = shorterReport
320+
reports[i].Output = rep.Output
321+
}
322+
}
323+
}
324+
return reports
291325
}
292326

293327
// GCE console connection sometimes fails with this message.
@@ -933,13 +967,15 @@ var groupGoRuntimeErrors = oops{
933967
},
934968
}
935969

936-
const reportSeparator = "\n<<<<<<<<<<<<<<< tail report >>>>>>>>>>>>>>>\n\n"
970+
const reportSeparator = "<<<<<<<<<<<<<<< tail report >>>>>>>>>>>>>>>"
937971

938972
func MergeReportBytes(reps []*Report) []byte {
939973
var res []byte
940-
for _, rep := range reps {
974+
for i, rep := range reps {
975+
if i > 0 {
976+
res = append(res, []byte(reportSeparator)...)
977+
}
941978
res = append(res, rep.Report...)
942-
res = append(res, []byte(reportSeparator)...)
943979
}
944980
return res
945981
}

pkg/report/report_test.go

Lines changed: 69 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,14 @@ type ParseTest struct {
4141
EndLine string
4242
Corrupted bool
4343
Suppressed bool
44-
HasReport bool
45-
Report []byte
44+
45+
// HasReport is in charge of both Report and TailReports.
46+
HasReport bool
47+
Report []byte
48+
TailReports [][]byte
49+
4650
Executor string
51+
ContextIDs []string
4752
// Only used in report parsing:
4853
corruptedReason string
4954
}
@@ -64,6 +69,9 @@ func (test *ParseTest) Equal(other *ParseTest) bool {
6469
if test.HasReport && !bytes.Equal(test.Report, other.Report) {
6570
return false
6671
}
72+
if test.HasReport && !reflect.DeepEqual(test.TailReports, other.TailReports) {
73+
return false
74+
}
6775
return test.Executor == other.Executor
6876
}
6977

@@ -90,6 +98,9 @@ func (test *ParseTest) Headers() []byte {
9098
if test.Executor != "" {
9199
fmt.Fprintf(buf, "EXECUTOR: %s\n", test.Executor)
92100
}
101+
if len(test.ContextIDs) != 0 {
102+
fmt.Fprintf(buf, "CONTEXTS: %v\n", test.ContextIDs)
103+
}
93104
return buf.Bytes()
94105
}
95106

@@ -98,8 +109,8 @@ func testParseFile(t *testing.T, reporter *Reporter, fn string) {
98109
testParseImpl(t, reporter, test)
99110
}
100111

101-
func parseReport(t *testing.T, reporter *Reporter, fn string) *ParseTest {
102-
data, err := os.ReadFile(fn)
112+
func parseReport(t *testing.T, reporter *Reporter, testFileName string) *ParseTest {
113+
data, err := os.ReadFile(testFileName)
103114
if err != nil {
104115
t.Fatal(err)
105116
}
@@ -109,10 +120,11 @@ func parseReport(t *testing.T, reporter *Reporter, fn string) *ParseTest {
109120
phaseHeaders = iota
110121
phaseLog
111122
phaseReport
123+
phaseTailReports
112124
)
113125
phase := phaseHeaders
114126
test := &ParseTest{
115-
FileName: fn,
127+
FileName: testFileName,
116128
}
117129
prevEmptyLine := false
118130
s := bufio.NewScanner(bytes.NewReader(data))
@@ -134,8 +146,20 @@ func parseReport(t *testing.T, reporter *Reporter, fn string) *ParseTest {
134146
test.Log = append(test.Log, '\n')
135147
}
136148
case phaseReport:
137-
test.Report = append(test.Report, s.Bytes()...)
138-
test.Report = append(test.Report, '\n')
149+
if string(s.Bytes()) == "TAIL REPORTS:" {
150+
test.TailReports = [][]byte{{}}
151+
phase = phaseTailReports
152+
} else {
153+
test.Report = append(test.Report, s.Bytes()...)
154+
test.Report = append(test.Report, '\n')
155+
}
156+
case phaseTailReports:
157+
if string(s.Bytes()) == reportSeparator {
158+
test.TailReports = append(test.TailReports, []byte{})
159+
continue
160+
}
161+
test.TailReports[len(test.TailReports)-1] = append(test.TailReports[len(test.TailReports)-1], s.Bytes()...)
162+
test.TailReports[len(test.TailReports)-1] = append(test.TailReports[len(test.TailReports)-1], []byte{'\n'}...)
139163
}
140164
prevEmptyLine = len(s.Bytes()) == 0
141165
}
@@ -160,6 +184,7 @@ func parseHeaderLine(t *testing.T, test *ParseTest, ln string) {
160184
corruptedPrefix = "CORRUPTED: "
161185
suppressedPrefix = "SUPPRESSED: "
162186
executorPrefix = "EXECUTOR: "
187+
contextidPrefix = "CONTEXTS: "
163188
)
164189
switch {
165190
case strings.HasPrefix(ln, "#"):
@@ -195,60 +220,72 @@ func parseHeaderLine(t *testing.T, test *ParseTest, ln string) {
195220
}
196221
case strings.HasPrefix(ln, executorPrefix):
197222
test.Executor = ln[len(executorPrefix):]
223+
case strings.HasPrefix(ln, contextidPrefix):
224+
test.ContextIDs = strings.Split(ln[len(contextidPrefix):], ", ")
198225
default:
199226
t.Fatalf("unknown header field %q", ln)
200227
}
201228
}
202229

203-
func testFromReport(rep *Report) *ParseTest {
204-
if rep == nil {
230+
func testFromReports(reps ...*Report) *ParseTest {
231+
if reps == nil || len(reps) > 0 && reps[0] == nil {
205232
return &ParseTest{}
206233
}
207234
ret := &ParseTest{
208-
Title: rep.Title,
209-
AltTitles: rep.AltTitles,
210-
Corrupted: rep.Corrupted,
211-
corruptedReason: rep.CorruptedReason,
212-
Suppressed: rep.Suppressed,
213-
Type: crash.TitleToType(rep.Title),
214-
Frame: rep.Frame,
215-
Report: rep.Report,
216-
}
217-
if rep.Executor != nil {
218-
ret.Executor = fmt.Sprintf("proc=%d, id=%d", rep.Executor.ProcID, rep.Executor.ExecID)
235+
Title: reps[0].Title,
236+
AltTitles: reps[0].AltTitles,
237+
Corrupted: reps[0].Corrupted,
238+
corruptedReason: reps[0].CorruptedReason,
239+
Suppressed: reps[0].Suppressed,
240+
Type: crash.TitleToType(reps[0].Title),
241+
Frame: reps[0].Frame,
242+
Report: reps[0].Report,
243+
}
244+
if reps[0].Executor != nil {
245+
ret.Executor = fmt.Sprintf("proc=%d, id=%d", reps[0].Executor.ProcID, reps[0].Executor.ExecID)
219246
}
220247
sort.Strings(ret.AltTitles)
248+
ret.ContextIDs = append(ret.ContextIDs, reps[0].ContextID)
249+
for i := 1; i < len(reps); i++ {
250+
ret.TailReports = append(ret.TailReports, reps[i].Report)
251+
ret.ContextIDs = append(ret.ContextIDs, reps[i].ContextID)
252+
}
221253
return ret
222254
}
223255

224256
func testParseImpl(t *testing.T, reporter *Reporter, test *ParseTest) {
225-
rep := reporter.Parse(test.Log)
257+
gotReports := ParseAll(reporter, test.Log, 0)
258+
259+
var firstReport *Report
260+
if len(gotReports) > 0 {
261+
firstReport = gotReports[0]
262+
}
226263
containsCrash := reporter.ContainsCrash(test.Log)
227264
expectCrash := (test.Title != "")
228265
if expectCrash && !containsCrash {
229266
t.Fatalf("did not find crash")
230267
}
231268
if !expectCrash && containsCrash {
232-
t.Fatalf("found unexpected crash")
269+
t.Fatalf("found unexpected crash: %s", firstReport.Title)
233270
}
234-
if rep != nil && rep.Title == "" {
271+
if firstReport != nil && firstReport.Title == "" {
235272
t.Fatalf("found crash, but title is empty")
236273
}
237-
parsed := testFromReport(rep)
274+
parsed := testFromReports(gotReports...)
238275
if !test.Equal(parsed) {
239276
if *flagUpdate && test.StartLine+test.EndLine == "" {
240277
updateReportTest(t, test, parsed)
241278
}
242279
t.Fatalf("want:\n%s\ngot:\n%sCorrupted reason: %q",
243280
test.Headers(), parsed.Headers(), parsed.corruptedReason)
244281
}
245-
if parsed.Title != "" && len(rep.Report) == 0 {
282+
if parsed.Title != "" && len(firstReport.Report) == 0 {
246283
t.Fatalf("found crash message but report is empty")
247284
}
248-
if rep == nil {
285+
if firstReport == nil {
249286
return
250287
}
251-
checkReport(t, reporter, rep, test)
288+
checkReport(t, reporter, firstReport, test)
252289
}
253290

254291
func checkReport(t *testing.T, reporter *Reporter, rep *Report, test *ParseTest) {
@@ -285,11 +322,6 @@ func checkReport(t *testing.T, reporter *Reporter, rep *Report, test *ParseTest)
285322
if rep1 == nil || rep1.Title != rep.Title || rep1.StartPos != rep.StartPos {
286323
t.Fatalf("did not find the same report from rep.StartPos=%v", rep.StartPos)
287324
}
288-
// If we parse from EndPos, we must not find the same report.
289-
rep2 := reporter.ParseFrom(test.Log, rep.EndPos)
290-
if rep2 != nil && rep2.Title == rep.Title {
291-
t.Fatalf("found the same report after rep.EndPos=%v", rep.EndPos)
292-
}
293325
}
294326
}
295327

@@ -303,6 +335,10 @@ func updateReportTest(t *testing.T, test, parsed *ParseTest) {
303335
fmt.Fprintf(buf, "\n%s", test.Log)
304336
if test.HasReport {
305337
fmt.Fprintf(buf, "REPORT:\n%s", parsed.Report)
338+
if len(parsed.TailReports) > 0 {
339+
fmt.Fprintf(buf, "TAIL REPORTS:\n")
340+
buf.Write(bytes.Join(parsed.TailReports, []byte(reportSeparator+"\n")))
341+
}
306342
}
307343
if err := os.WriteFile(test.FileName, buf.Bytes(), 0640); err != nil {
308344
t.Logf("failed to update test file: %v", err)
@@ -395,7 +431,7 @@ func testSymbolizeFile(t *testing.T, reporter *Reporter, fn string) {
395431
if err != nil {
396432
t.Fatalf("failed to symbolize: %v", err)
397433
}
398-
parsed := testFromReport(rep)
434+
parsed := testFromReports(rep)
399435
if !test.Equal(parsed) {
400436
if *flagUpdate {
401437
updateReportTest(t, test, parsed)

tools/syz-symbolize/symbolize.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func main() {
5757
if err != nil {
5858
tool.Failf("failed to open input file: %v", err)
5959
}
60-
reps := report.ParseAll(reporter, text)
60+
reps := report.ParseAll(reporter, text, 0)
6161
if len(reps) == 0 {
6262
rep := &report.Report{Report: text}
6363
if err := reporter.Symbolize(rep); err != nil {

vm/vm.go

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -494,30 +494,24 @@ func (mon *monitor) extractErrors(defaultError string) []*report.Report {
494494
reps[0].Output = append(reps[0].Output, vmDiagnosisStart...)
495495
reps[0].Output = append(reps[0].Output, diagOutput...)
496496
}
497-
return reps
497+
threadID := reps[0].ContextID
498+
return getReportChain(reps, threadID)
498499
}
499500

500-
func (mon *monitor) createReports(defaultError string) []*report.Report {
501-
curPos := mon.curPos
501+
// getReportChain extracts the reports belonging to the specific context/thread ID.
502+
func getReportChain(reps []*report.Report, contextID string) []*report.Report {
502503
var res []*report.Report
503-
for {
504-
rep := mon.reporter.ParseFrom(mon.output, curPos)
505-
if rep == nil {
506-
if defaultError == "" || len(res) > 0 {
507-
return res
508-
}
509-
typ := crash.UnknownType
510-
if defaultError == lostConnectionCrash {
511-
typ = crash.LostConnection
512-
}
513-
return []*report.Report{{
514-
Title: defaultError,
515-
Output: mon.output,
516-
Suppressed: report.IsSuppressed(mon.reporter, mon.output),
517-
Type: typ,
518-
}}
504+
for _, rep := range reps {
505+
if rep.ContextID == contextID {
506+
res = append(res, rep)
519507
}
520-
curPos = rep.SkipPos
508+
}
509+
return res
510+
}
511+
512+
func (mon *monitor) createReports(defaultError string) []*report.Report {
513+
var res []*report.Report
514+
for _, rep := range report.ParseAll(mon.reporter, mon.output, mon.curPos) {
521515
start := max(rep.StartPos-mon.beforeContext, 0)
522516
end := min(rep.EndPos+mon.afterContext, len(rep.Output))
523517
rep.Output = rep.Output[start:end]
@@ -527,6 +521,19 @@ func (mon *monitor) createReports(defaultError string) []*report.Report {
527521
res = append(res, rep)
528522
}
529523
}
524+
if defaultError == "" || len(res) > 0 {
525+
return res
526+
}
527+
typ := crash.UnknownType
528+
if defaultError == lostConnectionCrash {
529+
typ = crash.LostConnection
530+
}
531+
return []*report.Report{{
532+
Title: defaultError,
533+
Output: mon.output,
534+
Suppressed: report.IsSuppressed(mon.reporter, mon.output),
535+
Type: typ,
536+
}}
530537
}
531538

532539
func (mon *monitor) waitForOutput() {

0 commit comments

Comments
 (0)