all: remember console output for all patch tests

Currently syzbot only saves a log if there was a build/test error.

Closes #3185

8 files changed, 137 insertions, 105 deletions

diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go
index fb2e4e878..f0ff7750d 100644
--- a/dashboard/app/jobs_test.go
+++ b/dashboard/app/jobs_test.go
@@ -240,13 +240,15 @@ patch:          %[3]v
 	pollResp = c.client2.pollJobs(build.Manager)
 	c.expectEQ(pollResp.Type, dashapi.JobTestPatch)
 	jobDoneReq = &dashapi.JobDoneReq{
-		ID:    pollResp.ID,
-		Build: *build,
+		ID:       pollResp.ID,
+		Build:    *build,
+		CrashLog: []byte("console output"),
 	}
 	c.client2.JobDone(jobDoneReq)
 	{
 		dbJob, dbBuild, _ := c.loadJob(pollResp.ID)
 		patchLink := externalLink(c.ctx, textPatch, dbJob.Patch)
+		logLink := externalLink(c.ctx, textCrashLog, dbJob.CrashLog)
 		kernelConfigLink := externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig)
 		msg := c.pollEmailBug()
 		c.expectEQ(len(msg.Attachments), 0)
@@ -260,13 +262,14 @@ Tested on:
 
 commit:         11111111 kernel_commit_title1
 git tree:       repo1 branch1
+console output: %[4]v
 kernel config:  %[3]v
 dashboard link: https://testapp.appspot.com/bug?extid=%[1]v
 compiler:       compiler1
 patch:          %[2]v
 
 Note: testing is done by a robot and is best-effort only.
-`, extBugID, patchLink, kernelConfigLink))
+`, extBugID, patchLink, kernelConfigLink, logLink))
 		c.checkURLContents(patchLink, []byte(patch))
 		c.checkURLContents(kernelConfigLink, build.KernelConfig)
 	}
diff --git a/pkg/bisect/bisect.go b/pkg/bisect/bisect.go
index b2ec3dd46..9c02a747f 100644
--- a/pkg/bisect/bisect.go
+++ b/pkg/bisect/bisect.go
@@ -488,15 +488,16 @@ func (env *env) test() (*testResult, error) {
 	return res, nil
 }
 
-func (env *env) processResults(current *vcs.Commit, results []error) (bad, good int, rep *report.Report) {
+func (env *env) processResults(current *vcs.Commit, results []instance.EnvTestResult) (
+	bad, good int, rep *report.Report) {
 	var verdicts []string
 	for i, res := range results {
-		if res == nil {
+		if res.Error == nil {
 			good++
 			verdicts = append(verdicts, "OK")
 			continue
 		}
-		switch err := res.(type) {
+		switch err := res.Error.(type) {
 		case *instance.TestError:
 			if err.Boot {
 				verdicts = append(verdicts, fmt.Sprintf("boot failed: %v", err))
diff --git a/pkg/bisect/bisect_test.go b/pkg/bisect/bisect_test.go
index eebe48e37..75135650e 100644
--- a/pkg/bisect/bisect_test.go
+++ b/pkg/bisect/bisect_test.go
@@ -48,25 +48,24 @@ func (env *testEnv) BuildKernel(compilerBin, cCache, userspaceDir, cmdlineFile,
 	return "", details, nil
 }
 
-func (env *testEnv) Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]error, error) {
+func (env *testEnv) Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]instance.EnvTestResult, error) {
 	commit := env.headCommit()
 	if commit >= env.test.brokenStart && commit <= env.test.brokenEnd ||
 		env.config == "baseline-skip" {
 		return nil, fmt.Errorf("broken build")
 	}
+	var ret []instance.EnvTestResult
 	if (env.config == "baseline-repro" || env.config == "new-minimized-config" || env.config == "original config") &&
 		(!env.test.fix && commit >= env.test.culprit || env.test.fix &&
 			commit < env.test.culprit) {
-		var errors []error
 		if env.test.flaky {
-			errors = crashErrors(1, numVMs-1, "crash occurs")
+			ret = crashErrors(1, numVMs-1, "crash occurs")
 		} else {
-			errors = crashErrors(numVMs, 0, "crash occurs")
+			ret = crashErrors(numVMs, 0, "crash occurs")
 		}
-		return errors, nil
+		return ret, nil
 	}
-
-	return make([]error, numVMs), nil
+	return make([]instance.EnvTestResult, numVMs), nil
 }
 
 func (env *testEnv) headCommit() int {
@@ -504,17 +503,19 @@ func checkTest(t *testing.T, test BisectionTest) {
 	}
 }
 
-func crashErrors(crashing, nonCrashing int, title string) []error {
-	var errors []error
+func crashErrors(crashing, nonCrashing int, title string) []instance.EnvTestResult {
+	var ret []instance.EnvTestResult
 	for i := 0; i < crashing; i++ {
-		errors = append(errors, &instance.CrashError{
-			Report: &report.Report{
-				Title: fmt.Sprintf("crashes at %v", title),
+		ret = append(ret, instance.EnvTestResult{
+			Error: &instance.CrashError{
+				Report: &report.Report{
+					Title: fmt.Sprintf("crashes at %v", title),
+				},
 			},
 		})
 	}
 	for i := 0; i < nonCrashing; i++ {
-		errors = append(errors, nil)
+		ret = append(ret, instance.EnvTestResult{})
 	}
-	return errors
+	return ret
 }
diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go
index f5e84b8eb..74587fe60 100644
--- a/pkg/instance/instance.go
+++ b/pkg/instance/instance.go
@@ -30,7 +30,7 @@ import (
 type Env interface {
 	BuildSyzkaller(string, string) error
 	BuildKernel(string, string, string, string, string, []byte) (string, build.ImageDetails, error)
-	Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]error, error)
+	Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]EnvTestResult, error)
 }
 
 type env struct {
@@ -207,7 +207,7 @@ func (err *CrashError) Error() string {
 // Test boots numVMs VMs, tests basic kernel operation, and optionally tests the provided reproducer.
 // TestError is returned if there is a problem with kernel/image (crash, reboot loop, etc).
 // CrashError is returned if the reproducer crashes kernel.
-func (env *env) Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]error, error) {
+func (env *env) Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]EnvTestResult, error) {
 	if err := mgrconfig.Complete(env.cfg); err != nil {
 		return nil, err
 	}
@@ -222,7 +222,7 @@ func (env *env) Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]error, e
 	if n := vmPool.Count(); numVMs > n {
 		numVMs = n
 	}
-	res := make(chan error, numVMs)
+	res := make(chan EnvTestResult, numVMs)
 	for i := 0; i < numVMs; i++ {
 		inst := &inst{
 			cfg:           env.cfg,
@@ -236,11 +236,11 @@ func (env *env) Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]error, e
 		}
 		go func() { res <- inst.test() }()
 	}
-	var errors []error
+	var ret []EnvTestResult
 	for i := 0; i < numVMs; i++ {
-		errors = append(errors, <-res)
+		ret = append(ret, <-res)
 	}
-	return errors, nil
+	return ret, nil
 }
 
 type inst struct {
@@ -255,15 +255,24 @@ type inst struct {
 	reproC        []byte
 }
 
-func (inst *inst) test() error {
+type EnvTestResult struct {
+	Error     error
+	RawOutput []byte
+}
+
+func (inst *inst) test() EnvTestResult {
 	vmInst, err := inst.vmPool.Create(inst.vmIndex)
 	if err != nil {
 		testErr := &TestError{
 			Boot:  true,
 			Title: err.Error(),
 		}
+		ret := EnvTestResult{
+			Error: testErr,
+		}
 		if bootErr, ok := err.(vm.BootErrorer); ok {
 			testErr.Title, testErr.Output = bootErr.BootError()
+			ret.RawOutput = testErr.Output
 			rep := inst.reporter.Parse(testErr.Output)
 			if rep != nil && rep.Type == report.UnexpectedReboot {
 				// Avoid detecting any boot crash as "unexpected kernel reboot".
@@ -282,19 +291,18 @@ func (inst *inst) test() error {
 			testErr.Report = rep
 			testErr.Title = rep.Title
 		}
-		return testErr
+		return ret
 	}
 	defer vmInst.Close()
 	inst.vm = vmInst
-	if err := inst.testInstance(); err != nil {
-		return err
+	ret := EnvTestResult{}
+	if ret.Error = inst.testInstance(); ret.Error != nil {
+		return ret
 	}
 	if len(inst.reproSyz) != 0 || len(inst.reproC) != 0 {
-		if err := inst.testRepro(); err != nil {
-			return err
-		}
+		ret.RawOutput, ret.Error = inst.testRepro()
 	}
-	return nil
+	return ret
 }
 
 // testInstance tests basic operation of the provided VM
@@ -359,28 +367,29 @@ func (inst *inst) testInstance() error {
 	}
 }
 
-func (inst *inst) testRepro() error {
+func (inst *inst) testRepro() ([]byte, error) {
 	var err error
 	execProg, err := SetupExecProg(inst.vm, inst.cfg, inst.reporter, &OptionalConfig{
 		OldFlagsCompatMode: !inst.optionalFlags,
 	})
 	if err != nil {
-		return err
+		return nil, err
 	}
-	transformError := func(res *RunResult, err error) error {
+	transformError := func(res *RunResult, err error) ([]byte, error) {
 		if err != nil {
-			return err
+			return nil, err
 		}
 		if res != nil && res.Report != nil {
-			return &CrashError{Report: res.Report}
+			return res.RawOutput, &CrashError{Report: res.Report}
 		}
-		return nil
+		return res.RawOutput, nil
 	}
+	out := []byte{}
 	if len(inst.reproSyz) > 0 {
 		var opts csource.Options
 		opts, err = csource.DeserializeOptions(inst.reproOpts)
 		if err != nil {
-			return err
+			return nil, err
 		}
 		// Combine repro options and default options in a way that increases chances to reproduce the crash.
 		// First, we always enable threaded/collide as it should be [almost] strictly better.
@@ -390,16 +399,16 @@ func (inst *inst) testRepro() error {
 			opts.Sandbox = "none"
 		}
 		opts.Repeat, opts.Threaded = true, true
-		err = transformError(execProg.RunSyzProg(inst.reproSyz,
+		out, err = transformError(execProg.RunSyzProg(inst.reproSyz,
 			inst.cfg.Timeouts.NoOutputRunningTime, opts))
 	}
 	if err == nil && len(inst.reproC) > 0 {
 		// We should test for more than full "no output" timeout, but the problem is that C reproducers
 		// don't print anything, so we will get a false "no output" crash.
-		err = transformError(execProg.RunCProgRaw(inst.reproC, inst.cfg.Target,
+		out, err = transformError(execProg.RunCProgRaw(inst.reproC, inst.cfg.Target,
 			inst.cfg.Timeouts.NoOutput/2))
 	}
-	return err
+	return out, err
 }
 
 type OptionalFuzzerArgs struct {
diff --git a/syz-ci/jobs.go b/syz-ci/jobs.go
index 9d3e0df25..40c32d38e 100644
--- a/syz-ci/jobs.go
+++ b/syz-ci/jobs.go
@@ -494,7 +494,6 @@ func (jp *JobProcessor) bisect(job *Job, mgrcfg *mgrconfig.Config) error {
 
 func (jp *JobProcessor) testPatch(job *Job, mgrcfg *mgrconfig.Config) error {
 	req, resp, mgr := job.req, job.resp, job.mgr
-
 	env, err := instance.NewEnv(mgrcfg)
 	if err != nil {
 		return err
@@ -566,34 +565,40 @@ func (jp *JobProcessor) testPatch(job *Job, mgrcfg *mgrconfig.Config) error {
 	if err != nil {
 		return err
 	}
-	rep, err := aggregateTestResults(results)
+	ret, err := aggregateTestResults(results)
 	if err != nil {
 		return err
 	}
+	rep := ret.report
 	if rep != nil {
 		resp.CrashTitle = rep.Title
 		resp.CrashReport = rep.Report
-		resp.CrashLog = rep.Output
 	}
+	resp.CrashLog = ret.rawOutput
 	return nil
 }
 
-func aggregateTestResults(results []error) (*report.Report, error) {
+type patchTestResult struct {
+	report    *report.Report
+	rawOutput []byte
+}
+
+func aggregateTestResults(results []instance.EnvTestResult) (*patchTestResult, error) {
 	// We can have transient errors and other errors of different types.
 	// We need to avoid reporting transient "failed to boot" or "failed to copy binary" errors.
 	// If any of the instances crash during testing, we report this with the highest priority.
 	// Then if any of the runs succeed, we report that (to avoid transient errors).
 	// If all instances failed to boot, then we report one of these errors.
-	anySuccess := false
 	var anyErr, testErr error
-	var resReport *report.Report
+	var resReport, resSuccess *patchTestResult
+	anyErr = fmt.Errorf("no env test runs")
 	for _, res := range results {
-		if res == nil {
-			anySuccess = true
+		if res.Error == nil {
+			resSuccess = &patchTestResult{rawOutput: res.RawOutput}
 			continue
 		}
-		anyErr = res
-		switch err := res.(type) {
+		anyErr = res.Error
+		switch err := res.Error.(type) {
 		case *instance.TestError:
 			// We should not put rep into resp.CrashTitle/CrashReport,
 			// because that will be treated as patch not fixing the bug.
@@ -603,16 +608,16 @@ func aggregateTestResults(results []error) (*report.Report, error) {
 				testErr = fmt.Errorf("%v\n\n%s", err.Title, err.Output)
 			}
 		case *instance.CrashError:
-			if resReport == nil || (len(resReport.Report) == 0 && len(err.Report.Report) != 0) {
-				resReport = err.Report
+			if resReport == nil || (len(resReport.report.Report) == 0 && len(err.Report.Report) != 0) {
+				resReport = &patchTestResult{report: err.Report, rawOutput: res.RawOutput}
 			}
 		}
 	}
 	if resReport != nil {
 		return resReport, nil
 	}
-	if anySuccess {
-		return nil, nil
+	if resSuccess != nil {
+		return resSuccess, nil
 	}
 	if testErr != nil {
 		return nil, testErr
diff --git a/syz-ci/jobs_test.go b/syz-ci/jobs_test.go
index b80f2d9f4..1ad7b7efc 100644
--- a/syz-ci/jobs_test.go
+++ b/syz-ci/jobs_test.go
@@ -14,93 +14,99 @@ import (
 
 func TestAggregateTestResults(t *testing.T) {
 	tests := []struct {
-		results []error
+		results []instance.EnvTestResult
 		title   string
 		err     error
+		rawOut  []byte
 	}{
 		{
-			results: []error{nil, nil, nil},
+			results: []instance.EnvTestResult{{}, {}, {RawOutput: []byte{1, 2, 3}}},
 			title:   "",
 			err:     nil,
+			rawOut:  []byte{1, 2, 3},
 		},
 		{
-			results: []error{
-				&instance.CrashError{Report: &report.Report{Title: "title1"}},
-				&instance.CrashError{Report: &report.Report{Title: "title2"}},
-				&instance.CrashError{Report: &report.Report{Title: "title3"}},
+			results: []instance.EnvTestResult{
+				{Error: &instance.CrashError{Report: &report.Report{Title: "title1"}}},
+				{Error: &instance.CrashError{Report: &report.Report{Title: "title2"}}},
+				{Error: &instance.CrashError{Report: &report.Report{Title: "title3"}}},
 			},
 			title: "title1",
 			err:   nil,
 		},
 		{
-			results: []error{
-				nil,
-				&instance.CrashError{Report: &report.Report{Title: "title2"}},
-				nil,
+			results: []instance.EnvTestResult{
+				{},
+				{Error: &instance.CrashError{Report: &report.Report{Title: "title2"}}},
+				{},
 			},
 			title: "title2",
 			err:   nil,
 		},
 		{
-			results: []error{
-				&instance.TestError{Title: "test error1"},
-				&instance.CrashError{Report: &report.Report{Title: "title2"}},
-				&instance.TestError{Title: "test error2"},
+			results: []instance.EnvTestResult{
+				{Error: &instance.TestError{Title: "test error1"}},
+				{Error: &instance.CrashError{Report: &report.Report{Title: "title2"}}},
+				{Error: &instance.TestError{Title: "test error2"}},
 			},
 			title: "title2",
 			err:   nil,
 		},
 		{
-			results: []error{
-				&instance.TestError{Title: "test error1"},
-				&instance.TestError{Title: "test error2"},
-				nil,
+			results: []instance.EnvTestResult{
+				{Error: &instance.TestError{Title: "test error1"}},
+				{Error: &instance.TestError{Title: "test error2"}},
+				{},
 			},
 			title: "",
 			err:   nil,
 		},
 		{
-			results: []error{
-				&instance.TestError{Title: "test error1"},
-				&instance.TestError{Title: "test error2"},
-				&instance.TestError{Title: "test error3", Output: []byte("output")},
+			results: []instance.EnvTestResult{
+				{Error: &instance.TestError{Title: "test error1"}},
+				{Error: &instance.TestError{Title: "test error2"}},
+				{Error: &instance.TestError{Title: "test error3", Output: []byte("output")}},
 			},
 			title: "",
 			err:   errors.New("test error3\n\noutput"),
 		},
 		{
-			results: []error{
-				errors.New("infra error1"),
-				errors.New("infra error2"),
-				&instance.TestError{Title: "test error", Report: &report.Report{
+			results: []instance.EnvTestResult{
+				{Error: errors.New("infra error1")},
+				{Error: errors.New("infra error2")},
+				{Error: &instance.TestError{Title: "test error", Report: &report.Report{
 					Title:  "report title",
 					Report: []byte("report body"),
 					Output: []byte("output"),
-				}},
+				}}},
 			},
 			title: "",
 			err:   errors.New("report title\n\nreport body\n\noutput"),
 		},
 		{
-			results: []error{
-				errors.New("infra error1"),
-				errors.New("infra error2"),
-				errors.New("infra error3"),
+			results: []instance.EnvTestResult{
+				{Error: errors.New("infra error1")},
+				{Error: errors.New("infra error2")},
+				{Error: errors.New("infra error3")},
 			},
 			title: "",
 			err:   errors.New("infra error3"),
 		},
 		{
-			results: []error{
-				&instance.CrashError{Report: &report.Report{Title: "title1"}},
-				&instance.CrashError{Report: &report.Report{
-					Title:  "title2",
-					Report: []byte("report"),
-				}},
-				&instance.CrashError{Report: &report.Report{Title: "title3"}},
+			results: []instance.EnvTestResult{
+				{Error: &instance.CrashError{Report: &report.Report{Title: "title1"}}},
+				{Error: &instance.CrashError{
+					Report: &report.Report{
+						Title:  "title2",
+						Report: []byte("report"),
+					}},
+					RawOutput: []byte{2, 3, 4},
+				},
+				{Error: &instance.CrashError{Report: &report.Report{Title: "title3"}}},
 			},
-			title: "title2",
-			err:   nil,
+			title:  "title2",
+			err:    nil,
+			rawOut: []byte{2, 3, 4},
 		},
 	}
 	for i, test := range tests {
@@ -109,11 +115,18 @@ func TestAggregateTestResults(t *testing.T) {
 			t.Errorf("test #%v: got err: %q, want: %q", i, err, test.err)
 		}
 		got := ""
-		if rep != nil {
-			got = rep.Title
+		if rep != nil && rep.report != nil {
+			got = rep.report.Title
 		}
 		if got != test.title {
 			t.Errorf("test #%v: got title: %q, want: %q", i, got, test.title)
 		}
+		var gotOutput []byte
+		if rep != nil {
+			gotOutput = rep.rawOutput
+		}
+		if fmt.Sprint(test.rawOut) != fmt.Sprint(gotOutput) {
+			t.Errorf("test #%v: got raw out: %q, want: %q", i, gotOutput, test.rawOut)
+		}
 	}
 }
diff --git a/syz-ci/manager.go b/syz-ci/manager.go
index e8751d740..6f7a10ba5 100644
--- a/syz-ci/manager.go
+++ b/syz-ci/manager.go
@@ -415,11 +415,11 @@ func (mgr *Manager) testImage(imageDir string, info *BuildInfo) error {
 	failures := 0
 	var failureErr error
 	for _, res := range results {
-		if res == nil {
+		if res.Error == nil {
 			continue
 		}
 		failures++
-		switch err := res.(type) {
+		switch err := res.Error.(type) {
 		case *instance.TestError:
 			if rep := err.Report; rep != nil {
 				what := "test"
@@ -442,7 +442,7 @@ func (mgr *Manager) testImage(imageDir string, info *BuildInfo) error {
 				failureErr = fmt.Errorf("VM testing failed with: %v", err)
 			}
 		default:
-			failureErr = res
+			failureErr = res.Error
 		}
 	}
 	if failures > maxFailures {
diff --git a/tools/syz-testbuild/testbuild.go b/tools/syz-testbuild/testbuild.go
index 72c0d235b..961fb2e4c 100644
--- a/tools/syz-testbuild/testbuild.go
+++ b/tools/syz-testbuild/testbuild.go
@@ -149,11 +149,11 @@ func test(repo vcs.Repo, bisecter vcs.Bisecter, kernelConfig []byte, env instanc
 	}
 	var verdicts []string
 	for i, res := range results {
-		if res == nil {
+		if res.Error == nil {
 			verdicts = append(verdicts, "OK")
 			continue
 		}
-		switch err := res.(type) {
+		switch err := res.Error.(type) {
 		case *instance.TestError:
 			if err.Boot {
 				verdicts = append(verdicts, fmt.Sprintf("boot failed: %v", err))