Skip to content

Commit

Permalink
sys/targets: add OS/Arch name consts
Browse files Browse the repository at this point in the history
We use strings to identify OS/Arch.
These strings are duplicated throughout the code base massively.
golangci-lint points to possiblity of typos and duplication.
We already had to define these names in pkg/csource
and disable checking for prog package. A future change triggers
such warnings in another package.

Add OS/Arch name consts to sys/targets so that they can be used
to refer to OS/Arch. Use the consts everywhere.
  • Loading branch information
dvyukov committed Oct 26, 2020
1 parent d46bc75 commit e6e35db
Show file tree
Hide file tree
Showing 63 changed files with 323 additions and 258 deletions.
3 changes: 2 additions & 1 deletion dashboard/app/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/google/syzkaller/dashboard/dashapi"
"github.com/google/syzkaller/pkg/email"
"github.com/google/syzkaller/pkg/hash"
"github.com/google/syzkaller/sys/targets"
"golang.org/x/net/context"
"google.golang.org/appengine"
db "google.golang.org/appengine/datastore"
Expand Down Expand Up @@ -784,7 +785,7 @@ func saveCrash(c context.Context, ns string, req *dashapi.Crash, bugKey *db.Key,
} else if len(req.ReproSyz) != 0 {
prio += 2e12
}
if build.Arch == "amd64" {
if build.Arch == targets.AMD64 {
prio += 1e3
}
crash := &Crash{
Expand Down
7 changes: 4 additions & 3 deletions dashboard/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"time"

"github.com/google/syzkaller/dashboard/dashapi"
"github.com/google/syzkaller/sys/targets"
"google.golang.org/appengine/user"
)

Expand Down Expand Up @@ -287,9 +288,9 @@ func testBuild(id int) *dashapi.Build {
return &dashapi.Build{
Manager: fmt.Sprintf("manager%v", id),
ID: fmt.Sprintf("build%v", id),
OS: "linux",
Arch: "amd64",
VMArch: "amd64",
OS: targets.Linux,
Arch: targets.AMD64,
VMArch: targets.AMD64,
SyzkallerCommit: fmt.Sprintf("syzkaller_commit%v", id),
CompilerID: fmt.Sprintf("compiler%v", id),
KernelRepo: fmt.Sprintf("repo%v", id),
Expand Down
3 changes: 2 additions & 1 deletion dashboard/app/email_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/google/syzkaller/dashboard/dashapi"
"github.com/google/syzkaller/pkg/email"
"github.com/google/syzkaller/sys/targets"
)

// nolint: funlen
Expand Down Expand Up @@ -121,7 +122,7 @@ For more options, visit https://groups.google.com/d/optout.

// Now report syz reproducer and check updated email.
build2 := testBuild(10)
build2.Arch = "386"
build2.Arch = targets.I386
build2.KernelRepo = testConfig.Namespaces["test2"].Repos[0].URL
build2.KernelBranch = testConfig.Namespaces["test2"].Repos[0].Branch
build2.KernelCommitTitle = "a really long title, longer than 80 chars, really long-long-long-long-long-long title"
Expand Down
5 changes: 3 additions & 2 deletions dashboard/app/reporting.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/google/syzkaller/dashboard/dashapi"
"github.com/google/syzkaller/pkg/email"
"github.com/google/syzkaller/pkg/html"
"github.com/google/syzkaller/sys/targets"
"golang.org/x/net/context"
db "google.golang.org/appengine/datastore"
"google.golang.org/appengine/log"
Expand Down Expand Up @@ -1175,9 +1176,9 @@ func (a bugReportSorter) Less(i, j int) bool {
// Currently Linux-specific.
func kernelArch(arch string) string {
switch arch {
case "386":
case targets.I386:
return "i386"
case "amd64":
case targets.AMD64:
return "" // this is kinda the default, so we don't notify about it
default:
return arch
Expand Down
13 changes: 7 additions & 6 deletions dashboard/app/reporting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/google/syzkaller/dashboard/dashapi"
"github.com/google/syzkaller/sys/targets"
)

func TestReportBug(t *testing.T) {
Expand Down Expand Up @@ -45,9 +46,9 @@ func TestReportBug(t *testing.T) {
Namespace: "test1",
Config: []byte(`{"Index":1}`),
ID: rep.ID,
OS: "linux",
Arch: "amd64",
VMArch: "amd64",
OS: targets.Linux,
Arch: targets.AMD64,
VMArch: targets.AMD64,
First: true,
Moderation: true,
Title: "title1",
Expand Down Expand Up @@ -208,9 +209,9 @@ func TestInvalidBug(t *testing.T) {
Namespace: "test1",
Config: []byte(`{"Index":1}`),
ID: rep.ID,
OS: "linux",
Arch: "amd64",
VMArch: "amd64",
OS: targets.Linux,
Arch: targets.AMD64,
VMArch: targets.AMD64,
First: true,
Moderation: true,
Title: "title1 (2)",
Expand Down
4 changes: 3 additions & 1 deletion pkg/ast/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@ import (
"reflect"
"strings"
"testing"

"github.com/google/syzkaller/sys/targets"
)

func TestParseAll(t *testing.T) {
files, err := filepath.Glob(filepath.Join("..", "..", "sys", "linux", "*.txt"))
files, err := filepath.Glob(filepath.Join("..", "..", "sys", targets.Linux, "*.txt"))
if err != nil || len(files) == 0 {
t.Fatalf("failed to read sys dir: %v", err)
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/bisect/bisect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/google/syzkaller/pkg/mgrconfig"
"github.com/google/syzkaller/pkg/report"
"github.com/google/syzkaller/pkg/vcs"
"github.com/google/syzkaller/sys/targets"
)

// testEnv will implement instance.BuilderTester. This allows us to
Expand Down Expand Up @@ -106,7 +107,7 @@ func createTestRepo(t *testing.T) string {
}

func runBisection(t *testing.T, baseDir string, test BisectionTest) (*Result, error) {
r, err := vcs.NewRepo("test", "64", baseDir, vcs.OptPrecious)
r, err := vcs.NewRepo(targets.TestOS, targets.TestArch64, baseDir, vcs.OptPrecious)
if err != nil {
t.Fatal(err)
}
Expand All @@ -120,8 +121,8 @@ func runBisection(t *testing.T, baseDir string, test BisectionTest) (*Result, er
Fix: test.fix,
Trace: trace,
Manager: mgrconfig.Config{
TargetOS: "test",
TargetVMArch: "64",
TargetOS: targets.TestOS,
TargetVMArch: targets.TestArch64,
Type: "qemu",
KernelSrc: baseDir,
},
Expand Down
25 changes: 13 additions & 12 deletions pkg/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/google/syzkaller/pkg/osutil"
"github.com/google/syzkaller/pkg/report"
"github.com/google/syzkaller/pkg/vcs"
"github.com/google/syzkaller/sys/targets"
)

// Params is input arguments for the Image function.
Expand Down Expand Up @@ -118,17 +119,17 @@ func getBuilder(targetOS, targetArch, vmType string) (builder, error) {
vms []string
b builder
}{
{"linux", "amd64", []string{"gvisor"}, gvisor{}},
{"linux", "amd64", []string{"gce", "qemu"}, linux{}},
{"linux", "ppc64le", []string{"qemu"}, linux{}},
{"linux", "s390x", []string{"qemu"}, linux{}},
{"fuchsia", "amd64", []string{"qemu"}, fuchsia{}},
{"fuchsia", "arm64", []string{"qemu"}, fuchsia{}},
{"akaros", "amd64", []string{"qemu"}, akaros{}},
{"openbsd", "amd64", []string{"gce", "vmm"}, openbsd{}},
{"netbsd", "amd64", []string{"gce", "qemu"}, netbsd{}},
{"freebsd", "amd64", []string{"gce", "qemu"}, freebsd{}},
{"test", "64", []string{"qemu"}, test{}},
{targets.Linux, targets.AMD64, []string{"gvisor"}, gvisor{}},
{targets.Linux, targets.AMD64, []string{"gce", "qemu"}, linux{}},
{targets.Linux, targets.PPC64LE, []string{"qemu"}, linux{}},
{targets.Linux, targets.S390x, []string{"qemu"}, linux{}},
{targets.Fuchsia, targets.AMD64, []string{"qemu"}, fuchsia{}},
{targets.Fuchsia, targets.ARM64, []string{"qemu"}, fuchsia{}},
{targets.Akaros, targets.AMD64, []string{"qemu"}, akaros{}},
{targets.OpenBSD, targets.AMD64, []string{"gce", "vmm"}, openbsd{}},
{targets.NetBSD, targets.AMD64, []string{"gce", "qemu"}, netbsd{}},
{targets.FreeBSD, targets.AMD64, []string{"gce", "qemu"}, freebsd{}},
{targets.TestOS, targets.TestArch64, []string{"qemu"}, test{}},
}
for _, s := range supported {
if targetOS == s.OS && targetArch == s.arch {
Expand Down Expand Up @@ -193,7 +194,7 @@ func extractRootCause(err error, OS, kernelSrc string) error {
Output: verr.Output,
guiltyFile: file,
}
if file != "" && OS == "linux" {
if file != "" && OS == targets.Linux {
maintainers, err := report.GetLinuxMaintainers(kernelSrc, file)
if err != nil {
kernelErr.Output = append(kernelErr.Output, err.Error()...)
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/fuchsia.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (fu fuchsia) build(params *Params) error {
return err
}

sysTarget := targets.Get("fuchsia", params.TargetArch)
sysTarget := targets.Get(targets.Fuchsia, params.TargetArch)
if sysTarget == nil {
return fmt.Errorf("unsupported fuchsia arch %v", params.TargetArch)
}
Expand Down
11 changes: 6 additions & 5 deletions pkg/build/linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"time"

"github.com/google/syzkaller/pkg/osutil"
"github.com/google/syzkaller/sys/targets"
)

type linux struct{}
Expand Down Expand Up @@ -57,9 +58,9 @@ func (linux linux) buildKernel(params *Params) error {
// We build only zImage/bzImage as we currently don't use modules.
var target string
switch params.TargetArch {
case "386", "amd64", "s390x":
case targets.I386, targets.AMD64, targets.S390x:
target = "bzImage"
case "ppc64le":
case targets.PPC64LE:
target = "zImage"
}

Expand Down Expand Up @@ -110,11 +111,11 @@ func (linux) createImage(params *Params) error {

var kernelImage string
switch params.TargetArch {
case "386", "amd64":
case targets.I386, targets.AMD64:
kernelImage = "arch/x86/boot/bzImage"
case "ppc64le":
case targets.PPC64LE:
kernelImage = "arch/powerpc/boot/zImage.pseries"
case "s390x":
case targets.S390x:
kernelImage = "arch/s390/boot/bzImage"
}
kernelImagePath := filepath.Join(params.KernelDir, filepath.FromSlash(kernelImage))
Expand Down
3 changes: 2 additions & 1 deletion pkg/build/netbsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/google/syzkaller/pkg/mgrconfig"
"github.com/google/syzkaller/pkg/osutil"
"github.com/google/syzkaller/pkg/report"
"github.com/google/syzkaller/sys/targets"
"github.com/google/syzkaller/vm"
)

Expand Down Expand Up @@ -104,7 +105,7 @@ func (ctx netbsd) copyKernelToDisk(targetArch, vmType, outputDir, kernel string)
Image: filepath.Join(outputDir, "image"),
SSHKey: filepath.Join(outputDir, "key"),
SSHUser: "root",
TargetOS: "netbsd",
TargetOS: targets.NetBSD,
TargetArch: targetArch,
TargetVMArch: targetArch,
Type: "qemu",
Expand Down
10 changes: 5 additions & 5 deletions pkg/compiler/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ func TestData(t *testing.T) {
// E.g. if we failed to parse descriptions, we won't run type checking at all.
// Because of this we have one file per phase.
for _, name := range []string{"errors.txt", "errors2.txt", "errors3.txt", "warnings.txt", "all.txt"} {
for _, arch := range []string{"32_shmem", "64"} {
for _, arch := range []string{targets.TestArch32Shmem, targets.TestArch64} {
name, arch := name, arch
t.Run(fmt.Sprintf("%v/%v", name, arch), func(t *testing.T) {
t.Parallel()
target := targets.List["test"][arch]
target := targets.List[targets.TestOS][arch]
fileName := filepath.Join("testdata", name)
em := ast.NewErrorMatcher(t, fileName)
astDesc := ast.Parse(em.Data, name, em.ErrorHandler)
Expand Down Expand Up @@ -182,7 +182,7 @@ s2 {
if desc == nil {
t.Fatal("failed to parse")
}
p := Compile(desc, map[string]uint64{"SYS_foo": 1}, targets.List["test"]["64"], eh)
p := Compile(desc, map[string]uint64{"SYS_foo": 1}, targets.List[targets.TestOS][targets.TestArch64], eh)
if p == nil {
t.Fatal("failed to compile")
}
Expand All @@ -201,7 +201,7 @@ func TestCollectUnusedError(t *testing.T) {
t.Fatal("failed to parse")
}

_, err := CollectUnused(desc, targets.List["test"]["64"], nopErrorHandler)
_, err := CollectUnused(desc, targets.List[targets.TestOS][targets.TestArch64], nopErrorHandler)
if err == nil {
t.Fatal("CollectUnused should have failed but didn't")
}
Expand Down Expand Up @@ -252,7 +252,7 @@ func TestCollectUnused(t *testing.T) {
t.Fatalf("Test %d: failed to parse", i)
}

nodes, err := CollectUnused(desc, targets.List["test"]["64"], nil)
nodes, err := CollectUnused(desc, targets.List[targets.TestOS][targets.TestArch64], nil)
if err != nil {
t.Fatalf("Test %d: CollectUnused failed: %v", i, err)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/compiler/consts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestExtractConsts(t *testing.T) {
if desc == nil {
t.Fatalf("failed to parse input")
}
target := targets.List["linux"]["amd64"]
target := targets.List[targets.Linux][targets.AMD64]
fileInfo := ExtractConsts(desc, target, func(pos ast.Pos, msg string) {
t.Fatalf("%v: %v", pos, msg)
})
Expand Down Expand Up @@ -69,7 +69,7 @@ func TestConstErrors(t *testing.T) {
em.DumpErrors()
t.Fatalf("parsing failed")
}
target := targets.List["linux"]["amd64"]
target := targets.List[targets.Linux][targets.AMD64]
ExtractConsts(desc, target, em.ErrorHandler)
em.Check()
}
2 changes: 1 addition & 1 deletion pkg/compiler/fuzz.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ func Fuzz(data []byte) int {
}

var (
fuzzTarget = targets.Get("test", "64")
fuzzTarget = targets.Get(targets.TestOS, targets.TestArch64)
fuzzConsts = map[string]uint64{"A": 1, "B": 2, "C": 3, "SYS_A": 4, "SYS_B": 5, "SYS_C": 6}
)
Loading

0 comments on commit e6e35db

Please sign in to comment.