Skip to content

Commit

Permalink
tools/syz-trace2syz: start adding proper error handling
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dvyukov committed Dec 7, 2018
1 parent 8056889 commit 742f85b
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 20 deletions.
14 changes: 9 additions & 5 deletions tools/syz-trace2syz/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package parser
import (
"bufio"
"bytes"
"fmt"
"strings"

"github.com/google/syzkaller/pkg/log"
Expand All @@ -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))
Expand All @@ -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
}
35 changes: 28 additions & 7 deletions tools/syz-trace2syz/parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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 {
Expand Down
16 changes: 10 additions & 6 deletions tools/syz-trace2syz/proggen/proggen.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package proggen

import (
"encoding/binary"
"fmt"
"io/ioutil"
"math/rand"

Expand All @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion tools/syz-trace2syz/proggen/proggen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
5 changes: 4 additions & 1 deletion tools/syz-trace2syz/trace2syz.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 742f85b

Please sign in to comment.