pkg/compiler: add tests for generation phase

Add errors3.txt with tests for errors that are produced during generation phase.
Refactor tests to reduce duplication.
Tidy struct/union size errors: better locations and make testable.

8 files changed, 74 insertions, 110 deletions

diff --git a/pkg/ast/parser_test.go b/pkg/ast/parser_test.go
index f1458e3e7..edbabf3d3 100644
--- a/pkg/ast/parser_test.go
+++ b/pkg/ast/parser_test.go
@@ -108,13 +108,13 @@ func TestErrors(t *testing.T) {
 			em := NewErrorMatcher(t, filepath.Join("testdata", name))
 			desc := Parse(em.Data, name, em.ErrorHandler)
 			if desc != nil && em.Count() != 0 {
-				em.DumpErrors(t)
+				em.DumpErrors()
 				t.Fatalf("parsing succeed, but got errors")
 			}
 			if desc == nil && em.Count() == 0 {
 				t.Fatalf("parsing failed, but got no errors")
 			}
-			em.Check(t)
+			em.Check()
 		})
 	}
 }
diff --git a/pkg/ast/test_util.go b/pkg/ast/test_util.go
index 3ffefcccb..741bed553 100644
--- a/pkg/ast/test_util.go
+++ b/pkg/ast/test_util.go
@@ -14,6 +14,7 @@ import (
 )
 
 type ErrorMatcher struct {
+	t      *testing.T
 	Data   []byte
 	expect []*errorDesc
 	got    []*errorDesc
@@ -56,6 +57,7 @@ func NewErrorMatcher(t *testing.T, file string) *ErrorMatcher {
 		t.Fatalf("failed to scan input file: %v", err)
 	}
 	return &ErrorMatcher{
+		t:      t,
 		Data:   stripped,
 		expect: errors,
 	}
@@ -79,7 +81,7 @@ func (em *ErrorMatcher) Count() int {
 	return len(em.got)
 }
 
-func (em *ErrorMatcher) Check(t *testing.T) {
+func (em *ErrorMatcher) Check() {
 nextErr:
 	for _, e := range em.got {
 		for _, want := range em.expect {
@@ -89,18 +91,18 @@ nextErr:
 			want.matched = true
 			continue nextErr
 		}
-		t.Errorf("unexpected error:\n%v:%v:%v: %v", e.file, e.line, e.col, e.text)
+		em.t.Errorf("unexpected error:\n%v:%v:%v: %v", e.file, e.line, e.col, e.text)
 	}
 	for _, want := range em.expect {
 		if want.matched {
 			continue
 		}
-		t.Errorf("unmatched error:\n%v:%v: %v", want.file, want.line, want.text)
+		em.t.Errorf("unmatched error:\n%v:%v: %v", want.file, want.line, want.text)
 	}
 }
 
-func (em *ErrorMatcher) DumpErrors(t *testing.T) {
+func (em *ErrorMatcher) DumpErrors() {
 	for _, e := range em.got {
-		t.Logf("%v:%v:%v: %v", e.file, e.line, e.col, e.text)
+		em.t.Logf("%v:%v:%v: %v", e.file, e.line, e.col, e.text)
 	}
 }
diff --git a/pkg/compiler/compiler_test.go b/pkg/compiler/compiler_test.go
index c6421b17f..c051702a2 100644
--- a/pkg/compiler/compiler_test.go
+++ b/pkg/compiler/compiler_test.go
@@ -51,47 +51,62 @@ func TestCompileAll(t *testing.T) {
 	}
 }
 
-func TestNoErrors(t *testing.T) {
+func TestData(t *testing.T) {
 	t.Parallel()
-	for _, name := range []string{"all.txt"} {
+	// Compile the canned descriptions in testdata and match expected errors.
+	// Errors are produced in batches in different compilation phases.
+	// If one phase produces errors, subsequent phases are not executed.
+	// 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"} {
 			name, arch := name, arch
 			t.Run(fmt.Sprintf("%v/%v", name, arch), func(t *testing.T) {
 				t.Parallel()
 				target := targets.List["test"][arch]
-				eh := func(pos ast.Pos, msg string) {
-					t.Logf("%v: %v", pos, msg)
-				}
 				fileName := filepath.Join("testdata", name)
-				data, err := ioutil.ReadFile(fileName)
-				if err != nil {
-					t.Fatal(err)
-				}
-				astDesc := ast.Parse(data, name, eh)
+				em := ast.NewErrorMatcher(t, fileName)
+				astDesc := ast.Parse(em.Data, name, em.ErrorHandler)
 				if astDesc == nil {
+					em.DumpErrors()
 					t.Fatalf("parsing failed")
 				}
-				formatted := ast.Format(astDesc)
-				if !bytes.Equal(data, formatted) {
-					if *flagUpdate {
-						ioutil.WriteFile(fileName, formatted, 0644)
-					}
-					t.Fatalf("description is not formatted")
+				constInfo := ExtractConsts(astDesc, target, em.ErrorHandler)
+				if name == "errors.txt" {
+					em.Check()
+					return
 				}
-				constInfo := ExtractConsts(astDesc, target, eh)
 				if constInfo == nil {
+					em.DumpErrors()
 					t.Fatalf("const extraction failed")
 				}
 				consts := map[string]uint64{
-					"C0": 0,
-					"C1": 1,
-					"C2": 2,
+					"SYS_foo": 1,
+					"C0":      0,
+					"C1":      1,
+					"C2":      2,
 				}
 				FabricateSyscallConsts(target, constInfo, consts)
-				desc := Compile(astDesc, consts, target, eh)
+				delete(consts, "SYS_unsupported")
+				desc := Compile(astDesc, consts, target, em.ErrorHandler)
+				if name == "errors2.txt" || name == "errors3.txt" {
+					em.Check()
+					return
+				}
 				if desc == nil {
+					em.DumpErrors()
 					t.Fatalf("compilation failed")
 				}
+				if name == "warnings.txt" {
+					em.Check()
+					return
+				}
+				if formatted := ast.Format(astDesc); !bytes.Equal(em.Data, formatted) {
+					if *flagUpdate {
+						ioutil.WriteFile(fileName, formatted, 0644)
+					}
+					t.Fatalf("description is not formatted")
+				}
 				if len(desc.Unsupported) != 0 {
 					t.Fatalf("something is unsupported:\n%+v", desc.Unsupported)
 				}
@@ -110,83 +125,6 @@ func TestNoErrors(t *testing.T) {
 	}
 }
 
-func TestErrors(t *testing.T) {
-	t.Parallel()
-	for _, arch := range []string{"32_shmem", "64"} {
-		target := targets.List["test"][arch]
-		t.Run(arch, func(t *testing.T) {
-			t.Parallel()
-			em := ast.NewErrorMatcher(t, filepath.Join("testdata", "errors.txt"))
-			desc := ast.Parse(em.Data, "errors.txt", em.ErrorHandler)
-			if desc == nil {
-				em.DumpErrors(t)
-				t.Fatalf("parsing failed")
-			}
-			ExtractConsts(desc, target, em.ErrorHandler)
-			em.Check(t)
-		})
-	}
-}
-
-func TestErrors2(t *testing.T) {
-	t.Parallel()
-	consts := map[string]uint64{
-		"SYS_foo": 1,
-		"C0":      0,
-		"C1":      1,
-		"C2":      2,
-	}
-	for _, arch := range []string{"32_shmem", "64"} {
-		target := targets.List["test"][arch]
-		t.Run(arch, func(t *testing.T) {
-			t.Parallel()
-			em := ast.NewErrorMatcher(t, filepath.Join("testdata", "errors2.txt"))
-			desc := ast.Parse(em.Data, "errors2.txt", em.ErrorHandler)
-			if desc == nil {
-				em.DumpErrors(t)
-				t.Fatalf("parsing failed")
-			}
-			info := ExtractConsts(desc, target, em.ErrorHandler)
-			if info == nil {
-				em.DumpErrors(t)
-				t.Fatalf("const extraction failed")
-			}
-			Compile(desc, consts, target, em.ErrorHandler)
-			em.Check(t)
-		})
-	}
-}
-
-func TestWarnings(t *testing.T) {
-	t.Parallel()
-	consts := map[string]uint64{
-		"SYS_foo": 1,
-	}
-	for _, arch := range []string{"32_shmem", "64"} {
-		target := targets.List["test"][arch]
-		t.Run(arch, func(t *testing.T) {
-			t.Parallel()
-			em := ast.NewErrorMatcher(t, filepath.Join("testdata", "warnings.txt"))
-			desc := ast.Parse(em.Data, "warnings.txt", em.ErrorHandler)
-			if desc == nil {
-				em.DumpErrors(t)
-				t.Fatalf("parsing failed")
-			}
-			info := ExtractConsts(desc, target, em.ErrorHandler)
-			if info == nil {
-				em.DumpErrors(t)
-				t.Fatalf("const extraction failed")
-			}
-			p := Compile(desc, consts, target, em.ErrorHandler)
-			if p == nil {
-				em.DumpErrors(t)
-				t.Fatalf("compilation failed")
-			}
-			em.Check(t)
-		})
-	}
-}
-
 func TestFuzz(t *testing.T) {
 	t.Parallel()
 	for _, data := range []string{
diff --git a/pkg/compiler/consts_test.go b/pkg/compiler/consts_test.go
index 69810c877..77a124b04 100644
--- a/pkg/compiler/consts_test.go
+++ b/pkg/compiler/consts_test.go
@@ -66,10 +66,10 @@ func TestConstErrors(t *testing.T) {
 	em := ast.NewErrorMatcher(t, filepath.Join("testdata", name))
 	desc := ast.Parse(em.Data, name, em.ErrorHandler)
 	if desc == nil {
-		em.DumpErrors(t)
+		em.DumpErrors()
 		t.Fatalf("parsing failed")
 	}
 	target := targets.List["linux"]["amd64"]
 	ExtractConsts(desc, target, em.ErrorHandler)
-	em.Check(t)
+	em.Check()
 }
diff --git a/pkg/compiler/gen.go b/pkg/compiler/gen.go
index a82f8aab9..1ced4f7e3 100644
--- a/pkg/compiler/gen.go
+++ b/pkg/compiler/gen.go
@@ -249,7 +249,7 @@ func (ctx *structGen) walkStruct(t *prog.StructType) {
 		}
 		if sizeAttr != sizeUnassigned {
 			if t.TypeSize > sizeAttr {
-				comp.error(structNode.Pos, "struct %v has size attribute %v"+
+				comp.error(structNode.Attrs[0].Pos, "struct %v has size attribute %v"+
 					" which is less than struct size %v",
 					structNode.Name.Name, sizeAttr, t.TypeSize)
 			}
@@ -270,10 +270,10 @@ func (ctx *structGen) walkUnion(t *prog.UnionType) {
 	varlen, sizeAttr := comp.parseUnionAttrs(structNode)
 	t.TypeSize = 0
 	if !varlen {
-		for _, fld := range t.Fields {
+		for i, fld := range t.Fields {
 			sz := fld.Size()
 			if sizeAttr != sizeUnassigned && sz > sizeAttr {
-				comp.error(structNode.Pos, "union %v has size attribute %v"+
+				comp.error(structNode.Fields[i].Pos, "union %v has size attribute %v"+
 					" which is less than field %v size %v",
 					structNode.Name.Name, sizeAttr, fld.Name(), sz)
 			}
diff --git a/pkg/compiler/testdata/errors.txt b/pkg/compiler/testdata/errors.txt
index e0b003481..d5611a531 100644
--- a/pkg/compiler/testdata/errors.txt
+++ b/pkg/compiler/testdata/errors.txt
@@ -1,6 +1,8 @@
 # Copyright 2017 syzkaller project authors. All rights reserved.
 # Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
+# Errors that happen during type checking phase.
+
 foo$0(x fileoff, y int8, z buffer[in])
 foo$1(x "bar")			### unexpected string "bar", expect type
 foo$2(x 123, y "bar")		### unexpected int 123, expect type	### unexpected string "bar", expect type
diff --git a/pkg/compiler/testdata/errors2.txt b/pkg/compiler/testdata/errors2.txt
index b5ab19ebf..2b24d3628 100644
--- a/pkg/compiler/testdata/errors2.txt
+++ b/pkg/compiler/testdata/errors2.txt
@@ -1,6 +1,8 @@
 # Copyright 2017 syzkaller project authors. All rights reserved.
 # Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
+# Errors that happen during checking phase.
+
 # Recursive resources.
 
 resource r0[r0]			### recursive resource r0->r0
diff --git a/pkg/compiler/testdata/errors3.txt b/pkg/compiler/testdata/errors3.txt
new file mode 100644
index 000000000..65bd20092
--- /dev/null
+++ b/pkg/compiler/testdata/errors3.txt
@@ -0,0 +1,20 @@
+# Copyright 2020 syzkaller project authors. All rights reserved.
+# Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
+
+# Errors that happen during description generation phase.
+
+discrimination(a int32)
+discrimination$1(a int16)	### discrimination$1 arg a is redeclared with size 2, previously declared with size 4 at LOCATION
+
+bad_size_attr_struct {
+	a	int64
+} [size[4]]			### struct bad_size_attr_struct has size attribute 4 which is less than struct size 8
+
+bad_size_attr_union [
+	a	int8
+	b	int16
+	c	int32		### union bad_size_attr_union has size attribute 2 which is less than field int32 size 4
+	d	int64		### union bad_size_attr_union has size attribute 2 which is less than field int64 size 8
+] [size[2]]
+
+bad_size_call(a ptr[in, bad_size_attr_struct], b ptr[in, bad_size_attr_union])