Skip to content

Commit db79ee4

Browse files
committed
pkg/bisect: ignore irrelevant lost connection crashes
These have been the cause of too many invalid bisection results recently. This is the first step towards the more universal approach of #5414.
1 parent 0706496 commit db79ee4

File tree

2 files changed

+48
-16
lines changed

2 files changed

+48
-16
lines changed

pkg/bisect/bisect.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,12 +1014,24 @@ func mostFrequentReports(reports []*report.Report) (*report.Report, []crash.Type
10141014
func (env *env) isTransientError(rep *report.Report) bool {
10151015
// If we're not chasing a SYZFATAL error, ignore them.
10161016
// Otherwise it indicates some transient problem of the tested kernel revision.
1017-
hadSyzFailure := false
1018-
for _, t := range env.reportTypes {
1019-
hadSyzFailure = hadSyzFailure || t == crash.SyzFailure
1017+
if rep.Type == crash.SyzFailure {
1018+
hadSyzFailure := false
1019+
for _, t := range env.reportTypes {
1020+
hadSyzFailure = hadSyzFailure || t == crash.SyzFailure
1021+
}
1022+
return len(env.reportTypes) > 0 && !hadSyzFailure
1023+
}
1024+
// Lost connection is a frequent source of flaky results.
1025+
// Ignore if it is was not in the canonical crash types set.
1026+
if rep.Type == crash.LostConnection {
1027+
hadLostConnection := false
1028+
for _, t := range env.reportTypes {
1029+
hadLostConnection = hadLostConnection || t == crash.LostConnection
1030+
}
1031+
return len(env.reportTypes) > 0 && !hadLostConnection
10201032
}
1021-
return rep.Type == crash.SyzFailure &&
1022-
len(env.reportTypes) > 0 && !hadSyzFailure
1033+
// All other errors are okay.
1034+
return false
10231035
}
10241036

10251037
func (env *env) saveDebugFile(hash string, idx int, data []byte) {

pkg/bisect/bisect_test.go

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,18 +116,27 @@ func (env *testEnv) Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]inst
116116
}
117117
return ret, nil
118118
}
119-
ret = make([]instance.EnvTestResult, numVMs-1)
119+
ret = make([]instance.EnvTestResult, numVMs)
120120
if env.test.injectSyzFailure {
121-
ret = append(ret, instance.EnvTestResult{
122-
Error: &instance.TestError{
121+
ret[0] = instance.EnvTestResult{
122+
Error: &instance.CrashError{
123123
Report: &report.Report{
124124
Title: "SYZFATAL: test",
125125
Type: crash.SyzFailure,
126126
},
127127
},
128-
})
129-
} else {
130-
ret = append(ret, instance.EnvTestResult{})
128+
}
129+
} else if env.test.injectLostConnection {
130+
for i := 0; i < numVMs/3; i++ {
131+
ret[i] = instance.EnvTestResult{
132+
Error: &instance.CrashError{
133+
Report: &report.Report{
134+
Title: "lost connection to test machine",
135+
Type: crash.LostConnection,
136+
},
137+
},
138+
}
139+
}
131140
}
132141
return ret, nil
133142
}
@@ -312,11 +321,12 @@ type BisectionTest struct {
312321
expectErr bool
313322
expectErrType any
314323
// Expect res.Report != nil.
315-
expectRep bool
316-
noopChange bool
317-
isRelease bool
318-
flaky bool
319-
injectSyzFailure bool
324+
expectRep bool
325+
noopChange bool
326+
isRelease bool
327+
flaky bool
328+
injectSyzFailure bool
329+
injectLostConnection bool
320330
// Expected number of returned commits for inconclusive bisection.
321331
commitLen int
322332
// For cause bisection: Oldest commit returned by bisection.
@@ -496,6 +506,16 @@ var bisectionTests = []BisectionTest{
496506
fixCommit: "500",
497507
isRelease: true,
498508
},
509+
// Tests that bisection returns the correct fix commit despite `lost connection to test machine`.
510+
{
511+
name: "fix-finds-fix-despite-lost-connection",
512+
fix: true,
513+
startCommit: 400,
514+
injectLostConnection: true,
515+
commitLen: 1,
516+
fixCommit: "500",
517+
isRelease: true,
518+
},
499519
// Tests that bisection returns the correct fix commit in case of SYZFATAL.
500520
{
501521
name: "fix-finds-fix-for-syzfatal",

0 commit comments

Comments
 (0)