pkg/report: separate reporter wrapper from OS-specific implementations

Currently a number of report post-processing activities are implemented as a
decorator over the interface that defines OS-specific implementations.
Following exactly the same interface is too restrictive in this case as adding
extra parameters to the post-processing forces the developer to adjust all
implementations thay may not need these parameters at all.

Untie the wrapper from the Reporter interface. Use a package-private
reporterImpl interface for the OS-specific implementations, while having an
exported Reporter structure. Make sure that Reporter is stored and
passed as a pointer.

20 files changed, 58 insertions, 44 deletions

diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go
index 70846711f..794d4484f 100644
--- a/pkg/instance/instance.go
+++ b/pkg/instance/instance.go
@@ -243,7 +243,7 @@ func (env *env) Test(numVMs int, reproSyz, reproOpts, reproC []byte) ([]error, e
 type inst struct {
 	cfg           *mgrconfig.Config
 	optionalFlags bool
-	reporter      report.Reporter
+	reporter      *report.Reporter
 	vmPool        *vm.Pool
 	vm            *vm.Instance
 	vmIndex       int
diff --git a/pkg/report/akaros.go b/pkg/report/akaros.go
index ad0d3c3dd..e3f6c2a25 100644
--- a/pkg/report/akaros.go
+++ b/pkg/report/akaros.go
@@ -20,7 +20,7 @@ type akaros struct {
 	objfile string
 }
 
-func ctorAkaros(cfg *config) (Reporter, []string, error) {
+func ctorAkaros(cfg *config) (reporterImpl, []string, error) {
 	ctx := &akaros{
 		config: cfg,
 	}
diff --git a/pkg/report/bsd.go b/pkg/report/bsd.go
index 327ddc8c6..b627ed4f4 100644
--- a/pkg/report/bsd.go
+++ b/pkg/report/bsd.go
@@ -23,7 +23,7 @@ type bsd struct {
 	symbols      map[string][]symbolizer.Symbol
 }
 
-func ctorBSD(cfg *config, oopses []*oops, symbolizeRes []*regexp.Regexp) (Reporter, error) {
+func ctorBSD(cfg *config, oopses []*oops, symbolizeRes []*regexp.Regexp) (reporterImpl, error) {
 	var symbols map[string][]symbolizer.Symbol
 	kernelObject := ""
 	if cfg.kernelObj != "" {
diff --git a/pkg/report/darwin.go b/pkg/report/darwin.go
index aea8ab455..1b93d5c5d 100644
--- a/pkg/report/darwin.go
+++ b/pkg/report/darwin.go
@@ -7,7 +7,7 @@ import (
 	"regexp"
 )
 
-func ctorDarwin(cfg *config) (Reporter, []string, error) {
+func ctorDarwin(cfg *config) (reporterImpl, []string, error) {
 	symbolizeRes := []*regexp.Regexp{}
 	ctx, err := ctorBSD(cfg, darwinOopses, symbolizeRes)
 	return ctx, nil, err
diff --git a/pkg/report/freebsd.go b/pkg/report/freebsd.go
index a379060af..fb4755d60 100644
--- a/pkg/report/freebsd.go
+++ b/pkg/report/freebsd.go
@@ -12,7 +12,7 @@ type freebsd struct {
 	*config
 }
 
-func ctorFreebsd(cfg *config) (Reporter, []string, error) {
+func ctorFreebsd(cfg *config) (reporterImpl, []string, error) {
 	ctx := &freebsd{
 		config: cfg,
 	}
diff --git a/pkg/report/fuchsia.go b/pkg/report/fuchsia.go
index e83d6fadf..39c175ccd 100644
--- a/pkg/report/fuchsia.go
+++ b/pkg/report/fuchsia.go
@@ -38,7 +38,7 @@ var (
 	}
 )
 
-func ctorFuchsia(cfg *config) (Reporter, []string, error) {
+func ctorFuchsia(cfg *config) (reporterImpl, []string, error) {
 	ctx := &fuchsia{
 		config: cfg,
 	}
diff --git a/pkg/report/fuzz.go b/pkg/report/fuzz.go
index 8952baf76..2e747929a 100644
--- a/pkg/report/fuzz.go
+++ b/pkg/report/fuzz.go
@@ -13,7 +13,7 @@ import (
 func Fuzz(data []byte) int {
 	res := 0
 	for os, reporter := range fuzzReporters {
-		typ := reporter.(*reporterWrapper).typ
+		typ := reporter.typ
 		containsCrash := reporter.ContainsCrash(data)
 		rep := reporter.Parse(data)
 		if containsCrash != (rep != nil) {
@@ -64,8 +64,8 @@ func Fuzz(data []byte) int {
 	return res
 }
 
-var fuzzReporters = func() map[string]Reporter {
-	reporters := make(map[string]Reporter)
+var fuzzReporters = func() map[string]*Reporter {
+	reporters := make(map[string]*Reporter)
 	for os := range ctors {
 		if os == targets.Windows {
 			continue
@@ -80,7 +80,7 @@ var fuzzReporters = func() map[string]Reporter {
 		if err != nil {
 			panic(err)
 		}
-		if _, ok := reporter.(*stub); ok {
+		if _, ok := reporter.impl.(*stub); ok {
 			continue
 		}
 		reporters[os] = reporter
diff --git a/pkg/report/gvisor.go b/pkg/report/gvisor.go
index ebe790e78..15961afad 100644
--- a/pkg/report/gvisor.go
+++ b/pkg/report/gvisor.go
@@ -12,7 +12,7 @@ type gvisor struct {
 	*config
 }
 
-func ctorGvisor(cfg *config) (Reporter, []string, error) {
+func ctorGvisor(cfg *config) (reporterImpl, []string, error) {
 	ctx := &gvisor{
 		config: cfg,
 	}
diff --git a/pkg/report/linux.go b/pkg/report/linux.go
index 16ca3c56d..6955a6057 100644
--- a/pkg/report/linux.go
+++ b/pkg/report/linux.go
@@ -33,7 +33,7 @@ type linux struct {
 	eoi                   []byte
 }
 
-func ctorLinux(cfg *config) (Reporter, []string, error) {
+func ctorLinux(cfg *config) (reporterImpl, []string, error) {
 	var symbols map[string][]symbolizer.Symbol
 	vmlinux := ""
 	if cfg.kernelObj != "" {
diff --git a/pkg/report/netbsd.go b/pkg/report/netbsd.go
index fea878622..d954b5e83 100644
--- a/pkg/report/netbsd.go
+++ b/pkg/report/netbsd.go
@@ -7,7 +7,7 @@ import (
 	"regexp"
 )
 
-func ctorNetbsd(cfg *config) (Reporter, []string, error) {
+func ctorNetbsd(cfg *config) (reporterImpl, []string, error) {
 	symbolizeRes := []*regexp.Regexp{
 		// stack
 		regexp.MustCompile(` at netbsd:([A-Za-z0-9_]+)\+0x([0-9a-f]+)`),
diff --git a/pkg/report/openbsd.go b/pkg/report/openbsd.go
index d24481776..8b38a2dbc 100644
--- a/pkg/report/openbsd.go
+++ b/pkg/report/openbsd.go
@@ -7,7 +7,7 @@ import (
 	"regexp"
 )
 
-func ctorOpenbsd(cfg *config) (Reporter, []string, error) {
+func ctorOpenbsd(cfg *config) (reporterImpl, []string, error) {
 	symbolizeRes := []*regexp.Regexp{
 		// stack
 		regexp.MustCompile(` at ([A-Za-z0-9_]+)\+0x([0-9a-f]+)`),
diff --git a/pkg/report/report.go b/pkg/report/report.go
index 502a5d552..7ee98c516 100644
--- a/pkg/report/report.go
+++ b/pkg/report/report.go
@@ -12,12 +12,13 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/google/syzkaller/pkg/mgrconfig"
 	"github.com/google/syzkaller/pkg/vcs"
 	"github.com/google/syzkaller/sys/targets"
 )
 
-type Reporter interface {
+type reporterImpl interface {
 	// ContainsCrash searches kernel console output for oops messages.
 	ContainsCrash(output []byte) bool
 
@@ -29,6 +30,12 @@ type Reporter interface {
 	Symbolize(rep *Report) error
 }
 
+type Reporter struct {
+	impl         reporterImpl
+	suppressions []*regexp.Regexp
+	typ          string
+}
+
 type Report struct {
 	// Title contains a representative description of the first oops.
 	Title string
@@ -90,7 +97,7 @@ func (t Type) String() string {
 }
 
 // NewReporter creates reporter for the specified OS/Type.
-func NewReporter(cfg *mgrconfig.Config) (Reporter, error) {
+func NewReporter(cfg *mgrconfig.Config) (*Reporter, error) {
 	typ := cfg.TargetOS
 	if cfg.Type == "gvisor" {
 		typ = cfg.Type
@@ -118,7 +125,7 @@ func NewReporter(cfg *mgrconfig.Config) (Reporter, error) {
 	if err != nil {
 		return nil, err
 	}
-	return &reporterWrapper{rep, supps, typ}, nil
+	return &Reporter{rep, supps, typ}, nil
 }
 
 const (
@@ -148,7 +155,7 @@ type config struct {
 	ignores        []*regexp.Regexp
 }
 
-type fn func(cfg *config) (Reporter, []string, error)
+type fn func(cfg *config) (reporterImpl, []string, error)
 
 func compileRegexps(list []string) ([]*regexp.Regexp, error) {
 	compiled := make([]*regexp.Regexp, len(list))
@@ -162,14 +169,8 @@ func compileRegexps(list []string) ([]*regexp.Regexp, error) {
 	return compiled, nil
 }
 
-type reporterWrapper struct {
-	Reporter
-	suppressions []*regexp.Regexp
-	typ          string
-}
-
-func (wrap *reporterWrapper) Parse(output []byte) *Report {
-	rep := wrap.Reporter.Parse(output)
+func (reporter *Reporter) Parse(output []byte) *Report {
+	rep := reporter.impl.Parse(output)
 	if rep == nil {
 		return nil
 	}
@@ -177,7 +178,7 @@ func (wrap *reporterWrapper) Parse(output []byte) *Report {
 	for i, title := range rep.AltTitles {
 		rep.AltTitles[i] = sanitizeTitle(replaceTable(dynamicTitleReplacement, title))
 	}
-	rep.Suppressed = matchesAny(rep.Output, wrap.suppressions)
+	rep.Suppressed = matchesAny(rep.Output, reporter.suppressions)
 	if bytes.Contains(rep.Output, gceConsoleHangup) {
 		rep.Corrupted = true
 	}
@@ -197,6 +198,19 @@ func (wrap *reporterWrapper) Parse(output []byte) *Report {
 	return rep
 }
 
+func (reporter *Reporter) ContainsCrash(output []byte) bool {
+	return reporter.impl.ContainsCrash(output)
+}
+
+func (reporter *Reporter) Symbolize(rep *Report) error {
+	return reporter.impl.Symbolize(rep)
+}
+
+// We need this method to enable comparisons though go-cmp.
+func (reporter Reporter) Equal(other Reporter) bool {
+	return cmp.Equal(reporter, other, cmp.AllowUnexported(Reporter{}))
+}
+
 func extractReportType(rep *Report) Type {
 	// Type/frame extraction logic should be integrated with oops types.
 	// But for now we do this more ad-hoc analysis here to at least isolate
@@ -219,13 +233,13 @@ func extractReportType(rep *Report) Type {
 	return Unknown
 }
 
-func IsSuppressed(reporter Reporter, output []byte) bool {
-	return matchesAny(output, reporter.(*reporterWrapper).suppressions) ||
+func IsSuppressed(reporter *Reporter, output []byte) bool {
+	return matchesAny(output, reporter.suppressions) ||
 		bytes.Contains(output, gceConsoleHangup)
 }
 
 // ParseAll returns all successive reports in output.
-func ParseAll(reporter Reporter, output []byte) (reports []*Report) {
+func ParseAll(reporter *Reporter, output []byte) (reports []*Report) {
 	for {
 		rep := reporter.Parse(output)
 		if rep == nil {
diff --git a/pkg/report/report_test.go b/pkg/report/report_test.go
index 355edc764..236186b1b 100644
--- a/pkg/report/report_test.go
+++ b/pkg/report/report_test.go
@@ -42,7 +42,7 @@ type ParseTest struct {
 	Report     []byte
 }
 
-func testParseFile(t *testing.T, reporter Reporter, fn string) {
+func testParseFile(t *testing.T, reporter *Reporter, fn string) {
 	data, err := ioutil.ReadFile(fn)
 	if err != nil {
 		t.Fatal(err)
@@ -151,7 +151,7 @@ func parseHeaderLine(t *testing.T, test *ParseTest, ln string) {
 	}
 }
 
-func testParseImpl(t *testing.T, reporter Reporter, test *ParseTest) {
+func testParseImpl(t *testing.T, reporter *Reporter, test *ParseTest) {
 	rep := reporter.Parse(test.Log)
 	containsCrash := reporter.ContainsCrash(test.Log)
 	expectCrash := (test.Title != "")
@@ -203,7 +203,7 @@ func testParseImpl(t *testing.T, reporter Reporter, test *ParseTest) {
 	checkReport(t, reporter, rep, test)
 }
 
-func checkReport(t *testing.T, reporter Reporter, rep *Report, test *ParseTest) {
+func checkReport(t *testing.T, reporter *Reporter, rep *Report, test *ParseTest) {
 	if test.HasReport && !bytes.Equal(rep.Report, test.Report) {
 		t.Fatalf("extracted wrong report:\n%s\nwant:\n%s", rep.Report, test.Report)
 	}
@@ -277,7 +277,7 @@ func TestGuiltyFile(t *testing.T) {
 	forEachFile(t, "guilty", testGuiltyFile)
 }
 
-func testGuiltyFile(t *testing.T, reporter Reporter, fn string) {
+func testGuiltyFile(t *testing.T, reporter *Reporter, fn string) {
 	data, err := ioutil.ReadFile(fn)
 	if err != nil {
 		t.Fatal(err)
@@ -322,7 +322,7 @@ func testGuiltyFile(t *testing.T, reporter Reporter, fn string) {
 	}
 }
 
-func forEachFile(t *testing.T, dir string, fn func(t *testing.T, reporter Reporter, fn string)) {
+func forEachFile(t *testing.T, dir string, fn func(t *testing.T, reporter *Reporter, fn string)) {
 	for os := range ctors {
 		if os == targets.Windows {
 			continue // not implemented
diff --git a/pkg/report/stub.go b/pkg/report/stub.go
index edd469606..d1946a338 100644
--- a/pkg/report/stub.go
+++ b/pkg/report/stub.go
@@ -7,7 +7,7 @@ type stub struct {
 	*config
 }
 
-func ctorStub(cfg *config) (Reporter, []string, error) {
+func ctorStub(cfg *config) (reporterImpl, []string, error) {
 	ctx := &stub{
 		config: cfg,
 	}
diff --git a/pkg/repro/repro.go b/pkg/repro/repro.go
index 4433c84a9..52f456a83 100644
--- a/pkg/repro/repro.go
+++ b/pkg/repro/repro.go
@@ -44,7 +44,7 @@ type Stats struct {
 
 type context struct {
 	target       *targets.Target
-	reporter     report.Reporter
+	reporter     *report.Reporter
 	crashTitle   string
 	crashType    report.Type
 	instances    chan *instance
@@ -63,7 +63,7 @@ type instance struct {
 	executorBin string
 }
 
-func Run(crashLog []byte, cfg *mgrconfig.Config, features *host.Features, reporter report.Reporter,
+func Run(crashLog []byte, cfg *mgrconfig.Config, features *host.Features, reporter *report.Reporter,
 	vmPool *vm.Pool, vmIndexes []int) (*Result, *Stats, error) {
 	if len(vmIndexes) == 0 {
 		return nil, nil, fmt.Errorf("no VMs provided")
diff --git a/syz-manager/manager.go b/syz-manager/manager.go
index add89116e..6bab4bb43 100644
--- a/syz-manager/manager.go
+++ b/syz-manager/manager.go
@@ -49,7 +49,7 @@ type Manager struct {
 	vmPool         *vm.Pool
 	target         *prog.Target
 	sysTarget      *targets.Target
-	reporter       report.Reporter
+	reporter       *report.Reporter
 	crashdir       string
 	serv           *RPCServer
 	corpusDB       *db.DB
diff --git a/syz-verifier/main.go b/syz-verifier/main.go
index eea9d92ed..0a047231d 100755
--- a/syz-verifier/main.go
+++ b/syz-verifier/main.go
@@ -83,7 +83,7 @@ type RPCServer struct {
 type poolInfo struct {
 	cfg      *mgrconfig.Config
 	pool     *vm.Pool
-	Reporter report.Reporter
+	Reporter *report.Reporter
 	//  runners keeps track of what programs have been sent to each Runner.
 	//  There is one Runner executing per VM instance.
 	runners map[int]runnerProgs
diff --git a/tools/syz-crush/crush.go b/tools/syz-crush/crush.go
index 5b2dcfffd..b795becc8 100644
--- a/tools/syz-crush/crush.go
+++ b/tools/syz-crush/crush.go
@@ -166,7 +166,7 @@ func storeCrash(cfg *mgrconfig.Config, rep *report.Report) {
 	}
 }
 
-func runInstance(cfg *mgrconfig.Config, reporter report.Reporter,
+func runInstance(cfg *mgrconfig.Config, reporter *report.Reporter,
 	vmPool *vm.Pool, index int, timeout time.Duration, runType FileType) *report.Report {
 	log.Printf("vm-%v: starting", index)
 	inst, err := vmPool.Create(index)
diff --git a/tools/syz-runtest/runtest.go b/tools/syz-runtest/runtest.go
index 8ef15af38..98b433b04 100644
--- a/tools/syz-runtest/runtest.go
+++ b/tools/syz-runtest/runtest.go
@@ -132,7 +132,7 @@ func main() {
 type Manager struct {
 	cfg              *mgrconfig.Config
 	vmPool           *vm.Pool
-	reporter         report.Reporter
+	reporter         *report.Reporter
 	requests         chan *runtest.RunRequest
 	checkResult      *rpctype.CheckArgs
 	checkResultReady chan bool
diff --git a/vm/vm.go b/vm/vm.go
index d5a1f35db..4664bb576 100644
--- a/vm/vm.go
+++ b/vm/vm.go
@@ -179,7 +179,7 @@ const (
 // Exit says which exit modes should be considered as errors/OK.
 // Returns a non-symbolized crash report, or nil if no error happens.
 func (inst *Instance) MonitorExecution(outc <-chan []byte, errc <-chan error,
-	reporter report.Reporter, exit ExitCondition) (rep *report.Report) {
+	reporter *report.Reporter, exit ExitCondition) (rep *report.Report) {
 	mon := &monitor{
 		inst:     inst,
 		outc:     outc,
@@ -266,7 +266,7 @@ type monitor struct {
 	inst     *Instance
 	outc     <-chan []byte
 	errc     <-chan error
-	reporter report.Reporter
+	reporter *report.Reporter
 	exit     ExitCondition
 	output   []byte
 	matchPos int