From 8e1b6f644671a0401ceadcaf0b611ab248b88355 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Sun, 24 Mar 2019 12:52:30 +0100 Subject: [PATCH] dashboard/app: fix infinite emails We override crash with the crash used for bisection to make the information more consistent. However if bisection crash only have syz repro and there is now another crash with C repro, then we always think that we have not reported C repro and continue sending the same report again and again. Don't override the crash with bisection crash in such case. --- dashboard/app/bisect_test.go | 81 ++++++++++++++++++++++++++++++++++++ dashboard/app/reporting.go | 8 +++- 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/dashboard/app/bisect_test.go b/dashboard/app/bisect_test.go index 00f87ed2..57ce55c7 100644 --- a/dashboard/app/bisect_test.go +++ b/dashboard/app/bisect_test.go @@ -7,6 +7,7 @@ package dash import ( "fmt" + "strings" "testing" "time" @@ -547,3 +548,83 @@ func TestBisectCauseExternal(t *testing.T) { c.expectEQ(bisect.Type, dashapi.ReportBisectCause) c.expectEQ(bisect.Title, rep.Title) } + +func TestBisectCauseReproSyz(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build := testBuild(1) + c.client2.UploadBuild(build) + crash := testCrashWithRepro(build, 1) + crash.ReproC = nil + c.client2.ReportCrash(crash) + + pollResp := c.client2.pollJobs(build.Manager) + jobID := pollResp.ID + done := &dashapi.JobDoneReq{ + ID: jobID, + Build: *build, + Log: []byte("bisect log"), + CrashTitle: "bisect crash title", + CrashLog: []byte("bisect crash log"), + } + done.Build.ID = jobID + c.expectOK(c.client2.JobDone(done)) + + crash.ReproC = []byte("int main") + c.client2.ReportCrash(crash) + + msg := c.client2.pollEmailBug() + if !strings.Contains(msg.Body, "syzbot found the following crash") { + t.Fatalf("wrong email header:\n%v", msg.Body) + } + if !strings.Contains(msg.Body, "Bisection is inconclusive") { + t.Fatalf("report does not contain bisection results:\n%v", msg.Body) + } +} + +func TestBisectCauseReproSyz2(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build := testBuild(1) + c.client2.UploadBuild(build) + crash := testCrashWithRepro(build, 1) + crash.ReproC = nil + c.client2.ReportCrash(crash) + + pollResp := c.client2.pollJobs(build.Manager) + jobID := pollResp.ID + done := &dashapi.JobDoneReq{ + ID: jobID, + Build: *build, + Log: []byte("bisect log"), + CrashTitle: "bisect crash title", + CrashLog: []byte("bisect crash log"), + } + done.Build.ID = jobID + c.expectOK(c.client2.JobDone(done)) + + msg := c.client2.pollEmailBug() + if !strings.Contains(msg.Body, "syzbot found the following crash") { + t.Fatalf("wrong email header:\n%v", msg.Body) + } + if !strings.Contains(msg.Body, "Bisection is inconclusive") { + t.Fatalf("report does not contain bisection results:\n%v", msg.Body) + } + + crash.ReproC = []byte("int main") + c.client2.ReportCrash(crash) + + msg = c.client2.pollEmailBug() + if !strings.Contains(msg.Body, "syzbot has found a reproducer for the following crash") { + t.Fatalf("wrong email header:\n%v", msg.Body) + } + // Do we need bisection results in this email as well? + // We already mailed them, so we could not mail them here. + // But if we don't include bisection results, need to check that CC is correct + // (includes bisection CC). + if !strings.Contains(msg.Body, "Bisection is inconclusive") { + t.Fatalf("report still contains bisection results:\n%v", msg.Body) + } +} diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go index b05d651e..5020219d 100644 --- a/dashboard/app/reporting.go +++ b/dashboard/app/reporting.go @@ -303,10 +303,16 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key var job *Job if bug.BisectCause == BisectYes { // If we have bisection results, report the crash/repro used for bisection. - job, crash, _, crashKey, err = loadBisectJob(c, bug) + job1, crash1, _, crashKey1, err := loadBisectJob(c, bug) if err != nil { return nil, err } + job = job1 + if crash1.ReproC != 0 || crash.ReproC == 0 { + // Don't override the crash in this case, + // otherwise we will always think that we haven't reported the C repro. + crash, crashKey = crash1, crashKey1 + } } crashLog, _, err := getText(c, textCrashLog, crash.Log) if err != nil {