From 742f85bb22809a5bec027ebec811c2b7de20796f Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Fri, 7 Dec 2018 12:05:43 +0100 Subject: [PATCH] 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. --- tools/syz-trace2syz/parser/parser.go | 14 ++++++--- tools/syz-trace2syz/parser/parser_test.go | 35 ++++++++++++++++----- tools/syz-trace2syz/proggen/proggen.go | 16 ++++++---- tools/syz-trace2syz/proggen/proggen_test.go | 5 ++- tools/syz-trace2syz/trace2syz.go | 5 ++- 5 files changed, 55 insertions(+), 20 deletions(-) diff --git a/tools/syz-trace2syz/parser/parser.go b/tools/syz-trace2syz/parser/parser.go index 08be7f254587..7d999747d3cf 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 2a8373611262..b14fd4f8114a 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 becee3ee8faa..66a0cf84293f 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 136017beb0cb..1e7a5f3a67e5 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 d5fab69e91cb..d86333360f4a 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 {