pkg/bisect: mark jobs with untestable history as failed

Background info:

When a bisection is started, we only know the kernel commit on which
syzkaller encountered the crash. Before the actual bisection begins,
bisect() needs to find a good commit. Then we can bisect between them.

For fix bisections bisect() tests HEAD. If HEAD doesn't have the bug
the fixing commit must be somewhere in the middle. For cause bisection
we test a number of old releases, starting with the newest release.

Both this commit range search and the actual bisection later use test()
to build the kernel and run the reproducer.

During actual bisections we invoke test() for every step. If any test()
invocation returns a non nil error, the bisection it was called from
is aborted. Any non-fatal errors should be signaled via the testResult
returned from test(). For this reason a build/boot failure does not
return an error. Instead testResult.verdict will be vcs.BisectSkip.

The Problem:

Given the following call stack:
bisect() -> commitRange() -> commitRangeFor{Fix,Cause}() -> test()

Previously we reported fix bisections, where HEAD was build broken and
cause bisections where all tested releases were build broken as
inconclusive.

This is confusing for users. For fix bisections it looks like the
fixing commit is either the commit from the original crash or HEAD.
For cause bisections it looks like the breaking commit is either the
original commit or the commit of the oldest tested release. Neither
is correct.

For fix bisections we see this, when the HEAD of a tested branch is
build broken. When this happens all attempted fix bisections will get
nonsense results. For cause bisections we see this, when changes to the
bisection compilers or test rootfs cause us to not be able to build or
boot very old kernels.

These inconclusive bisection results will not be retried automatically
and we don't have an easy way to clear them.

The Solution:

For fix bisections: Retry the bisection later. If HEAD is completely
broken we will know, because fuzzing will stop.

For cause bisections: Mark the bisection as failed. The result is
unlikely to change in the future without intervention by the syzbot
admins. Users won't bother looking at those bisections and the
dashboard already has code to mass-retry failed bisections.

- In test(): Populate testResult.rep with a meaningful report before
  returning, after build/boot failures.
- In bisect(): Explicitly check the testResult.verdict of the last
  commit tested in commitRange(), instead of using testResult.rep==nil
  as an oracle for aborting the bisection.
- In bisect(): Don't return an inconclusive result when build/boot
  failures prevent us from finding a commit range to bisect between.

1 files changed, 37 insertions, 11 deletions

diff --git a/pkg/bisect/bisect_test.go b/pkg/bisect/bisect_test.go
index 2d613759b..d5c369272 100644
--- a/pkg/bisect/bisect_test.go
+++ b/pkg/bisect/bisect_test.go
@@ -203,14 +203,19 @@ type BisectionTest struct {
 	sameBinaryStart int
 	sameBinaryEnd   int
 	// expected output
-	expectErr    bool
-	expectRep    bool
-	noopChange   bool
-	isRelease    bool
-	flaky        bool
-	commitLen    int
+	expectErr bool
+	// Expect res.Report != nil.
+	expectRep  bool
+	noopChange bool
+	isRelease  bool
+	flaky      bool
+	// Expected number of returned commits for inconclusive bisection.
+	commitLen int
+	// For cause bisection: Oldest commit returned by bisection.
+	// For fix bisection: Newest commit returned by bisection.
 	oldestLatest int
-	// input and output
+	// For cause bisection: The commit introducing the bug.
+	// For fix bisection: The commit fixing the bug.
 	culprit         int
 	baselineConfig  string
 	resultingConfig string
@@ -329,7 +334,11 @@ var bisectionTests = []BisectionTest{
 		startCommit: 802,
 		brokenStart: 100,
 		brokenEnd:   800,
-		commitLen:   2,
+		// We mark these as failed, because build/boot failures of ancient releases are unlikely to get fixed
+		// without manual intervention by syz-ci admins.
+		commitLen: 0,
+		expectRep: false,
+		expectErr: true,
 	},
 	// Tests that bisection returns the correct fix commit.
 	{
@@ -348,16 +357,33 @@ var bisectionTests = []BisectionTest{
 		startCommit: 905,
 		expectErr:   true,
 	},
+	// Tests that no commits are returned when HEAD is build broken.
+	// Fix bisection equivalent of all-releases-broken.
+	{
+		name:         "fix-HEAD-broken",
+		fix:          true,
+		startCommit:  400,
+		brokenStart:  500,
+		brokenEnd:    1000,
+		culprit:      1000,
+		oldestLatest: 905,
+		// We mark these as re-tryable, because build/boot failures of HEAD will also be caught during regular fuzzing
+		// and are fixed by kernel devs or syz-ci admins in a timely manner.
+		commitLen: 0,
+		expectRep: true,
+		expectErr: false,
+	},
 	// Tests that no commits are returned when crash occurs on HEAD
 	// for fix bisection.
 	{
-		name:         "fix-crashes-HEAD",
+		name:         "fix-HEAD-crashes",
 		fix:          true,
 		startCommit:  400,
-		commitLen:    0,
-		expectRep:    true,
 		culprit:      1000,
 		oldestLatest: 905,
+		commitLen:    0,
+		expectRep:    true,
+		expectErr:    false,
 	},
 	// Tests that more than 1 commit is returned when fix bisection is inconclusive.
 	{