From c6509052a04c3ba3d383b17946ebd6cdbb9ca89c Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Wed, 5 Jul 2023 15:35:45 +0200
Subject: pkg/report: ignore transient crashes

There are cases when a crash, even though it didn't occur during boot
and image testing phases, still should not be counted during bisection.
Otherwise we risk diverting the whole process in the wrong direction.

One of such problem classes is getting SYZFATAL errors when we are not
bisecting a SYZFATAL crash.
---
 pkg/bisect/bisect.go      | 19 +++++++++++++++++--
 pkg/bisect/bisect_test.go | 34 +++++++++++++++++++++++++++++-----
 2 files changed, 46 insertions(+), 7 deletions(-)

(limited to 'pkg')

diff --git a/pkg/bisect/bisect.go b/pkg/bisect/bisect.go
index 3d3de92b6..9668144ef 100644
--- a/pkg/bisect/bisect.go
+++ b/pkg/bisect/bisect.go
@@ -698,14 +698,19 @@ func (env *env) processResults(current *vcs.Commit, results []instance.EnvTestRe
 			}
 			env.saveDebugFile(current.Hash, i, output)
 		case *instance.CrashError:
-			bad++
-			reports = append(reports, err.Report)
 			verdicts = append(verdicts, fmt.Sprintf("crashed: %v", err))
 			output := err.Report.Report
 			if len(output) == 0 {
 				output = err.Report.Output
 			}
 			env.saveDebugFile(current.Hash, i, output)
+			if env.isTransientError(err.Report) {
+				verdicts = append(verdicts, fmt.Sprintf("ignore: %v", err))
+				break
+			}
+			bad++
+			reports = append(reports, err.Report)
+			verdicts = append(verdicts, fmt.Sprintf("crashed: %v", err))
 		default:
 			infra++
 			verdicts = append(verdicts, fmt.Sprintf("failed: %v", err))
@@ -791,6 +796,16 @@ func mostFrequentReports(reports []*report.Report) (*report.Report, []crash.Type
 	return bestReport, bestTypes, len(bestTypes) != len(perType)
 }
 
+func (env *env) isTransientError(rep *report.Report) bool {
+	// If we're not chasing a SYZFATAL error, ignore them.
+	// Otherwise it indicates some transient problem of the tested kernel revision.
+	hadSyzFailure := false
+	for _, t := range env.reportTypes {
+		hadSyzFailure = hadSyzFailure || t == crash.SyzFailure
+	}
+	return rep.Type == crash.SyzFailure && !hadSyzFailure
+}
+
 func (env *env) saveDebugFile(hash string, idx int, data []byte) {
 	env.cfg.Trace.SaveFile(fmt.Sprintf("%v.%v", hash, idx), data)
 }
diff --git a/pkg/bisect/bisect_test.go b/pkg/bisect/bisect_test.go
index 80709c85e..47251b8cc 100644
--- a/pkg/bisect/bisect_test.go
+++ b/pkg/bisect/bisect_test.go
@@ -93,7 +93,20 @@ func (env *testEnv) Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]inst
 		}
 		return ret, nil
 	}
-	return make([]instance.EnvTestResult, numVMs), nil
+	ret = make([]instance.EnvTestResult, numVMs-1)
+	if env.test.injectSyzFailure {
+		ret = append(ret, instance.EnvTestResult{
+			Error: &instance.TestError{
+				Report: &report.Report{
+					Title: "SYZFATAL: test",
+					Type:  crash.SyzFailure,
+				},
+			},
+		})
+	} else {
+		ret = append(ret, instance.EnvTestResult{})
+	}
+	return ret, nil
 }
 
 func (env *testEnv) headCommit() int {
@@ -259,10 +272,11 @@ type BisectionTest struct {
 	expectErr     bool
 	expectErrType any
 	// Expect res.Report != nil.
-	expectRep  bool
-	noopChange bool
-	isRelease  bool
-	flaky      bool
+	expectRep        bool
+	noopChange       bool
+	isRelease        bool
+	flaky            bool
+	injectSyzFailure bool
 	// Expected number of returned commits for inconclusive bisection.
 	commitLen int
 	// For cause bisection: Oldest commit returned by bisection.
@@ -404,6 +418,16 @@ var bisectionTests = []BisectionTest{
 		culprit:     500,
 		isRelease:   true,
 	},
+	// Tests that bisection returns the correct fix commit despite SYZFATAL.
+	{
+		name:             "fix-finds-fix-despite-syzfatal",
+		fix:              true,
+		startCommit:      400,
+		injectSyzFailure: true,
+		commitLen:        1,
+		culprit:          500,
+		isRelease:        true,
+	},
 	// Tests that fix bisection returns error when crash does not reproduce
 	// on the original commit.
 	{
-- 
cgit mrf-deployment