pkg/runtest: check arch requirement early

Need to check arch requirement early as some programs
may fail to deserialize on some arches due to missing syscalls.
See discussion on #2380.
Also support negative arch requirements (-arch=amd64).

4 files changed, 99 insertions, 89 deletions

diff --git a/pkg/csource/csource_test.go b/pkg/csource/csource_test.go
index 951f9f09a..b7b2df486 100644
--- a/pkg/csource/csource_test.go
+++ b/pkg/csource/csource_test.go
@@ -4,12 +4,9 @@
 package csource
 
 import (
-	"bytes"
 	"fmt"
-	"io/ioutil"
 	"math/rand"
 	"os"
-	"path/filepath"
 	"regexp"
 	"runtime"
 	"strings"
@@ -17,7 +14,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/google/syzkaller/pkg/osutil"
 	"github.com/google/syzkaller/prog"
 	_ "github.com/google/syzkaller/sys"
 	"github.com/google/syzkaller/sys/targets"
@@ -47,18 +43,6 @@ func TestGenerate(t *testing.T) {
 	}
 }
 
-// This is the main configuration used by executor, so we want to test it as well.
-var executorOpts = Options{
-	Threaded:  true,
-	Collide:   true,
-	Repeat:    true,
-	Procs:     2,
-	Slowdown:  1,
-	Sandbox:   "none",
-	Repro:     true,
-	UseTmpDir: true,
-}
-
 func testTarget(t *testing.T, target *prog.Target, full bool) {
 	seed := time.Now().UnixNano()
 	if os.Getenv("CI") != "" {
@@ -78,7 +62,7 @@ func testTarget(t *testing.T, target *prog.Target, full bool) {
 	if !full || testing.Short() {
 		p.Calls = append(p.Calls, syzProg.Calls...)
 		opts = allOptionsSingle(target.OS)
-		opts = append(opts, executorOpts)
+		opts = append(opts, ExecutorOpts)
 	} else {
 		minimized, _ := prog.Minimize(syzProg, -1, false, func(p *prog.Prog, call int) bool {
 			return len(p.Calls) == len(syzProg.Calls)
@@ -127,51 +111,6 @@ func testOne(t *testing.T, p *prog.Prog, opts Options) {
 	defer os.Remove(bin)
 }
 
-func TestSysTests(t *testing.T) {
-	t.Parallel()
-	for _, target := range prog.AllTargets() {
-		target := target
-		sysTarget := targets.Get(target.OS, target.Arch)
-		if runtime.GOOS != sysTarget.BuildOS {
-			continue // we need at least preprocessor binary to generate sources
-		}
-		t.Run(target.OS+"/"+target.Arch, func(t *testing.T) {
-			t.Parallel()
-			dir := filepath.Join("..", "..", "sys", target.OS, targets.TestOS)
-			if !osutil.IsExist(dir) {
-				return
-			}
-			files, err := ioutil.ReadDir(dir)
-			if err != nil {
-				t.Fatalf("failed to read %v: %v", dir, err)
-			}
-			for _, finfo := range files {
-				file := filepath.Join(dir, finfo.Name())
-				if strings.HasSuffix(file, "~") || strings.HasSuffix(file, ".swp") {
-					continue
-				}
-				data, err := ioutil.ReadFile(file)
-				if err != nil {
-					t.Fatalf("failed to read %v: %v", file, err)
-				}
-				// syz_mount_image tests are very large and this test takes too long.
-				// syz-imagegen that generates does some of this testing (Deserialize/SerializeForExec).
-				if bytes.Contains(data, []byte("# requires: manual")) {
-					continue
-				}
-				p, err := target.Deserialize(data, prog.Strict)
-				if err != nil {
-					t.Fatalf("failed to parse program %v: %v", file, err)
-				}
-				_, err = Write(p, executorOpts)
-				if err != nil {
-					t.Fatalf("failed to generate C source for %v: %v", file, err)
-				}
-			}
-		})
-	}
-}
-
 func TestExecutorMacros(t *testing.T) {
 	// Ensure that executor does not mis-spell any of the SYZ_* macros.
 	target, _ := prog.GetTarget(targets.TestOS, targets.TestArch64)
diff --git a/pkg/csource/options.go b/pkg/csource/options.go
index 3c1790483..df8811559 100644
--- a/pkg/csource/options.go
+++ b/pkg/csource/options.go
@@ -310,3 +310,15 @@ func PrintAvailableFeaturesFlags() {
 		fmt.Printf("  %s - %s\n", name, features[name].Description)
 	}
 }
+
+// This is the main configuration used by executor, only for testing.
+var ExecutorOpts = Options{
+	Threaded:  true,
+	Collide:   true,
+	Repeat:    true,
+	Procs:     2,
+	Slowdown:  1,
+	Sandbox:   "none",
+	Repro:     true,
+	UseTmpDir: true,
+}
diff --git a/pkg/runtest/run.go b/pkg/runtest/run.go
index 4d81e2b60..437970768 100644
--- a/pkg/runtest/run.go
+++ b/pkg/runtest/run.go
@@ -192,10 +192,13 @@ func progFileList(dir, filter string) ([]string, error) {
 }
 
 func (ctx *Context) generateFile(progs chan *RunRequest, sandboxes []string, cover []bool, filename string) error {
-	p, requires, results, err := ctx.parseProg(filename)
+	p, requires, results, err := parseProg(ctx.Target, ctx.Dir, filename)
 	if err != nil {
 		return err
 	}
+	if p == nil {
+		return nil
+	}
 	sysTarget := targets.Get(ctx.Target.OS, ctx.Target.Arch)
 nextSandbox:
 	for _, sandbox := range sandboxes {
@@ -210,10 +213,9 @@ nextSandbox:
 			}
 		}
 		properties := map[string]bool{
-			"manual":                  ctx.Tests != "", // "manual" tests run only if selected by the filter explicitly.
-			"arch=" + ctx.Target.Arch: true,
-			"sandbox=" + sandbox:      true,
-			"littleendian":            ctx.Target.LittleEndian,
+			"manual":             ctx.Tests != "", // "manual" tests run only if selected by the filter explicitly.
+			"sandbox=" + sandbox: true,
+			"littleendian":       ctx.Target.LittleEndian,
 		}
 		for _, threaded := range []bool{false, true} {
 			name := name
@@ -272,34 +274,21 @@ nextSandbox:
 	return nil
 }
 
-func (ctx *Context) parseProg(filename string) (*prog.Prog, map[string]bool, *ipc.ProgInfo, error) {
-	return parseProg(ctx.Target, ctx.Dir, filename)
-}
-
 func parseProg(target *prog.Target, dir, filename string) (*prog.Prog, map[string]bool, *ipc.ProgInfo, error) {
 	data, err := ioutil.ReadFile(filepath.Join(dir, filename))
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("failed to read %v: %v", filename, err)
 	}
+	requires := parseRequires(data)
+	// Need to check arch requirement early as some programs
+	// may fail to deserialize on some arches due to missing syscalls.
+	if !checkArch(requires, target.Arch) {
+		return nil, nil, nil, nil
+	}
 	p, err := target.Deserialize(data, prog.Strict)
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("failed to deserialize %v: %v", filename, err)
 	}
-	requires := make(map[string]bool)
-	for _, comment := range p.Comments {
-		const prefix = "requires:"
-		if !strings.HasPrefix(comment, prefix) {
-			continue
-		}
-		for _, req := range strings.Fields(comment[len(prefix):]) {
-			positive := true
-			if req[0] == '-' {
-				positive = false
-				req = req[1:]
-			}
-			requires[req] = positive
-		}
-	}
 	errnos := map[string]int{
 		"":           0,
 		"EPERM":      1,
@@ -339,6 +328,37 @@ func parseProg(target *prog.Target, dir, filename string) (*prog.Prog, map[strin
 	return p, requires, info, nil
 }
 
+func parseRequires(data []byte) map[string]bool {
+	requires := make(map[string]bool)
+	for s := bufio.NewScanner(bytes.NewReader(data)); s.Scan(); {
+		const prefix = "# requires:"
+		line := s.Text()
+		if !strings.HasPrefix(line, prefix) {
+			continue
+		}
+		for _, req := range strings.Fields(line[len(prefix):]) {
+			positive := true
+			if req[0] == '-' {
+				positive = false
+				req = req[1:]
+			}
+			requires[req] = positive
+		}
+	}
+	return requires
+}
+
+func checkArch(requires map[string]bool, arch string) bool {
+	for req, positive := range requires {
+		const prefix = "arch="
+		if strings.HasPrefix(req, prefix) &&
+			arch != req[len(prefix):] == positive {
+			return false
+		}
+	}
+	return true
+}
+
 func (ctx *Context) produceTest(progs chan *RunRequest, req *RunRequest, name string,
 	properties, requires map[string]bool, results *ipc.ProgInfo) {
 	req.name = name
diff --git a/pkg/runtest/run_test.go b/pkg/runtest/run_test.go
index d72f5940e..d02cc24e6 100644
--- a/pkg/runtest/run_test.go
+++ b/pkg/runtest/run_test.go
@@ -99,8 +99,9 @@ func test(t *testing.T, sysTarget *targets.Target) {
 }
 
 func TestParsing(t *testing.T) {
+	t.Parallel()
 	for OS, arches := range targets.List {
-		dir := filepath.Join("..", "..", "sys", OS, targets.TestOS)
+		dir := filepath.Join("..", "..", "sys", OS, "test")
 		if !osutil.IsExist(dir) {
 			continue
 		}
@@ -113,13 +114,51 @@ func TestParsing(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
+			sysTarget := targets.Get(target.OS, target.Arch)
 			t.Run(fmt.Sprintf("%v/%v", target.OS, target.Arch), func(t *testing.T) {
+				t.Parallel()
 				for _, file := range files {
-					if _, _, _, err := parseProg(target, dir, file); err != nil {
-						t.Errorf("failed to parse %v/%v for %v: %v", dir, file, arch, err)
+					p, requires, _, err := parseProg(target, dir, file)
+					if err != nil {
+						t.Errorf("failed to parse %v: %v", file, err)
+					}
+					if p == nil {
+						continue
+					}
+					if runtime.GOOS != sysTarget.BuildOS {
+						continue // we need at least preprocessor binary to generate sources
+					}
+					// syz_mount_image tests are very large and this test takes too long.
+					// syz-imagegen that generates does some of this testing (Deserialize/SerializeForExec).
+					if requires["manual"] {
+						continue
+					}
+					if _, err = csource.Write(p, csource.ExecutorOpts); err != nil {
+						t.Errorf("failed to generate C source for %v: %v", file, err)
 					}
 				}
 			})
 		}
 	}
 }
+
+func TestRequires(t *testing.T) {
+	{
+		requires := parseRequires([]byte("# requires: manual arch=amd64"))
+		if !checkArch(requires, "amd64") {
+			t.Fatalf("amd64 does not pass check")
+		}
+		if checkArch(requires, "riscv64") {
+			t.Fatalf("riscv64 passes check")
+		}
+	}
+	{
+		requires := parseRequires([]byte("# requires: -arch=arm64 manual -arch=riscv64"))
+		if !checkArch(requires, "amd64") {
+			t.Fatalf("amd64 does not pass check")
+		}
+		if checkArch(requires, "riscv64") {
+			t.Fatalf("riscv64 passes check")
+		}
+	}
+}