pkg/report: don't decompile opcodes for hanged reports

It doesn't bring any extra value and only makes the reports bigger.

Don't do such decompilation for hang-related reports. Refactor the
opcode tests to rely more on the more generic NewReporter constructor.

3 files changed, 36 insertions, 44 deletions

diff --git a/pkg/report/linux.go b/pkg/report/linux.go
index d69370dcd..cff8b7d5f 100644
--- a/pkg/report/linux.go
+++ b/pkg/report/linux.go
@@ -367,7 +367,7 @@ func (ctx *linux) Symbolize(rep *Report) error {
 		}
 	}
 
-	rep.Report = ctx.decompileReportOpcodes(rep.Report)
+	rep.Report = ctx.decompileOpcodes(rep.Report, rep)
 
 	// We still do this even if we did not symbolize,
 	// because tests pass in already symbolized input.
@@ -623,14 +623,19 @@ func (ctx *linux) parseOpcodes(codeSlice string) (parsedOpcodes, error) {
 	}, nil
 }
 
-// decompileReportOpcodes detects the most meaningful "Code: " lines from the report, decompiles
+// decompileOpcodes detects the most meaningful "Code: " lines from the report, decompiles
 // them and appends a human-readable listing to the end of the report.
-func (ctx *linux) decompileReportOpcodes(report []byte) []byte {
+func (ctx *linux) decompileOpcodes(text []byte, report *Report) []byte {
+	if report.Type == Hang {
+		// Even though Hang reports do contain the Code: section, there's no point in
+		// decompiling that. So just return the text.
+		return text
+	}
 	// Iterate over all "Code: " lines and pick the first that could be decompiled
 	// that might be of interest to the user.
 	var decompiled *decompiledOpcodes
 	var prevLine []byte
-	for s := bufio.NewScanner(bytes.NewReader(report)); s.Scan(); prevLine = append([]byte{}, s.Bytes()...) {
+	for s := bufio.NewScanner(bytes.NewReader(text)); s.Scan(); prevLine = append([]byte{}, s.Bytes()...) {
 		// We want to avoid decompiling code from user-space as it is not of big interest during
 		// debugging kernel problems.
 		// For now this check only works for x86/amd64, but Linux on other architectures supported
@@ -651,7 +656,7 @@ func (ctx *linux) decompileReportOpcodes(report []byte) []byte {
 	}
 
 	if decompiled == nil {
-		return report
+		return text
 	}
 
 	skipInfo := ""
@@ -664,7 +669,7 @@ func (ctx *linux) decompileReportOpcodes(report []byte) []byte {
 	// the most important information at the top of the report, so that it is visible from
 	// the syzbot dashboard without scrolling.
 	headLine := fmt.Sprintf("----------------\nCode disassembly (best guess)%v:\n", skipInfo)
-	report = append(report, headLine...)
+	text = append(text, headLine...)
 
 	for idx, opcode := range decompiled.opcodes {
 		line := opcode.FullDescription
@@ -673,9 +678,9 @@ func (ctx *linux) decompileReportOpcodes(report []byte) []byte {
 		} else {
 			line += "\n"
 		}
-		report = append(report, line...)
+		text = append(text, line...)
 	}
-	return report
+	return text
 }
 
 func (ctx *linux) extractGuiltyFile(rep *Report) string {
diff --git a/pkg/report/linux_test.go b/pkg/report/linux_test.go
index bba6326e0..5aba2f19e 100644
--- a/pkg/report/linux_test.go
+++ b/pkg/report/linux_test.go
@@ -238,15 +238,19 @@ func TestLinuxSymbolizeLine(t *testing.T) {
 	}
 }
 
-func prepareLinuxReporter(t *testing.T, arch string) *linux {
-	config := &config{
-		target: targets.Get(targets.Linux, arch),
+func prepareLinuxReporter(t *testing.T, arch string) (*Reporter, *linux) {
+	cfg := &mgrconfig.Config{
+		Derived: mgrconfig.Derived{
+			TargetOS:   targets.Linux,
+			TargetArch: arch,
+			SysTarget:  targets.Get(targets.Linux, arch),
+		},
 	}
-	reporter, _, err := ctorLinux(config)
+	reporter, err := NewReporter(cfg)
 	if err != nil {
-		t.Errorf("Failed to create a reporter instance %#v", arch)
+		t.Errorf("Failed to create a reporter instance for %#v: %v", arch, err)
 	}
-	return reporter.(*linux)
+	return reporter, reporter.impl.(*linux)
 }
 
 func TestParseLinuxOpcodes(t *testing.T) {
@@ -362,8 +366,8 @@ func TestParseLinuxOpcodes(t *testing.T) {
 		test := test // Capturing the value.
 		t.Run(fmt.Sprintf("%s/%v", test.arch, idx), func(t *testing.T) {
 			t.Parallel()
-			reporter := prepareLinuxReporter(t, test.arch)
-			ret, err := reporter.parseOpcodes(test.input)
+			_, linuxReporter := prepareLinuxReporter(t, test.arch)
+			ret, err := linuxReporter.parseOpcodes(test.input)
 			if test.output == nil && err == nil {
 				t.Errorf("Expected an error on input %#v", test)
 			} else if test.output != nil && err != nil {
@@ -390,10 +394,9 @@ func TestDisassemblyInReports(t *testing.T) {
 		if !obj.IsDir() {
 			continue
 		}
-		reporter := prepareLinuxReporter(t, obj.Name())
-
-		if reporter.target.BrokenCompiler != "" {
-			t.Skip("skipping the test due to broken cross-compiler:\n" + reporter.target.BrokenCompiler)
+		reporter, linuxReporter := prepareLinuxReporter(t, obj.Name())
+		if linuxReporter.target.BrokenCompiler != "" {
+			t.Skip("skipping the test due to broken cross-compiler:\n" + linuxReporter.target.BrokenCompiler)
 		}
 
 		testPath := filepath.Join(archPath, obj.Name())
@@ -408,13 +411,13 @@ func TestDisassemblyInReports(t *testing.T) {
 			}
 			filePath := filepath.Join(testPath, strings.TrimSuffix(file.Name(), ".in"))
 			t.Run(obj.Name()+"/"+file.Name(), func(t *testing.T) {
-				testDisassembly(t, reporter, filePath)
+				testDisassembly(t, reporter, linuxReporter, filePath)
 			})
 		}
 	}
 }
 
-func testDisassembly(t *testing.T, reporter *linux, testFilePrefix string) {
+func testDisassembly(t *testing.T, reporter *Reporter, linuxReporter *linux, testFilePrefix string) {
 	t.Parallel()
 
 	input, err := ioutil.ReadFile(testFilePrefix + ".in")
@@ -422,7 +425,12 @@ func testDisassembly(t *testing.T, reporter *linux, testFilePrefix string) {
 		t.Fatalf("failed to read input file: %v", err)
 	}
 
-	result := reporter.decompileReportOpcodes(input)
+	report := reporter.Parse(input)
+	if report == nil {
+		t.Fatalf("no bug report was found")
+	}
+
+	result := linuxReporter.decompileOpcodes(input, report)
 	if *flagUpdate {
 		osutil.WriteFile(testFilePrefix+".out", result)
 	}
diff --git a/pkg/report/testdata/linux/decompile/amd64/2.out b/pkg/report/testdata/linux/decompile/amd64/2.out
index 96ae47e75..7559caf25 100644
--- a/pkg/report/testdata/linux/decompile/amd64/2.out
+++ b/pkg/report/testdata/linux/decompile/amd64/2.out
@@ -162,24 +162,3 @@ R13: 0000000000000000 R14: ffff8880b9c55de8 R15: 0000000000000001
  worker_thread+0x658/0x11f0 kernel/workqueue.c:2422
  kthread+0x3e5/0x4d0 kernel/kthread.c:319
  ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295
-----------------
-Code disassembly (best guess):
-   0:	1d ad 91 bc 7e       	sbb    $0x7ebc91ad,%eax
-   5:	65 8b 05 a6 91 bc 7e 	mov    %gs:0x7ebc91a6(%rip),%eax        # 0x7ebc91b2
-   c:	a9 00 ff ff 00       	test   $0xffff00,%eax
-  11:	74 45                	je     0x58
-  13:	bf 01 00 00 00       	mov    $0x1,%edi
-  18:	e8 15 2a 09 00       	callq  0x92a32
-  1d:	e8 50 84 35 00       	callq  0x358472
-  22:	fb                   	sti
-  23:	65 8b 05 88 91 bc 7e 	mov    %gs:0x7ebc9188(%rip),%eax        # 0x7ebc91b2
-* 2a:	85 c0                	test   %eax,%eax <-- trapping instruction
-  2c:	74 58                	je     0x86
-  2e:	5b                   	pop    %rbx
-  2f:	5d                   	pop    %rbp
-  30:	c3                   	retq
-  31:	65 8b 05 d6 98 bc 7e 	mov    %gs:0x7ebc98d6(%rip),%eax        # 0x7ebc990e
-  38:	85 c0                	test   %eax,%eax
-  3a:	75 a2                	jne    0xffffffde
-  3c:	0f 0b                	ud2
-  3e:	eb 9e                	jmp    0xffffffde