pkg/build: use the build environment in clean() calls

This unifies the build() and clean() interfaces such that if a custom
compiler or make binary is provided in the manager or bisection config,
they can be taken into account by the clean() interface.

pkg/bisect/bisect.go

@@ -242,8 +242,18 @@ func (env *env) bisect() (*Result, error) {
 	}
 
 	cfg := env.cfg
-	if err := build.Clean(cfg.Manager.TargetOS, cfg.Manager.TargetVMArch,
-		cfg.Manager.Type, cfg.Manager.KernelSrc); err != nil {
+	buildCfg := build.BuildKernelConfig{
+		CompilerBin:  cfg.DefaultCompiler,
+		MakeBin:      cfg.Make,
+		LinkerBin:    cfg.Linker,
+		CcacheBin:    cfg.Ccache,
+		UserspaceDir: cfg.Kernel.Userspace,
+		CmdlineFile:  cfg.Kernel.Cmdline,
+		SysctlFile:   cfg.Kernel.Sysctl,
+		KernelConfig: cfg.Kernel.Config,
+		BuildCPUs:    cfg.BuildCPUs,
+	}
+	if err := env.inst.CleanKernel(buildCfg); err != nil {
 		return nil, fmt.Errorf("kernel clean failed: %w", err)
 	}
 	env.logf("building syzkaller on %v", cfg.Syzkaller.Commit)
@@ -606,12 +616,8 @@ func (env *env) build() (*vcs.Commit, string, error) {
 	}
 	env.logf("testing commit %v %v", current.Hash, env.cfg.CompilerType)
 	buildStart := time.Now()
-	mgr := env.cfg.Manager
-	if err := build.Clean(mgr.TargetOS, mgr.TargetVMArch, mgr.Type, mgr.KernelSrc); err != nil {
-		return current, "", fmt.Errorf("kernel clean failed: %w", err)
-	}
 	kern := &env.cfg.Kernel
-	_, imageDetails, err := env.inst.BuildKernel(&instance.BuildKernelConfig{
+	buildCfg := build.BuildKernelConfig{
 		CompilerBin:  bisectEnv.Compiler,
 		MakeBin:      env.cfg.Make,
 		LinkerBin:    env.cfg.Linker,
@@ -621,7 +627,11 @@ func (env *env) build() (*vcs.Commit, string, error) {
 		SysctlFile:   kern.Sysctl,
 		KernelConfig: bisectEnv.KernelConfig,
 		BuildCPUs:    env.cfg.BuildCPUs,
-	})
+	}
+	if err := env.inst.CleanKernel(buildCfg); err != nil {
+		return current, "", fmt.Errorf("kernel clean failed: %w", err)
+	}
+	_, imageDetails, err := env.inst.BuildKernel(buildCfg)
 	if imageDetails.CompilerID != "" {
 		env.logf("compiler: %v", imageDetails.CompilerID)
 	}

pkg/bisect/bisect_test.go

@@ -35,7 +35,11 @@ func (env *testEnv) BuildSyzkaller(repo, commit string) (string, error) {
 	return "", nil
 }
 
-func (env *testEnv) BuildKernel(buildCfg *instance.BuildKernelConfig) (string, build.ImageDetails, error) {
+func (env *testEnv) CleanKernel(buildCfg build.BuildKernelConfig) error {
+	return nil
+}
+
+func (env *testEnv) BuildKernel(buildCfg build.BuildKernelConfig) (string, build.ImageDetails, error) {
 	commit := env.headCommit()
 	configHash := hash.String(buildCfg.KernelConfig)
 	details := build.ImageDetails{}

pkg/build/android.go

@@ -61,10 +61,10 @@ func (a android) readCompiler(path string) (string, error) {
 
 func (a android) build(params Params) (ImageDetails, error) {
 	var details ImageDetails
-	if params.CmdlineFile != "" {
+	if params.BuildCfg.CmdlineFile != "" {
 		return details, fmt.Errorf("cmdline file is not supported for android images")
 	}
-	if params.SysctlFile != "" {
+	if params.BuildCfg.SysctlFile != "" {
 		return details, fmt.Errorf("sysctl file is not supported for android images")
 	}
 
@@ -179,11 +179,11 @@ func (a android) embedImages(w io.Writer, srcDir string, imageNames ...string) e
 	return nil
 }
 
-func (a android) clean(kernelDir, targetArch string) error {
-	if err := osutil.RemoveAll(filepath.Join(kernelDir, "out")); err != nil {
+func (a android) clean(params Params) error {
+	if err := osutil.RemoveAll(filepath.Join(params.KernelDir, "out")); err != nil {
 		return fmt.Errorf("failed to clean 'out' directory: %w", err)
 	}
-	if err := osutil.RemoveAll(filepath.Join(kernelDir, "dist")); err != nil {
+	if err := osutil.RemoveAll(filepath.Join(params.KernelDir, "dist")); err != nil {
 		return fmt.Errorf("failed to clean 'dist' directory: %w", err)
 	}
 	return nil

pkg/build/build.go

@@ -23,24 +23,28 @@ import (
 	"github.com/google/syzkaller/sys/targets"
 )
 
-// Params is input arguments for the Image function.
-type Params struct {
-	TargetOS     string
-	TargetArch   string
-	VMType       string
-	KernelDir    string
-	OutputDir    string
-	Compiler     string
-	Make         string
-	Linker       string
-	Ccache       string
+type BuildKernelConfig struct {
+	MakeBin      string
+	CompilerBin  string
+	LinkerBin    string
+	CcacheBin    string
 	UserspaceDir string
 	CmdlineFile  string
 	SysctlFile   string
-	Config       []byte
-	Tracer       debugtracer.DebugTracer
-	BuildCPUs    int // If 0, all CPUs will be used.
-	Build        json.RawMessage
+	KernelConfig []byte
+	BuildCPUs    int
+}
+
+// Params is input arguments for the Image and Clean functions.
+type Params struct {
+	TargetOS   string
+	TargetArch string
+	VMType     string
+	KernelDir  string
+	OutputDir  string
+	Tracer     debugtracer.DebugTracer
+	Build      json.RawMessage
+	BuildCfg   BuildKernelConfig
 }
 
 // Information that is returned from the Image function.
@@ -74,8 +78,8 @@ func Image(params Params) (details ImageDetails, err error) {
 	if params.Tracer == nil {
 		params.Tracer = &debugtracer.NullTracer{}
 	}
-	if params.BuildCPUs == 0 {
-		params.BuildCPUs = runtime.NumCPU()
+	if params.BuildCfg.BuildCPUs == 0 {
+		params.BuildCfg.BuildCPUs = runtime.NumCPU()
 	}
 	var builder builder
 	builder, err = getBuilder(params.TargetOS, params.TargetArch, params.VMType)
@@ -85,9 +89,10 @@ func Image(params Params) (details ImageDetails, err error) {
 	if err = osutil.MkdirAll(filepath.Join(params.OutputDir, "obj")); err != nil {
 		return
 	}
-	if len(params.Config) != 0 {
+	if len(params.BuildCfg.KernelConfig) != 0 {
 		// Write kernel config early, so that it's captured on build failures.
-		if err = osutil.WriteFile(filepath.Join(params.OutputDir, "kernel.config"), params.Config); err != nil {
+		err = osutil.WriteFile(filepath.Join(params.OutputDir, "kernel.config"), params.BuildCfg.KernelConfig)
+		if err != nil {
 			err = fmt.Errorf("failed to write config file: %w", err)
 			return
 		}
@@ -96,7 +101,7 @@ func Image(params Params) (details ImageDetails, err error) {
 	if details.CompilerID == "" {
 		// Fill in the compiler info even if the build failed.
 		var idErr error
-		details.CompilerID, idErr = compilerIdentity(params.Compiler)
+		details.CompilerID, idErr = compilerIdentity(params.BuildCfg.CompilerBin)
 		if err == nil {
 			err = idErr
 		} // Try to preserve the build error otherwise.
@@ -113,12 +118,12 @@ func Image(params Params) (details ImageDetails, err error) {
 	return
 }
 
-func Clean(targetOS, targetArch, vmType, kernelDir string) error {
-	builder, err := getBuilder(targetOS, targetArch, vmType)
+func Clean(params Params) error {
+	builder, err := getBuilder(params.TargetOS, params.TargetArch, params.VMType)
 	if err != nil {
 		return err
 	}
-	return builder.clean(kernelDir, targetArch)
+	return builder.clean(params)
 }
 
 type KernelError struct {
@@ -146,7 +151,7 @@ func (e InfraError) Error() string {
 
 type builder interface {
 	build(params Params) (ImageDetails, error)
-	clean(kernelDir, targetArch string) error
+	clean(params Params) error
 }
 
 func getBuilder(targetOS, targetArch, vmType string) (builder, error) {

pkg/build/cuttlefish.go

@@ -80,10 +80,10 @@ func (c cuttlefish) readCompiler(archivePath string) (string, error) {
 func (c cuttlefish) build(params Params) (ImageDetails, error) {
 	var details ImageDetails
 
-	if params.CmdlineFile != "" {
+	if params.BuildCfg.CmdlineFile != "" {
 		return details, fmt.Errorf("cmdline file is not supported for android cuttlefish images")
 	}
-	if params.SysctlFile != "" {
+	if params.BuildCfg.SysctlFile != "" {
 		return details, fmt.Errorf("sysctl file is not supported for android cuttlefish images")
 	}
 
@@ -94,7 +94,7 @@ func (c cuttlefish) build(params Params) (ImageDetails, error) {
 		return details, fmt.Errorf("failed to clean before kernel build: %w", err)
 	}
 	// Default to build.sh if compiler is not specified.
-	if params.Compiler == "bazel" {
+	if params.BuildCfg.CompilerBin == "bazel" {
 		if err := c.runBazel(params.KernelDir); err != nil {
 			return details, fmt.Errorf("failed to build kernel: %w", err)
 		}
@@ -165,9 +165,9 @@ func (c cuttlefish) build(params Params) (ImageDetails, error) {
 	return details, nil
 }
 
-func (c cuttlefish) clean(kernelDir, targetArch string) error {
-	if err := osutil.RemoveAll(filepath.Join(kernelDir, "out")); err != nil {
+func (c cuttlefish) clean(params Params) error {
+	if err := osutil.RemoveAll(filepath.Join(params.KernelDir, "out")); err != nil {
 		return err
 	}
-	return osutil.RemoveAll(filepath.Join(kernelDir, "dist"))
+	return osutil.RemoveAll(filepath.Join(params.KernelDir, "dist"))
 }

pkg/build/darwin.go

@@ -14,7 +14,7 @@ func (ctx darwin) build(params Params) (ImageDetails, error) {
 	return ImageDetails{}, fmt.Errorf("pkg/build: darwin.build not implemented")
 }
 
-func (ctx darwin) clean(kernelDir, targetArch string) error {
+func (ctx darwin) clean(params Params) error {
 	// TODO(HerrSpace): Implement this.
 	return fmt.Errorf("pkg/build: darwin.build not implemented")
 }

pkg/build/freebsd.go

@@ -20,7 +20,7 @@ func (ctx freebsd) build(params Params) (ImageDetails, error) {
 	confDir := fmt.Sprintf("%v/sys/%v/conf/", params.KernelDir, params.TargetArch)
 	confFile := "SYZKALLER"
 
-	config := params.Config
+	config := params.BuildCfg.KernelConfig
 	if config == nil {
 		config = []byte(`
 include "./GENERIC"
@@ -45,11 +45,11 @@ options 	DIAGNOSTIC
 		return ImageDetails{}, err
 	}
 	objPrefix := filepath.Join(params.KernelDir, "obj")
-	output, err := ctx.make(params.KernelDir, objPrefix, params.BuildCPUs, "kernel-toolchain")
+	output, err := ctx.make(params.KernelDir, objPrefix, params.BuildCfg.BuildCPUs, "kernel-toolchain")
 	if err != nil {
 		return ImageDetails{}, err
 	}
-	if _, err := ctx.make(params.KernelDir, objPrefix, params.BuildCPUs, "buildkernel",
+	if _, err := ctx.make(params.KernelDir, objPrefix, params.BuildCfg.BuildCPUs, "buildkernel",
 		"WITH_EXTRA_TCP_STACKS=", fmt.Sprintf("KERNCONF=%v", confFile)); err != nil {
 		// The kernel-toolchain make target has to be built separately
 		// because FreeBSD's build doesn't correctly order the two
@@ -61,8 +61,8 @@ options 	DIAGNOSTIC
 	kernelObjDir := filepath.Join(objPrefix, params.KernelDir,
 		fmt.Sprintf("%v.%v", params.TargetArch, params.TargetArch), "sys", confFile)
 	for _, s := range []struct{ dir, src, dst string }{
-		{params.UserspaceDir, "image", "image"},
-		{params.UserspaceDir, "key", "key"},
+		{params.BuildCfg.UserspaceDir, "image", "image"},
+		{params.BuildCfg.UserspaceDir, "key", "key"},
 		{kernelObjDir, "kernel.full", "obj/kernel.full"},
 	} {
 		fullSrc := filepath.Join(s.dir, s.src)
@@ -119,9 +119,9 @@ sudo mdconfig -d -u ${md#md}
 	return ImageDetails{}, nil
 }
 
-func (ctx freebsd) clean(kernelDir, targetArch string) error {
-	objPrefix := filepath.Join(kernelDir, "obj")
-	_, err := ctx.make(kernelDir, objPrefix, runtime.NumCPU(), "cleanworld")
+func (ctx freebsd) clean(params Params) error {
+	objPrefix := filepath.Join(params.KernelDir, "obj")
+	_, err := ctx.make(params.KernelDir, objPrefix, runtime.NumCPU(), "cleanworld")
 	return err
 }
 

pkg/build/fuchsia.go

@@ -94,7 +94,7 @@ func (fu fuchsia) build(params Params) (ImageDetails, error) {
 	return ImageDetails{}, nil
 }
 
-func (fu fuchsia) clean(kernelDir, targetArch string) error {
+func (fu fuchsia) clean(params Params) error {
 	// We always do clean build because incremental build is frequently broken.
 	// So no need to clean separately.
 	return nil

pkg/build/gvisor.go

@@ -20,14 +20,14 @@ type gvisor struct{}
 var bazelTargetPath = regexp.MustCompile(`(?sm:.*^)\s*Outputs: \[(.*)\](?sm:$.*)`)
 
 func (gvisor gvisor) build(params Params) (ImageDetails, error) {
-	if params.Compiler == "" {
-		params.Compiler = "bazel"
+	if params.BuildCfg.CompilerBin == "" {
+		params.BuildCfg.CompilerBin = "bazel"
 	}
 
 	// Bring down bazel daemon right away. We don't need it running and consuming memory.
-	defer osutil.RunCmd(10*time.Minute, params.KernelDir, params.Compiler, "shutdown")
+	defer osutil.RunCmd(10*time.Minute, params.KernelDir, params.BuildCfg.CompilerBin, "shutdown")
 
-	config, err := parseGVisorConfig(params.Config)
+	config, err := parseGVisorConfig(params.BuildCfg.KernelConfig)
 	if err != nil {
 		return ImageDetails{}, fmt.Errorf("cannot parse gVisor configuration: %w", err)
 	}
@@ -69,15 +69,15 @@ func (gvisor gvisor) build(params Params) (ImageDetails, error) {
 	// The 1 hour timeout is quite high. But we've seen false positives with 20 mins
 	// on the first build after bazel/deps update. Also other gvisor instances running
 	// on the same machine contribute to longer build times.
-	if _, err := osutil.RunCmd(60*time.Minute, params.KernelDir, params.Compiler, buildArgs...); err != nil {
+	if _, err := osutil.RunCmd(60*time.Minute, params.KernelDir, params.BuildCfg.CompilerBin, buildArgs...); err != nil {
 		return ImageDetails{}, err
 	}
 
 	// Find out a path to the runsc binary.
 	aqueryArgs := append([]string{"aquery"}, args...)
 	aqueryArgs = append(aqueryArgs, fmt.Sprintf("mnemonic(\"GoLink\", %s)", target))
 	log.Logf(0, "bazel: %v", aqueryArgs)
-	out, err := osutil.RunCmd(10*time.Minute, params.KernelDir, params.Compiler, aqueryArgs...)
+	out, err := osutil.RunCmd(10*time.Minute, params.KernelDir, params.BuildCfg.CompilerBin, aqueryArgs...)
 	if err != nil {
 		return ImageDetails{}, err
 	}
@@ -95,7 +95,7 @@ func (gvisor gvisor) build(params Params) (ImageDetails, error) {
 	return ImageDetails{}, osutil.CopyFile(outBinary, filepath.Join(params.OutputDir, "obj", sysTarget.KernelObject))
 }
 
-func (gvisor) clean(kernelDir, targetArch string) error {
+func (gvisor) clean(params Params) error {
 	// Let's assume that bazel always properly handles build without cleaning (until proven otherwise).
 	return nil
 }