From db33b3dda2268ab83d456651df956500b1caea5f Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Mon, 11 Jul 2022 13:26:59 +0000 Subject: 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. --- pkg/report/linux.go | 21 ++++++++------ pkg/report/linux_test.go | 38 +++++++++++++++---------- pkg/report/testdata/linux/decompile/amd64/2.out | 21 -------------- 3 files changed, 36 insertions(+), 44 deletions(-) (limited to 'pkg') 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 -- cgit mrf-deployment