pkg/report: include Maintainers into report

Currently getting a complete report requires a complex,
multi-step dance (including getting information that
external users are not interested in -- guilty file).

Simplify interface down to 2 functions: Parse and Symbolize.
Parse does what it did before, Symbolize symbolizes report
and fills in maintainers. This simplifies both implementations
of Reporter interface and all users of the interface.

Potentially we could get this down to 1 function Parse
that does everything. However, (1) Symbolize can fail,
while Parse cannot, (2) usually we want to ignore (log)
Symbolize errors, but otherwise proceed with the report,
(3) repro does not need symbolization for all but the
last report.
This commit is contained in:
Dmitry Vyukov 2017-11-29 17:53:07 +01:00
parent 46c864566a
commit 29b0fd90e6
14 changed files with 79 additions and 173 deletions

View File

@ -35,18 +35,6 @@ func (ctx *akaros) Parse(output []byte) *Report {
panic("not implemented")
}
func (ctx *akaros) Symbolize(text []byte) ([]byte, error) {
panic("not implemented")
}
func (ctx *akaros) ExtractConsoleOutput(output []byte) (result []byte) {
panic("not implemented")
}
func (ctx *akaros) ExtractGuiltyFile(report []byte) string {
panic("not implemented")
}
func (ctx *akaros) GetMaintainers(file string) ([]string, error) {
func (ctx *akaros) Symbolize(rep *Report) error {
panic("not implemented")
}

View File

@ -5,7 +5,6 @@ package report
import (
"bytes"
"fmt"
"regexp"
"github.com/google/syzkaller/pkg/symbolizer"
@ -76,20 +75,8 @@ func (ctx *freebsd) Parse(output []byte) *Report {
return rep
}
func (ctx *freebsd) Symbolize(text []byte) ([]byte, error) {
return nil, fmt.Errorf("not implemented")
}
func (ctx *freebsd) ExtractConsoleOutput(output []byte) (result []byte) {
return output
}
func (ctx *freebsd) ExtractGuiltyFile(report []byte) string {
return ""
}
func (ctx *freebsd) GetMaintainers(file string) ([]string, error) {
return nil, fmt.Errorf("not implemented")
func (ctx *freebsd) Symbolize(rep *Report) error {
return nil
}
var freebsdOopses = []*oops{

View File

@ -35,18 +35,6 @@ func (ctx *fuchsia) Parse(output []byte) *Report {
panic("not implemented")
}
func (ctx *fuchsia) Symbolize(text []byte) ([]byte, error) {
panic("not implemented")
}
func (ctx *fuchsia) ExtractConsoleOutput(output []byte) (result []byte) {
panic("not implemented")
}
func (ctx *fuchsia) ExtractGuiltyFile(report []byte) string {
panic("not implemented")
}
func (ctx *fuchsia) GetMaintainers(file string) ([]string, error) {
func (ctx *fuchsia) Symbolize(rep *Report) error {
panic("not implemented")
}

View File

@ -32,12 +32,15 @@ type linux struct {
func ctorLinux(kernelSrc, kernelObj string, symbols map[string][]symbolizer.Symbol,
ignores []*regexp.Regexp) (Reporter, error) {
vmlinux := filepath.Join(kernelObj, "vmlinux")
if symbols == nil {
var err error
symbols, err = symbolizer.ReadSymbols(vmlinux)
if err != nil {
return nil, err
vmlinux := ""
if kernelObj != "" {
vmlinux = filepath.Join(kernelObj, "vmlinux")
if symbols == nil {
var err error
symbols, err = symbolizer.ReadSymbols(vmlinux)
if err != nil {
return nil, err
}
}
}
ctx := &linux{
@ -77,7 +80,7 @@ func (ctx *linux) ContainsCrash(output []byte) bool {
}
func (ctx *linux) Parse(output []byte) *Report {
output = ctx.ExtractConsoleOutput(output)
output = ctx.extractConsoleOutput(output)
rep := &Report{
Output: output,
}
@ -170,7 +173,26 @@ func (ctx *linux) Parse(output []byte) *Report {
return rep
}
func (ctx *linux) Symbolize(text []byte) ([]byte, error) {
func (ctx *linux) Symbolize(rep *Report) error {
if ctx.vmlinux == "" {
return nil
}
symbolized, err := ctx.symbolize(rep.Report)
if err != nil {
return err
}
rep.Report = symbolized
guiltyFile := ctx.extractGuiltyFile(rep.Report)
if guiltyFile != "" {
rep.Maintainers, err = ctx.getMaintainers(guiltyFile)
if err != nil {
return err
}
}
return nil
}
func (ctx *linux) symbolize(text []byte) ([]byte, error) {
symb := symbolizer.NewSymbolizer()
defer symb.Close()
// Strip vmlinux location from all paths.
@ -253,7 +275,7 @@ func symbolizeLine(symbFunc func(bin string, pc uint64) ([]symbolizer.Frame, err
return symbolized
}
func (ctx *linux) ExtractConsoleOutput(output []byte) (result []byte) {
func (ctx *linux) extractConsoleOutput(output []byte) (result []byte) {
for pos := 0; pos < len(output); {
next := bytes.IndexByte(output[pos:], '\n')
if next != -1 {
@ -277,7 +299,7 @@ func (ctx *linux) ExtractConsoleOutput(output []byte) (result []byte) {
return
}
func (ctx *linux) ExtractGuiltyFile(report []byte) string {
func (ctx *linux) extractGuiltyFile(report []byte) string {
files := ctx.extractFiles(report)
nextFile:
for _, file := range files {
@ -291,13 +313,13 @@ nextFile:
return ""
}
func (ctx *linux) GetMaintainers(file string) ([]string, error) {
mtrs, err := ctx.getMaintainers(file, false)
func (ctx *linux) getMaintainers(file string) ([]string, error) {
mtrs, err := ctx.getMaintainersImpl(file, false)
if err != nil {
return nil, err
}
if len(mtrs) <= 1 {
mtrs, err = ctx.getMaintainers(file, true)
mtrs, err = ctx.getMaintainersImpl(file, true)
if err != nil {
return nil, err
}
@ -305,7 +327,7 @@ func (ctx *linux) GetMaintainers(file string) ([]string, error) {
return mtrs, nil
}
func (ctx *linux) getMaintainers(file string, blame bool) ([]string, error) {
func (ctx *linux) getMaintainersImpl(file string, blame bool) ([]string, error) {
args := []string{"--no-n", "--no-rolestats"}
if blame {
args = append(args, "--git-blame")

View File

@ -2910,8 +2910,9 @@ Call Trace:
if err != nil {
t.Fatal(err)
}
linux := reporter.(*linux)
for report, guilty0 := range tests {
if guilty := reporter.ExtractGuiltyFile([]byte(report)); guilty != guilty0 {
if guilty := linux.extractGuiltyFile([]byte(report)); guilty != guilty0 {
t.Logf("log:\n%s", report)
t.Logf("want guilty:\n%s", guilty0)
t.Logf("got guilty:\n%s", guilty)

View File

@ -4,7 +4,6 @@
package report
import (
"fmt"
"regexp"
"github.com/google/syzkaller/pkg/symbolizer"
@ -36,18 +35,6 @@ func (ctx *netbsd) Parse(output []byte) *Report {
return nil
}
func (ctx *netbsd) Symbolize(text []byte) ([]byte, error) {
return nil, fmt.Errorf("not implemented")
}
func (ctx *netbsd) ExtractConsoleOutput(output []byte) (result []byte) {
return output
}
func (ctx *netbsd) ExtractGuiltyFile(report []byte) string {
return ""
}
func (ctx *netbsd) GetMaintainers(file string) ([]string, error) {
return nil, fmt.Errorf("not implemented")
func (ctx *netbsd) Symbolize(rep *Report) error {
return nil
}

View File

@ -22,11 +22,8 @@ type Reporter interface {
// Returns nil if no oops found.
Parse(output []byte) *Report
Symbolize(text []byte) ([]byte, error)
ExtractConsoleOutput(output []byte) (result []byte)
ExtractGuiltyFile(report []byte) string
GetMaintainers(file string) ([]string, error)
// Symbolize symbolizes rep.Report and fills in Maintainers.
Symbolize(rep *Report) error
}
type Report struct {
@ -41,6 +38,8 @@ type Report struct {
EndPos int
// Corrupted indicates whether the report is truncated of corrupted in some other way.
Corrupted bool
// Maintainers is list of maintainer emails.
Maintainers []string
}
// NewReporter creates reporter for the specified OS:

View File

@ -35,18 +35,6 @@ func (ctx *windows) Parse(output []byte) *Report {
panic("not implemented")
}
func (ctx *windows) Symbolize(text []byte) ([]byte, error) {
panic("not implemented")
}
func (ctx *windows) ExtractConsoleOutput(output []byte) (result []byte) {
panic("not implemented")
}
func (ctx *windows) ExtractGuiltyFile(report []byte) string {
panic("not implemented")
}
func (ctx *windows) GetMaintainers(file string) ([]string, error) {
func (ctx *windows) Symbolize(rep *Report) error {
panic("not implemented")
}

View File

@ -35,7 +35,7 @@ type Result struct {
Opts csource.Options
CRepro bool
Stats Stats
// Information about the final crash that we reproduced.
// Information about the final (non-symbolized) crash that we reproduced.
// Can be different from what we started reproducing.
Report *report.Report
}

View File

@ -343,6 +343,10 @@ func (job *Job) testProgram(inst *vm.Instance, command string, reporter report.R
if rep == nil {
return false, nil
}
if err := reporter.Symbolize(rep); err != nil {
// TODO(dvyukov): send such errors to dashboard.
Logf(0, "job: failed to symbolize report: %v", err)
}
job.resp.CrashTitle = rep.Title
job.resp.CrashReport = rep.Report
job.resp.CrashLog = rep.Output

View File

@ -262,16 +262,6 @@ func (mgr *Manager) httpReport(w http.ResponseWriter, r *http.Request) {
}
fmt.Fprintf(w, "Syzkaller hit '%s' bug%s.\n\n", trimNewLines(desc), commitDesc)
if len(rep) != 0 {
guiltyFile := mgr.getReporter().ExtractGuiltyFile(rep)
if guiltyFile != "" {
fmt.Fprintf(w, "Guilty file: %v\n\n", guiltyFile)
maintainers, err := mgr.getReporter().GetMaintainers(guiltyFile)
if err == nil {
fmt.Fprintf(w, "Maintainers: %v\n\n", maintainers)
} else {
fmt.Fprintf(w, "Failed to extract maintainers: %v\n\n", err)
}
}
fmt.Fprintf(w, "%s\n\n", rep)
}
if len(prog) == 0 && len(cprog) == 0 {

View File

@ -580,22 +580,15 @@ func (mgr *Manager) saveCrash(crash *Crash) bool {
}
mgr.mu.Unlock()
crash.Report.Report = mgr.symbolizeReport(crash.Report.Report)
if err := mgr.getReporter().Symbolize(crash.Report); err != nil {
Logf(0, "failed to symbolize report: %v", err)
}
if mgr.dash != nil {
var maintainers []string
guiltyFile := mgr.getReporter().ExtractGuiltyFile(crash.Report.Report)
if guiltyFile != "" {
var err error
maintainers, err = mgr.getReporter().GetMaintainers(guiltyFile)
if err != nil {
Logf(0, "failed to get maintainers: %v", err)
}
}
dc := &dashapi.Crash{
BuildID: mgr.cfg.Tag,
Title: crash.Title,
Corrupted: crash.Corrupted,
Maintainers: maintainers,
Maintainers: crash.Maintainers,
Log: crash.Output,
Report: crash.Report.Report,
}
@ -685,7 +678,9 @@ func (mgr *Manager) saveFailedRepro(desc string) {
func (mgr *Manager) saveRepro(res *repro.Result, hub bool) {
rep := res.Report
rep.Report = mgr.symbolizeReport(rep.Report)
if err := mgr.getReporter().Symbolize(rep); err != nil {
Logf(0, "failed to symbolize repro: %v", err)
}
dir := filepath.Join(mgr.crashdir, hash.String([]byte(rep.Title)))
osutil.MkdirAll(dir)
@ -733,19 +728,10 @@ func (mgr *Manager) saveRepro(res *repro.Result, hub bool) {
}
if mgr.dash != nil {
var maintainers []string
guiltyFile := mgr.getReporter().ExtractGuiltyFile(res.Report.Report)
if guiltyFile != "" {
var err error
maintainers, err = mgr.getReporter().GetMaintainers(guiltyFile)
if err != nil {
Logf(0, "failed to get maintainers: %v", err)
}
}
dc := &dashapi.Crash{
BuildID: mgr.cfg.Tag,
Title: res.Report.Title,
Maintainers: maintainers,
Maintainers: res.Report.Maintainers,
Log: res.Report.Output,
Report: res.Report.Report,
ReproOpts: res.Opts.Serialize(),
@ -758,26 +744,19 @@ func (mgr *Manager) saveRepro(res *repro.Result, hub bool) {
}
}
func (mgr *Manager) symbolizeReport(text []byte) []byte {
if len(text) == 0 || mgr.cfg.Vmlinux == "" {
return text
}
symbolized, err := mgr.getReporter().Symbolize(text)
if err != nil {
Logf(0, "failed to symbolize report: %v", err)
return text
}
return symbolized
}
func (mgr *Manager) getReporter() report.Reporter {
if mgr.reporter == nil {
<-allSymbolsReady
var err error
// TODO(dvyukov): we should introduce cfg.Kernel_Obj dir instead of Vmlinux.
// This will be more general taking into account modules and other OSes.
mgr.reporter, err = report.NewReporter(mgr.cfg.TargetOS, mgr.cfg.Kernel_Src,
filepath.Dir(mgr.cfg.Vmlinux), allSymbols, mgr.cfg.ParsedIgnores)
kernelSrc, kernelObj := "", ""
if mgr.cfg.Vmlinux != "" {
kernelSrc = mgr.cfg.Kernel_Src
kernelObj = filepath.Dir(mgr.cfg.Vmlinux)
}
mgr.reporter, err = report.NewReporter(mgr.cfg.TargetOS, kernelSrc, kernelObj,
allSymbols, mgr.cfg.ParsedIgnores)
if err != nil {
Fatalf("%v", err)
}

View File

@ -17,7 +17,6 @@ var (
flagOS = flag.String("os", runtime.GOOS, "target os")
flagKernelSrc = flag.String("kernel_src", "", "path to kernel sources")
flagKernelObj = flag.String("kernel_obj", "", "path to kernel build/obj dir")
flagReport = flag.Bool("report", false, "extract report from the log")
)
func main() {
@ -37,40 +36,14 @@ func main() {
fmt.Fprintf(os.Stderr, "failed to open input file: %v\n", err)
os.Exit(1)
}
if *flagReport {
rep := reporter.Parse(text)
if rep == nil {
fmt.Fprintf(os.Stderr, "no crash found\n")
os.Exit(1)
}
text = rep.Report
text, err = reporter.Symbolize(text)
if err != nil {
fmt.Fprintf(os.Stderr, "failed to symbolize: %v\n", err)
os.Exit(1)
}
guiltyFile := reporter.ExtractGuiltyFile(text)
fmt.Printf("%v\n\n", rep.Title)
os.Stdout.Write(text)
fmt.Printf("\n")
fmt.Printf("guilty file: %v\n", guiltyFile)
if guiltyFile != "" {
maintainers, err := reporter.GetMaintainers(guiltyFile)
if err != nil {
fmt.Fprintf(os.Stderr, "failed to get maintainers: %v\n", err)
os.Exit(1)
}
fmt.Printf("maintainers: %v\n", maintainers)
}
} else {
if console := reporter.ExtractConsoleOutput(text); len(console) != 0 {
text = console
}
text, err = reporter.Symbolize(text)
if err != nil {
fmt.Fprintf(os.Stderr, "failed to symbolize: %v\n", err)
os.Exit(1)
}
os.Stdout.Write(text)
rep := reporter.Parse(text)
if rep == nil {
fmt.Fprintf(os.Stderr, "no crash found\n")
os.Exit(1)
}
if err := reporter.Symbolize(rep); err != nil {
fmt.Fprintf(os.Stderr, "failed to symbolize report: %v\n", err)
os.Exit(1)
}
os.Stdout.Write(rep.Report)
}

View File

@ -100,7 +100,7 @@ func (inst *Instance) Close() {
// It detects kernel oopses in output, lost connections, hangs, etc.
// outc/errc is what vm.Instance.Run returns, reporter parses kernel output for oopses.
// If canExit is false and the program exits, it is treated as an error.
// Returns crash report, or nil if no error happens.
// Returns a non-symbolized crash report, or nil if no error happens.
func MonitorExecution(outc <-chan []byte, errc <-chan error, reporter report.Reporter, canExit bool) (
rep *report.Report) {
var output []byte