tools/syz-trace2syz: start adding proper error handling

log.Fatal is not the proper way to handle errors.
It does not allow to write good tests, fuzzers
and utilities that crash all the time.

5 files changed, 55 insertions, 20 deletions

diff --git a/tools/syz-trace2syz/parser/parser.go b/tools/syz-trace2syz/parser/parser.go
index 08be7f254..7d999747d 100644
--- a/tools/syz-trace2syz/parser/parser.go
+++ b/tools/syz-trace2syz/parser/parser.go
@@ -6,6 +6,7 @@ package parser
 import (
 	"bufio"
 	"bytes"
+	"fmt"
 	"strings"
 
 	"github.com/google/syzkaller/pkg/log"
@@ -25,7 +26,7 @@ func shouldSkip(line string) bool {
 }
 
 // ParseLoop parses each line of a strace file in a loop.
-func ParseLoop(data []byte) *TraceTree {
+func ParseData(data []byte) (*TraceTree, error) {
 	tree := NewTraceTree()
 	// Creating the process tree
 	scanner := bufio.NewScanner(bytes.NewReader(data))
@@ -38,12 +39,15 @@ func ParseLoop(data []byte) *TraceTree {
 		log.Logf(4, "scanning call: %s", line)
 		ret, call := parseSyscall(scanner)
 		if call == nil || ret != 0 {
-			log.Fatalf("failed to parse line: %s", line)
+			return nil, fmt.Errorf("failed to parse line: %v", line)
 		}
 		tree.add(call)
 	}
-	if scanner.Err() != nil || len(tree.TraceMap) == 0 {
-		return nil
+	if err := scanner.Err(); err != nil {
+		return nil, err
 	}
-	return tree
+	if len(tree.TraceMap) == 0 {
+		return nil, nil
+	}
+	return tree, nil
 }
diff --git a/tools/syz-trace2syz/parser/parser_test.go b/tools/syz-trace2syz/parser/parser_test.go
index 2a8373611..b14fd4f81 100644
--- a/tools/syz-trace2syz/parser/parser_test.go
+++ b/tools/syz-trace2syz/parser/parser_test.go
@@ -59,7 +59,10 @@ func TestParseLoopBasic(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		tree := ParseLoop([]byte(test))
+		tree, err := ParseData([]byte(test))
+		if err != nil {
+			t.Fatal(err)
+		}
 		if tree.RootPid != -1 {
 			t.Fatalf("Incorrect Root Pid: %d", tree.RootPid)
 		}
@@ -92,7 +95,10 @@ func TestEvaluateExpressions(t *testing.T) {
 		{"open(4 >> 1) = 0", 2},
 	}
 	for i, test := range tests {
-		tree := ParseLoop([]byte(test.line))
+		tree, err := ParseData([]byte(test.line))
+		if err != nil {
+			t.Fatal(err)
+		}
 		if tree.RootPid != -1 {
 			t.Fatalf("failed test: %d. Incorrect Root Pid: %d", i, tree.RootPid)
 		}
@@ -114,7 +120,10 @@ func TestParseLoopPid(t *testing.T) {
 	data := `1  open() = 3
 			 1  fstat() = 0`
 
-	tree := ParseLoop([]byte(data))
+	tree, err := ParseData([]byte(data))
+	if err != nil {
+		t.Fatal(err)
+	}
 	if tree.RootPid != 1 {
 		t.Fatalf("Incorrect Root Pid: %d", tree.RootPid)
 	}
@@ -133,7 +142,10 @@ func TestParseLoop1Child(t *testing.T) {
 				   1 clone() = 2
                    2 read() = 16`
 
-	tree := ParseLoop([]byte(data1Child))
+	tree, err := ParseData([]byte(data1Child))
+	if err != nil {
+		t.Fatal(err)
+	}
 	if len(tree.TraceMap) != 2 {
 		t.Fatalf("Incorrect Root Pid. Expected: 2, Got %d", tree.RootPid)
 	}
@@ -155,7 +167,10 @@ func TestParseLoop2Childs(t *testing.T) {
                     2 read() = 16
                     1 clone() = 3
                     3 open() = 3`
-	tree := ParseLoop([]byte(data2Childs))
+	tree, err := ParseData([]byte(data2Childs))
+	if err != nil {
+		t.Fatal(err)
+	}
 	if len(tree.TraceMap) != 3 {
 		t.Fatalf("Incorrect Root Pid. Expected: 3, Got %d", tree.RootPid)
 	}
@@ -169,7 +184,10 @@ func TestParseLoop1Grandchild(t *testing.T) {
 						1 clone() = 2
 						2 clone() = 3
 						3 open() = 4`
-	tree := ParseLoop([]byte(data1Grandchild))
+	tree, err := ParseData([]byte(data1Grandchild))
+	if err != nil {
+		t.Fatal(err)
+	}
 	if len(tree.Ptree[tree.RootPid]) != 1 {
 		t.Fatalf("Expect RootPid to have 1 child. Got %d", tree.RootPid)
 	}
@@ -188,7 +206,10 @@ func TestParseGroupType(t *testing.T) {
 		{`open([1, 2, 3]) = 0`},
 	}
 	for _, test := range tests {
-		tree := ParseLoop([]byte(test.test))
+		tree, err := ParseData([]byte(test.test))
+		if err != nil {
+			t.Fatal(err)
+		}
 		call := tree.TraceMap[tree.RootPid].Calls[0]
 		_, ok := call.Args[0].(*GroupType)
 		if !ok {
diff --git a/tools/syz-trace2syz/proggen/proggen.go b/tools/syz-trace2syz/proggen/proggen.go
index becee3ee8..66a0cf842 100644
--- a/tools/syz-trace2syz/proggen/proggen.go
+++ b/tools/syz-trace2syz/proggen/proggen.go
@@ -5,6 +5,7 @@ package proggen
 
 import (
 	"encoding/binary"
+	"fmt"
 	"io/ioutil"
 	"math/rand"
 
@@ -13,22 +14,25 @@ import (
 	"github.com/google/syzkaller/tools/syz-trace2syz/parser"
 )
 
-func ParseFile(filename string, target *prog.Target) []*prog.Prog {
+func ParseFile(filename string, target *prog.Target) ([]*prog.Prog, error) {
 	data, err := ioutil.ReadFile(filename)
 	if err != nil {
-		log.Fatalf("error reading file: %v", err)
+		return nil, fmt.Errorf("error reading file: %v", err)
 	}
 	return ParseData(data, target)
 }
 
-func ParseData(data []byte, target *prog.Target) []*prog.Prog {
-	tree := parser.ParseLoop(data)
+func ParseData(data []byte, target *prog.Target) ([]*prog.Prog, error) {
+	tree, err := parser.ParseData(data)
+	if err != nil {
+		return nil, err
+	}
 	if tree == nil {
-		return nil
+		return nil, nil
 	}
 	var progs []*prog.Prog
 	parseTree(tree, tree.RootPid, target, &progs)
-	return progs
+	return progs, nil
 }
 
 // parseTree groups system calls in the trace by process id.
diff --git a/tools/syz-trace2syz/proggen/proggen_test.go b/tools/syz-trace2syz/proggen/proggen_test.go
index 136017beb..1e7a5f3a6 100644
--- a/tools/syz-trace2syz/proggen/proggen_test.go
+++ b/tools/syz-trace2syz/proggen/proggen_test.go
@@ -33,7 +33,10 @@ func initializeTarget(os, arch string) *prog.Target {
 
 func parseSingleTrace(t *testing.T, data string) *prog.Prog {
 	target := initializeTarget(OS, Arch)
-	traceTree := parser.ParseLoop([]byte(data))
+	traceTree, err := parser.ParseData([]byte(data))
+	if err != nil {
+		t.Fatal(err)
+	}
 	p := genProg(traceTree.TraceMap[traceTree.RootPid], target)
 	if p == nil {
 		t.Fatalf("failed to parse trace")
diff --git a/tools/syz-trace2syz/trace2syz.go b/tools/syz-trace2syz/trace2syz.go
index d5fab69e9..d86333360 100644
--- a/tools/syz-trace2syz/trace2syz.go
+++ b/tools/syz-trace2syz/trace2syz.go
@@ -72,7 +72,10 @@ func parseTraces(target *prog.Target) []*prog.Prog {
 	log.Logf(0, "parsing %v traces", totalFiles)
 	for i, file := range names {
 		log.Logf(1, "parsing file %v/%v: %v", i+1, totalFiles, filepath.Base(names[i]))
-		progs := proggen.ParseFile(file, target)
+		progs, err := proggen.ParseFile(file, target)
+		if err != nil {
+			log.Fatalf("%v", err)
+		}
 		ret = append(ret, progs...)
 		if deserializeDir != "" {
 			for i, p := range progs {