Skip to content

Commit

Permalink
pkg/build: use the build environment in clean() calls
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
FlorentRevest committed Oct 2, 2024
1 parent abce7a1 commit 7167531
Show file tree
Hide file tree
Showing 16 changed files with 87 additions and 60 deletions.
26 changes: 18 additions & 8 deletions pkg/bisect/bisect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 := &instance.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)
Expand Down Expand Up @@ -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 := &instance.BuildKernelConfig{
CompilerBin: bisectEnv.Compiler,
MakeBin: env.cfg.Make,
LinkerBin: env.cfg.Linker,
Expand All @@ -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)
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/bisect/bisect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ func (env *testEnv) BuildSyzkaller(repo, commit string) (string, error) {
return "", nil
}

func (env *testEnv) CleanKernel(buildCfg *instance.BuildKernelConfig) error {
return nil
}

func (env *testEnv) BuildKernel(buildCfg *instance.BuildKernelConfig) (string, build.ImageDetails, error) {
commit := env.headCommit()
configHash := hash.String(buildCfg.KernelConfig)
Expand Down
6 changes: 3 additions & 3 deletions pkg/build/android.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions pkg/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/google/syzkaller/sys/targets"
)

// Params is input arguments for the Image function.
// Params is input arguments for the Image and Clean functions.
type Params struct {
TargetOS string
TargetArch string
Expand Down Expand Up @@ -113,12 +113,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 {
Expand Down Expand Up @@ -146,7 +146,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) {
Expand Down
6 changes: 3 additions & 3 deletions pkg/build/cuttlefish.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
2 changes: 1 addition & 1 deletion pkg/build/darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
6 changes: 3 additions & 3 deletions pkg/build/freebsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/build/fuchsia.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/gvisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/build/linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"path"
"path/filepath"
"regexp"
"runtime"
"time"

"github.com/google/syzkaller/pkg/debugtracer"
Expand Down Expand Up @@ -141,8 +140,8 @@ func (linux) createImage(params Params, kernelPath string) error {
return nil
}

func (linux) clean(kernelDir, targetArch string) error {
return runMakeImpl(targetArch, "", "", "", "", kernelDir, runtime.NumCPU(), []string{"distclean"})
func (linux) clean(params Params) error {
return runMake(params, "distclean")
}

func (linux) writeFile(file string, data []byte) error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/build/netbsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ func (ctx netbsd) build(params Params) (ImageDetails, error) {
filepath.Join(compileDir, "netbsd"))
}

func (ctx netbsd) clean(kernelDir, targetArch string) error {
_, err := osutil.RunCmd(10*time.Minute, kernelDir, "./build.sh", "-m", targetArch,
func (ctx netbsd) clean(params Params) error {
_, err := osutil.RunCmd(10*time.Minute, params.KernelDir, "./build.sh", "-m", params.TargetArch,
"-U", "-j"+strconv.Itoa(runtime.NumCPU()), "cleandir")
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/openbsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (ctx openbsd) build(params Params) (ImageDetails, error) {
return ImageDetails{}, nil
}

func (ctx openbsd) clean(kernelDir, targetArch string) error {
func (ctx openbsd) clean(params Params) error {
// Building clean is fast enough and incremental builds in face of
// changing config files don't work. Instead of optimizing for the
// case where humans have to think, let's bludgeon it with a
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ func (tb test) build(params Params) (ImageDetails, error) {
return ImageDetails{}, nil
}

func (tb test) clean(string, string) error {
func (tb test) clean(Params) error {
return nil
}
35 changes: 24 additions & 11 deletions pkg/instance/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (

type Env interface {
BuildSyzkaller(string, string) (string, error)
CleanKernel(*BuildKernelConfig) error
BuildKernel(*BuildKernelConfig) (string, build.ImageDetails, error)
Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]EnvTestResult, error)
}
Expand Down Expand Up @@ -135,19 +136,13 @@ func (env *env) BuildSyzkaller(repoURL, commit string) (string, error) {
return buildLog, nil
}

func (env *env) BuildKernel(buildCfg *BuildKernelConfig) (
string, build.ImageDetails, error) {
if env.buildSem != nil {
env.buildSem.Wait()
defer env.buildSem.Signal()
}
imageDir := filepath.Join(env.cfg.Workdir, "image")
params := build.Params{
func (env *env) buildParamsFromCfg(buildCfg *BuildKernelConfig) build.Params {
return build.Params{
TargetOS: env.cfg.TargetOS,
TargetArch: env.cfg.TargetVMArch,
VMType: env.cfg.Type,
KernelDir: env.cfg.KernelSrc,
OutputDir: imageDir,
OutputDir: filepath.Join(env.cfg.Workdir, "image"),
Make: buildCfg.MakeBin,
Compiler: buildCfg.CompilerBin,
Linker: buildCfg.LinkerBin,
Expand All @@ -158,20 +153,38 @@ func (env *env) BuildKernel(buildCfg *BuildKernelConfig) (
Config: buildCfg.KernelConfig,
BuildCPUs: buildCfg.BuildCPUs,
}
}

func (env *env) BuildKernel(buildCfg *BuildKernelConfig) (
string, build.ImageDetails, error) {
if env.buildSem != nil {
env.buildSem.Wait()
defer env.buildSem.Signal()
}
params := env.buildParamsFromCfg(buildCfg)
details, err := build.Image(params)
if err != nil {
return "", details, err
}
if err := SetConfigImage(env.cfg, imageDir, true); err != nil {
if err := SetConfigImage(env.cfg, params.OutputDir, true); err != nil {
return "", details, err
}
kernelConfigFile := filepath.Join(imageDir, "kernel.config")
kernelConfigFile := filepath.Join(params.OutputDir, "kernel.config")
if !osutil.IsExist(kernelConfigFile) {
kernelConfigFile = ""
}
return kernelConfigFile, details, nil
}

func (env *env) CleanKernel(buildCfg *BuildKernelConfig) error {
if env.buildSem != nil {
env.buildSem.Wait()
defer env.buildSem.Signal()
}
params := env.buildParamsFromCfg(buildCfg)
return build.Clean(params)
}

func SetConfigImage(cfg *mgrconfig.Config, imageDir string, reliable bool) error {
cfg.KernelObj = filepath.Join(imageDir, "obj")
cfg.Image = filepath.Join(imageDir, "image")
Expand Down
23 changes: 12 additions & 11 deletions syz-ci/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,17 @@ func (jp *JobProcessor) testPatch(job *Job, mgrcfg *mgrconfig.Config) error {
resp.Build.KernelCommitTitle = kernelCommit.Title
resp.Build.KernelCommitDate = kernelCommit.CommitDate

if err := build.Clean(mgrcfg.TargetOS, mgrcfg.TargetVMArch, mgrcfg.Type, mgrcfg.KernelSrc); err != nil {
buildCfg := &instance.BuildKernelConfig{
CompilerBin: mgr.mgrcfg.Compiler,
MakeBin: mgr.mgrcfg.Make,
LinkerBin: mgr.mgrcfg.Linker,
CcacheBin: mgr.mgrcfg.Ccache,
UserspaceDir: mgr.mgrcfg.Userspace,
CmdlineFile: mgr.mgrcfg.KernelCmdline,
SysctlFile: mgr.mgrcfg.KernelSysctl,
KernelConfig: req.KernelConfig,
}
if err := env.CleanKernel(buildCfg); err != nil {
return fmt.Errorf("kernel clean failed: %w", err)
}
if len(req.Patch) != 0 {
Expand All @@ -643,16 +653,7 @@ func (jp *JobProcessor) testPatch(job *Job, mgrcfg *mgrconfig.Config) error {
[]byte("# CONFIG_DEBUG_INFO_BTF is not set"), -1)

log.Logf(0, "job: building kernel...")
kernelConfig, details, err := env.BuildKernel(&instance.BuildKernelConfig{
CompilerBin: mgr.mgrcfg.Compiler,
MakeBin: mgr.mgrcfg.Make,
LinkerBin: mgr.mgrcfg.Linker,
CcacheBin: mgr.mgrcfg.Ccache,
UserspaceDir: mgr.mgrcfg.Userspace,
CmdlineFile: mgr.mgrcfg.KernelCmdline,
SysctlFile: mgr.mgrcfg.KernelSysctl,
KernelConfig: req.KernelConfig,
})
kernelConfig, details, err := env.BuildKernel(buildCfg)
resp.Build.CompilerID = details.CompilerID
if err != nil {
return err
Expand Down
12 changes: 6 additions & 6 deletions tools/syz-testbuild/testbuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"os"
"runtime"

"github.com/google/syzkaller/pkg/build"
"github.com/google/syzkaller/pkg/instance"
"github.com/google/syzkaller/pkg/mgrconfig"
"github.com/google/syzkaller/pkg/osutil"
Expand Down Expand Up @@ -131,10 +130,7 @@ func test(repo vcs.Repo, bisecter vcs.Bisecter, kernelConfig []byte, env instanc
tool.Fail(err)
}
log.Printf("testing: %v %v using %v", com.Hash, com.Title, bisectEnv.Compiler)
if err := build.Clean(*flagOS, *flagArch, vmType, *flagKernelSrc); err != nil {
tool.Fail(err)
}
_, _, err = env.BuildKernel(&instance.BuildKernelConfig{
buildCfg := &instance.BuildKernelConfig{
CompilerBin: bisectEnv.Compiler,
MakeBin: makeBin,
LinkerBin: linker,
Expand All @@ -143,7 +139,11 @@ func test(repo vcs.Repo, bisecter vcs.Bisecter, kernelConfig []byte, env instanc
CmdlineFile: *flagKernelCmdline,
SysctlFile: *flagKernelSysctl,
KernelConfig: bisectEnv.KernelConfig,
})
}
if err := env.CleanKernel(buildCfg); err != nil {
tool.Fail(err)
}
_, _, err = env.BuildKernel(buildCfg)
if err != nil {
var verr *osutil.VerboseError
if errors.As(err, &verr) {
Expand Down

0 comments on commit 7167531

Please sign in to comment.