dashboard/app: remember what crashes we reported for what bugs

We currently can silently switch crashes when report
to the next reporting, or test a patch using a repro
from a different crash.
Remember what crash we reported for a bug and use it
in both cases.

9 files changed, 53 insertions, 16 deletions

diff --git a/dashboard/app/api.go b/dashboard/app/api.go
index 5f112c05c..93335e883 100644
--- a/dashboard/app/api.go
+++ b/dashboard/app/api.go
@@ -537,6 +537,7 @@ func purgeOldCrashes(c context.Context, bug *Bug, bugKey *datastore.Key) {
 		Ancestor(bugKey).
 		Filter("ReproC=", 0).
 		Filter("ReproSyz=", 0).
+		Filter("Reported=", time.Time{}).
 		Order("ReportLen").
 		Order("Time").
 		Limit(maxCrashes+batchSize).
diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go
index adefca286..e64ca0e4d 100644
--- a/dashboard/app/entities.go
+++ b/dashboard/app/entities.go
@@ -86,6 +86,7 @@ type BugReporting struct {
 	ExtID      string // arbitrary reporting ID that is passed back in dashapi.BugReport
 	Link       string
 	CC         string // additional emails added to CC list (|-delimited list)
+	CrashID    int64  // crash that we've last reported in this reporting
 	ReproLevel dashapi.ReproLevel
 	Reported   time.Time
 	Closed     time.Time
@@ -95,12 +96,13 @@ type Crash struct {
 	Manager     string
 	BuildID     string
 	Time        time.Time
-	Maintainers []string `datastore:",noindex"`
-	Log         int64    // reference to CrashLog text entity
-	Report      int64    // reference to CrashReport text entity
-	ReproOpts   []byte   `datastore:",noindex"`
-	ReproSyz    int64    // reference to ReproSyz text entity
-	ReproC      int64    // reference to ReproC text entity
+	Reported    time.Time // set if this crash was ever reported
+	Maintainers []string  `datastore:",noindex"`
+	Log         int64     // reference to CrashLog text entity
+	Report      int64     // reference to CrashReport text entity
+	ReproOpts   []byte    `datastore:",noindex"`
+	ReproSyz    int64     // reference to ReproSyz text entity
+	ReproC      int64     // reference to ReproC text entity
 	ReportLen   int
 }
 
diff --git a/dashboard/app/index.yaml b/dashboard/app/index.yaml
index 552054a9b..1af746189 100644
--- a/dashboard/app/index.yaml
+++ b/dashboard/app/index.yaml
@@ -30,6 +30,7 @@ indexes:
 - kind: Crash
   ancestor: yes
   properties:
+  - name: Reported
   - name: ReproC
   - name: ReproSyz
   - name: ReportLen
@@ -42,6 +43,8 @@ indexes:
     direction: desc
   - name: ReproSyz
     direction: desc
+  - name: Reported
+    direction: desc
   - name: ReportLen
     direction: desc
   - name: Time
diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go
index e26e3a75d..52e7a4b5e 100644
--- a/dashboard/app/jobs.go
+++ b/dashboard/app/jobs.go
@@ -40,7 +40,6 @@ func addTestJob(c context.Context, bugID, user, extID, patch, repo, branch strin
 	}
 	bugReporting, _ := bugReportingByID(bug, bugID)
 
-	// TODO(dvyukov): find the exact crash that we reported.
 	crash, crashKey, err := findCrashForBug(c, bug)
 	if err != nil {
 		return "", err
diff --git a/dashboard/app/main.go b/dashboard/app/main.go
index 44148430c..dfbeb18cb 100644
--- a/dashboard/app/main.go
+++ b/dashboard/app/main.go
@@ -343,7 +343,7 @@ func createUIBug(c context.Context, bug *Bug, state *ReportingState, managers []
 	reportingIdx, status, link := 0, "", ""
 	var err error
 	if bug.Status == BugStatusOpen {
-		_, _, _, reportingIdx, status, link, err = needReport(c, "", state, bug)
+		_, _, _, _, reportingIdx, status, link, err = needReport(c, "", state, bug)
 		if err != nil {
 			status = err.Error()
 		}
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index 675138c3e..048128771 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -64,11 +64,11 @@ func reportingPollBugs(c context.Context, typ string) []*dashapi.BugReport {
 }
 
 func handleReportBug(c context.Context, typ string, state *ReportingState, bug *Bug) (*dashapi.BugReport, error) {
-	reporting, bugReporting, crash, _, _, _, err := needReport(c, typ, state, bug)
+	reporting, bugReporting, crash, crashKey, _, _, _, err := needReport(c, typ, state, bug)
 	if err != nil || reporting == nil {
 		return nil, err
 	}
-	rep, err := createBugReport(c, bug, crash, bugReporting, reporting.Config)
+	rep, err := createBugReport(c, bug, crash, crashKey, bugReporting, reporting.Config)
 	if err != nil {
 		return nil, err
 	}
@@ -76,7 +76,9 @@ func handleReportBug(c context.Context, typ string, state *ReportingState, bug *
 	return rep, nil
 }
 
-func needReport(c context.Context, typ string, state *ReportingState, bug *Bug) (reporting *Reporting, bugReporting *BugReporting, crash *Crash, reportingIdx int, status, link string, err error) {
+func needReport(c context.Context, typ string, state *ReportingState, bug *Bug) (
+	reporting *Reporting, bugReporting *BugReporting, crash *Crash,
+	crashKey *datastore.Key, reportingIdx int, status, link string, err error) {
 	reporting, bugReporting, reportingIdx, status, err = currentReporting(c, bug)
 	if err != nil || reporting == nil {
 		return
@@ -107,7 +109,7 @@ func needReport(c context.Context, typ string, state *ReportingState, bug *Bug)
 		return
 	}
 
-	crash, _, err = findCrashForBug(c, bug)
+	crash, crashKey, err = findCrashForBug(c, bug)
 	if err != nil {
 		status = fmt.Sprintf("%v: no crashes!", reporting.Name)
 		reporting, bugReporting = nil, nil
@@ -178,7 +180,8 @@ func reproStr(level dashapi.ReproLevel) string {
 	}
 }
 
-func createBugReport(c context.Context, bug *Bug, crash *Crash, bugReporting *BugReporting, config interface{}) (*dashapi.BugReport, error) {
+func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *datastore.Key,
+	bugReporting *BugReporting, config interface{}) (*dashapi.BugReport, error) {
 	reportingConfig, err := json.Marshal(config)
 	if err != nil {
 		return nil, err
@@ -240,6 +243,7 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, bugReporting *Bu
 		KernelConfig: kernelConfig,
 		ReproC:       reproC,
 		ReproSyz:     reproSyz,
+		CrashID:      crashKey.IntID(),
 	}
 	if bugReporting.CC != "" {
 		rep.CC = strings.Split(bugReporting.CC, "|")
@@ -251,7 +255,6 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, bugReporting *Bu
 func reportingPollClosed(c context.Context, ids []string) ([]string, error) {
 	var bugs []*Bug
 	_, err := datastore.NewQuery("Bug").
-		//Filter("Status>=", BugStatusFixed).
 		GetAll(c, &bugs)
 	if err != nil {
 		log.Errorf(c, "%v", err)
@@ -266,7 +269,6 @@ func reportingPollClosed(c context.Context, ids []string) ([]string, error) {
 	var closed []string
 	for _, id := range ids {
 		bug := bugMap[id]
-		//log.Errorf(c, "CHECKING: %v: bug=%v", id, bug)
 		if bug == nil {
 			continue
 		}
@@ -276,7 +278,6 @@ func reportingPollClosed(c context.Context, ids []string) ([]string, error) {
 			log.Errorf(c, "%v", err)
 			continue
 		}
-		//log.Errorf(c, "CHECKING: %v: bug=%+v, reporting=%+v", id, bug, bugReporting)
 		if bug.Status >= BugStatusFixed || !bugReporting.Closed.IsZero() {
 			closed = append(closed, id)
 		}
@@ -500,6 +501,21 @@ func incomingCommandTx(c context.Context, now time.Time, cmd *dashapi.BugUpdate,
 			bug.PatchedOn = nil
 		}
 	}
+	if cmd.CrashID != 0 {
+		// Rememeber that we've reported this crash.
+		crash := new(Crash)
+		crashKey := datastore.NewKey(c, "Crash", "", cmd.CrashID, bugKey)
+		if err := datastore.Get(c, crashKey, crash); err != nil {
+			return false, internalError, fmt.Errorf("failed to get reported crash %v: %v",
+				cmd.CrashID, err)
+		}
+		crash.Reported = now
+		if _, err := datastore.Put(c, crashKey, crash); err != nil {
+			return false, internalError, fmt.Errorf("failed to put reported crash %v: %v",
+				cmd.CrashID, err)
+		}
+		bugReporting.CrashID = cmd.CrashID
+	}
 	if bugReporting.ExtID == "" {
 		bugReporting.ExtID = cmd.ExtID
 	}
@@ -582,6 +598,7 @@ func queryCrashesForBug(c context.Context, bugKey *datastore.Key, limit int) (
 		Ancestor(bugKey).
 		Order("-ReproC").
 		Order("-ReproSyz").
+		Order("-Reported").
 		Order("-ReportLen").
 		Order("-Time").
 		Limit(limit).
diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go
index 762a903b6..cd45145f0 100644
--- a/dashboard/app/reporting_email.go
+++ b/dashboard/app/reporting_email.go
@@ -99,6 +99,7 @@ func emailPollBugs(c context.Context) error {
 			ID:         rep.ID,
 			Status:     dashapi.BugStatusOpen,
 			ReproLevel: dashapi.ReproLevelNone,
+			CrashID:    rep.CrashID,
 		}
 		if len(rep.ReproC) != 0 {
 			cmd.ReproLevel = dashapi.ReproLevelC
diff --git a/dashboard/app/reporting_test.go b/dashboard/app/reporting_test.go
index c3a5bd901..bd743f33a 100644
--- a/dashboard/app/reporting_test.go
+++ b/dashboard/app/reporting_test.go
@@ -59,6 +59,7 @@ func TestReportBug(t *testing.T) {
 		KernelConfig: []byte("config1"),
 		Log:          []byte("log1"),
 		Report:       []byte("report1"),
+		CrashID:      rep.CrashID,
 	}
 	c.expectEQ(rep, want)
 
@@ -73,6 +74,10 @@ func TestReportBug(t *testing.T) {
 	want.ReproSyz = []byte("#some opts\ngetpid()")
 	c.expectOK(c.API(client1, key1, "report_crash", crash1, nil))
 	reports = reportAllBugs(c, 1)
+	if want.CrashID == reports[0].CrashID {
+		t.Fatal("get the same CrashID for new crash")
+	}
+	want.CrashID = reports[0].CrashID
 	c.expectEQ(reports[0], want)
 
 	cmd := &dashapi.BugUpdate{
@@ -104,6 +109,12 @@ func TestReportBug(t *testing.T) {
 	c.expectOK(c.API(client1, key1, "reporting_update", cmd, reply))
 	c.expectEQ(reply.OK, false)
 
+	// Report another crash with syz repro for this bug,
+	// ensure that we still report the original crash in the next reporting.
+	// That's what we've upstreammed, it's bad to switch crashes without reason.
+	crash1.Report = []byte("report2")
+	c.expectOK(c.API(client1, key1, "report_crash", crash1, nil))
+
 	// Check that we get the report in the second reporting.
 	c.expectOK(c.API(client1, key1, "reporting_poll_bugs", pr, resp))
 	c.expectEQ(len(resp.Reports), 1)
@@ -216,6 +227,7 @@ func TestInvalidBug(t *testing.T) {
 		Log:          []byte("log2"),
 		Report:       []byte("report2"),
 		ReproC:       []byte("int main() { return 1; }"),
+		CrashID:      rep.CrashID,
 	}
 	c.expectEQ(rep, want)
 
diff --git a/dashboard/dashapi/dashapi.go b/dashboard/dashapi/dashapi.go
index badf40ef4..e833fbfd2 100644
--- a/dashboard/dashapi/dashapi.go
+++ b/dashboard/dashapi/dashapi.go
@@ -225,6 +225,7 @@ type BugReport struct {
 	Report       []byte
 	ReproC       []byte
 	ReproSyz     []byte
+	CrashID      int64 // returned back in BugUpdate
 
 	CrashTitle string // job execution crash title
 	Error      []byte // job execution error
@@ -240,6 +241,7 @@ type BugUpdate struct {
 	DupOf      string
 	FixCommits []string // Titles of commits that fix this bug.
 	CC         []string // Additional emails to add to CC list in future emails.
+	CrashID    int64
 }
 
 type BugUpdateReply struct {