pkg/bisect: various improvements

A bunch of improvements after more wide bisection testing.
Improve logging.
Support returning several commits for inconclusive bisection.
Return Report with the final crash.
Remove code that was moved to pkg/vcs.

Update #501

1 files changed, 110 insertions, 115 deletions

diff --git a/pkg/bisect/bisect.go b/pkg/bisect/bisect.go
index 10a2eff9e..21b3a64b1 100644
--- a/pkg/bisect/bisect.go
+++ b/pkg/bisect/bisect.go
@@ -13,6 +13,7 @@ import (
 	"github.com/google/syzkaller/pkg/instance"
 	"github.com/google/syzkaller/pkg/mgrconfig"
 	"github.com/google/syzkaller/pkg/osutil"
+	"github.com/google/syzkaller/pkg/report"
 	"github.com/google/syzkaller/pkg/vcs"
 )
 
@@ -52,6 +53,7 @@ type ReproConfig struct {
 type env struct {
 	cfg       *Config
 	repo      vcs.Repo
+	bisecter  vcs.Bisecter
 	head      *vcs.Commit
 	inst      *instance.Env
 	numTests  int
@@ -59,85 +61,113 @@ type env struct {
 	testTime  time.Duration
 }
 
-type buildEnv struct {
-	compiler string
-}
-
-func Run(cfg *Config) (*vcs.Commit, error) {
+// Run does the bisection and returns:
+//  - if bisection is conclusive, the single cause/fix commit
+//    - for cause bisection report is the crash on the cause commit
+//    - for fix bisection report is nil
+//  - if bisection is inconclusive, range of potential cause/fix commits
+//    - report is nil in such case
+//  - if the crash still happens on the oldest release/HEAD (for cause/fix bisection correspondingly),
+//    no commits and the crash report on the oldest release/HEAD
+//  - if the crash is not reproduced on the start commit, an error
+func Run(cfg *Config) ([]*vcs.Commit, *report.Report, error) {
 	if err := checkConfig(cfg); err != nil {
-		return nil, err
+		return nil, nil, err
 	}
+	cfg.Manager.Cover = false // it's not supported somewhere back in time
 	repo, err := vcs.NewRepo(cfg.Manager.TargetOS, cfg.Manager.Type, cfg.Manager.KernelSrc)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
+	}
+	bisecter, ok := repo.(vcs.Bisecter)
+	if !ok {
+		return nil, nil, fmt.Errorf("bisection is not implemented for %v", cfg.Manager.TargetOS)
 	}
 	env := &env{
-		cfg:  cfg,
-		repo: repo,
+		cfg:      cfg,
+		repo:     repo,
+		bisecter: bisecter,
 	}
 	if cfg.Fix {
-		env.log("searching for fixing commit since %v", cfg.Kernel.Commit)
+		env.log("bisecting fixing commit since %v", cfg.Kernel.Commit)
 	} else {
-		env.log("searching for guilty commit starting from %v", cfg.Kernel.Commit)
+		env.log("bisecting cause commit starting from %v", cfg.Kernel.Commit)
 	}
 	start := time.Now()
-	res, err := env.bisect()
+	commits, rep, err := env.bisect()
 	env.log("revisions tested: %v, total time: %v (build: %v, test: %v)",
 		env.numTests, time.Since(start), env.buildTime, env.testTime)
 	if err != nil {
 		env.log("error: %v", err)
-		return nil, err
+		return nil, nil, err
 	}
-	if res == nil {
-		env.log("the crash is still unfixed")
-		return nil, nil
+	if len(commits) == 0 {
+		if cfg.Fix {
+			env.log("the crash still happens on HEAD")
+		} else {
+			env.log("the crash already happened on the oldest tested release")
+		}
+		env.log("crash: %v\n%s", rep.Title, rep.Report)
+		return nil, rep, nil
 	}
 	what := "bad"
 	if cfg.Fix {
 		what = "good"
 	}
-	env.log("first %v commit: %v %v", what, res.Hash, res.Title)
-	env.log("cc: %q", res.CC)
-	return res, nil
+	if len(commits) > 1 {
+		env.log("bisection is inconclusive, the first %v commit could be any of:", what)
+		for _, com := range commits {
+			env.log("%v", com.Hash)
+		}
+		return commits, nil, nil
+	}
+	com := commits[0]
+	env.log("first %v commit: %v %v", what, com.Hash, com.Title)
+	env.log("cc: %q", com.CC)
+	if rep != nil {
+		env.log("crash: %v\n%s", rep.Title, rep.Report)
+	}
+	return commits, rep, nil
 }
 
-func (env *env) bisect() (*vcs.Commit, error) {
+func (env *env) bisect() ([]*vcs.Commit, *report.Report, error) {
 	cfg := env.cfg
 	var err error
 	if env.inst, err = instance.NewEnv(&cfg.Manager); err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	if env.head, err = env.repo.Poll(cfg.Kernel.Repo, cfg.Kernel.Branch); err != nil {
-		return nil, err
+	if env.head, err = env.repo.CheckoutBranch(cfg.Kernel.Repo, cfg.Kernel.Branch); err != nil {
+		return nil, nil, err
 	}
 	if err := build.Clean(cfg.Manager.TargetOS, cfg.Manager.TargetVMArch,
 		cfg.Manager.Type, cfg.Manager.KernelSrc); err != nil {
-		return nil, fmt.Errorf("kernel clean failed: %v", err)
+		return nil, nil, fmt.Errorf("kernel clean failed: %v", err)
 	}
 	env.log("building syzkaller on %v", cfg.Syzkaller.Commit)
 	if err := env.inst.BuildSyzkaller(cfg.Syzkaller.Repo, cfg.Syzkaller.Commit); err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	if _, err := env.repo.SwitchCommit(cfg.Kernel.Commit); err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	if res, err := env.test(); err != nil {
-		return nil, err
+	res, _, rep0, err := env.test()
+	if err != nil {
+		return nil, nil, err
 	} else if res != vcs.BisectBad {
-		return nil, fmt.Errorf("the crash wasn't reproduced on the original commit")
+		return nil, nil, fmt.Errorf("the crash wasn't reproduced on the original commit")
 	}
-	res, bad, good, err := env.commitRange()
+	bad, good, rep1, err := env.commitRange()
 	if err != nil {
-		return nil, err
-	}
-	if res != nil {
-		return res, nil // happens on the oldest release
+		return nil, nil, err
 	}
 	if good == "" {
-		return nil, nil // still not fixed
+		return nil, rep1, nil // still not fixed/happens on the oldest release
 	}
-	commits, err := env.repo.(vcs.Bisecter).Bisect(bad, good, cfg.Trace, func() (vcs.BisectResult, error) {
-		res, err := env.test()
+	reports := make(map[string]*report.Report)
+	reports[cfg.Kernel.Commit] = rep0
+	commits, err := env.bisecter.Bisect(bad, good, cfg.Trace, func() (vcs.BisectResult, error) {
+		res, com, rep, err := env.test()
+		reports[com.Hash] = rep
 		if cfg.Fix {
 			if res == vcs.BisectBad {
 				res = vcs.BisectGood
@@ -147,130 +177,121 @@ func (env *env) bisect() (*vcs.Commit, error) {
 		}
 		return res, err
 	})
-	if err != nil {
-		return nil, err
-	}
+	var rep *report.Report
 	if len(commits) == 1 {
-		return commits[0], nil
+		rep = reports[commits[0].Hash]
 	}
-	return nil, nil
+	return commits, rep, err
 }
 
-func (env *env) commitRange() (*vcs.Commit, string, string, error) {
+func (env *env) commitRange() (string, string, *report.Report, error) {
 	if env.cfg.Fix {
 		return env.commitRangeForFix()
 	}
 	return env.commitRangeForBug()
 }
 
-func (env *env) commitRangeForFix() (*vcs.Commit, string, string, error) {
+func (env *env) commitRangeForFix() (string, string, *report.Report, error) {
 	env.log("testing current HEAD %v", env.head.Hash)
 	if _, err := env.repo.SwitchCommit(env.head.Hash); err != nil {
-		return nil, "", "", err
+		return "", "", nil, err
 	}
-	res, err := env.test()
+	res, _, rep, err := env.test()
 	if err != nil {
-		return nil, "", "", err
+		return "", "", nil, err
 	}
 	if res != vcs.BisectGood {
-		return nil, "", "", nil
+		return "", "", rep, nil
 	}
-	return nil, env.head.Hash, env.cfg.Kernel.Commit, nil
+	return env.head.Hash, env.cfg.Kernel.Commit, nil, nil
 }
 
-func (env *env) commitRangeForBug() (*vcs.Commit, string, string, error) {
+func (env *env) commitRangeForBug() (string, string, *report.Report, error) {
 	cfg := env.cfg
-	tags, err := env.repo.(vcs.Bisecter).PreviousReleaseTags(cfg.Kernel.Commit)
+	tags, err := env.bisecter.PreviousReleaseTags(cfg.Kernel.Commit)
 	if err != nil {
-		return nil, "", "", err
-	}
-	for i, tag := range tags {
-		if tag == "v3.8" {
-			// v3.8 does not work with modern perl, and as we go further in history
-			// make stops to work, then binutils, glibc, etc. So we stop at v3.8.
-			// Up to that point we only need an ancient gcc.
-			tags = tags[:i]
-			break
-		}
+		return "", "", nil, err
 	}
 	if len(tags) == 0 {
-		return nil, "", "", fmt.Errorf("no release tags before this commit")
+		return "", "", nil, fmt.Errorf("no release tags before this commit")
 	}
 	lastBad := cfg.Kernel.Commit
-	for i, tag := range tags {
+	var lastRep *report.Report
+	for _, tag := range tags {
 		env.log("testing release %v", tag)
-		commit, err := env.repo.SwitchCommit(tag)
-		if err != nil {
-			return nil, "", "", err
+		if _, err := env.repo.SwitchCommit(tag); err != nil {
+			return "", "", nil, err
 		}
-		res, err := env.test()
+		res, _, rep, err := env.test()
 		if err != nil {
-			return nil, "", "", err
+			return "", "", nil, err
 		}
 		if res == vcs.BisectGood {
-			return nil, lastBad, tag, nil
+			return lastBad, tag, nil, nil
 		}
 		if res == vcs.BisectBad {
 			lastBad = tag
-		}
-		if i == len(tags)-1 {
-			return commit, "", "", nil
+			lastRep = rep
 		}
 	}
-	panic("unreachable")
+	return "", "", lastRep, nil
 }
 
-func (env *env) test() (vcs.BisectResult, error) {
+func (env *env) test() (vcs.BisectResult, *vcs.Commit, *report.Report, error) {
 	cfg := env.cfg
 	env.numTests++
 	current, err := env.repo.HeadCommit()
 	if err != nil {
-		return 0, err
+		return 0, nil, nil, err
 	}
-	be, err := env.buildEnvForCommit(current.Hash)
+	bisectEnv, err := env.bisecter.EnvForCommit(current.Hash, cfg.Kernel.Config)
 	if err != nil {
-		return 0, err
+		return 0, nil, nil, err
 	}
-	compilerID, err := build.CompilerIdentity(be.compiler)
+	compiler := filepath.Join(cfg.BinDir, bisectEnv.Compiler, "bin", "gcc")
+	compilerID, err := build.CompilerIdentity(compiler)
 	if err != nil {
-		return 0, err
+		return 0, nil, nil, err
 	}
 	env.log("testing commit %v with %v", current.Hash, compilerID)
 	buildStart := time.Now()
 	if err := build.Clean(cfg.Manager.TargetOS, cfg.Manager.TargetVMArch,
 		cfg.Manager.Type, cfg.Manager.KernelSrc); err != nil {
-		return 0, fmt.Errorf("kernel clean failed: %v", err)
+		return 0, nil, nil, fmt.Errorf("kernel clean failed: %v", err)
 	}
-	err = env.inst.BuildKernel(be.compiler, cfg.Kernel.Userspace,
-		cfg.Kernel.Cmdline, cfg.Kernel.Sysctl, cfg.Kernel.Config)
+	err = env.inst.BuildKernel(compiler, cfg.Kernel.Userspace,
+		cfg.Kernel.Cmdline, cfg.Kernel.Sysctl, bisectEnv.KernelConfig)
 	env.buildTime += time.Since(buildStart)
 	if err != nil {
 		if verr, ok := err.(*osutil.VerboseError); ok {
 			env.log("%v", verr.Title)
 			env.saveDebugFile(current.Hash, 0, verr.Output)
+		} else if verr, ok := err.(build.KernelBuildError); ok {
+			env.log("%v", verr.Title)
+			env.saveDebugFile(current.Hash, 0, verr.Output)
 		} else {
 			env.log("%v", err)
 		}
-		return vcs.BisectSkip, nil
+		return vcs.BisectSkip, current, nil, nil
 	}
 	testStart := time.Now()
-	results, err := env.inst.Test(8, cfg.Repro.Syz, cfg.Repro.Opts, cfg.Repro.C)
+	results, err := env.inst.Test(10, cfg.Repro.Syz, cfg.Repro.Opts, cfg.Repro.C)
 	env.testTime += time.Since(testStart)
 	if err != nil {
 		env.log("failed: %v", err)
-		return vcs.BisectSkip, nil
+		return vcs.BisectSkip, current, nil, nil
 	}
-	bad, good := env.processResults(current, results)
+	bad, good, rep := env.processResults(current, results)
 	res := vcs.BisectSkip
 	if bad != 0 {
 		res = vcs.BisectBad
 	} else if good != 0 {
 		res = vcs.BisectGood
 	}
-	return res, nil
+	return res, current, rep, nil
 }
 
-func (env *env) processResults(current *vcs.Commit, results []error) (bad, good int) {
+func (env *env) processResults(current *vcs.Commit, results []error) (bad, good int, rep *report.Report) {
 	var verdicts []string
 	for i, res := range results {
 		if res == nil {
@@ -292,6 +313,7 @@ func (env *env) processResults(current *vcs.Commit, results []error) (bad, good
 			env.saveDebugFile(current.Hash, i, output)
 		case *instance.CrashError:
 			bad++
+			rep = err.Report
 			verdicts = append(verdicts, fmt.Sprintf("crashed: %v", err))
 			output := err.Report.Report
 			if len(output) == 0 {
@@ -316,33 +338,6 @@ func (env *env) processResults(current *vcs.Commit, results []error) (bad, good
 	return
 }
 
-// Note: linux-specific.
-func (env *env) buildEnvForCommit(commit string) (*buildEnv, error) {
-	cfg := env.cfg
-	tags, err := env.repo.(vcs.Bisecter).PreviousReleaseTags(commit)
-	if err != nil {
-		return nil, err
-	}
-	be := &buildEnv{
-		compiler: filepath.Join(cfg.BinDir, "gcc-"+linuxCompilerVersion(tags), "bin", "gcc"),
-	}
-	return be, nil
-}
-
-func linuxCompilerVersion(tags []string) string {
-	for _, tag := range tags {
-		switch tag {
-		case "v4.12":
-			return "8.1.0"
-		case "v4.11":
-			return "7.3.0"
-		case "v3.19":
-			return "5.5.0"
-		}
-	}
-	return "4.9.4"
-}
-
 func (env *env) saveDebugFile(hash string, idx int, data []byte) {
 	if env.cfg.DebugDir == "" || len(data) == 0 {
 		return