all: remember console output for all patch tests

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

Closes #3185

3 files changed, 52 insertions, 41 deletions

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 {