From 8ef497b65213b43164bcb9437d0c5bdd986cd52c Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Tue, 31 Jul 2018 20:38:33 +0200 Subject: gometalinter: clean up vetshadow This just cleans up existing warnings. vetshadow is not enabled yet because it crashes. Update #538 --- .gometalinter.json | 1 + dashboard/app/jobs.go | 12 +++--- pkg/ast/parser.go | 4 +- pkg/compiler/consts.go | 2 +- pkg/email/parser.go | 5 ++- pkg/ifuzz/ifuzz.go | 18 ++++----- pkg/repro/repro.go | 94 ++++++++++++++++++++++++---------------------- sys/syz-extract/extract.go | 22 +++++------ syz-ci/syzupdater.go | 2 +- 9 files changed, 84 insertions(+), 76 deletions(-) diff --git a/.gometalinter.json b/.gometalinter.json index 1624ae70d..e9d341ade 100644 --- a/.gometalinter.json +++ b/.gometalinter.json @@ -38,6 +38,7 @@ "(sys/(akaros|freebsd|fuchsia|linux|netbsd|test|windows)/init.*|sys/targets/common.go).* don't use ALL_CAPS in Go names", "exported .* should have comment", "comment on .* should be of the form", + "declaration of \"err\" shadows", "(pkg/csource/generated.go|pkg/report/linux.go|pkg/build/linux_generated.go).* line is [0-9]+ characters" ] } diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go index 7148a6b9e..b12c85776 100644 --- a/dashboard/app/jobs.go +++ b/dashboard/app/jobs.go @@ -54,7 +54,7 @@ func handleTestRequest(c context.Context, bugID, user, extID, link, patch, repo, if err := datastore.Get(c, bugKey, bug); err != nil { return err } - bugReporting := bugReportingByName(bug, bugReporting.Name) + bugReporting = bugReportingByName(bug, bugReporting.Name) bugCC := strings.Split(bugReporting.CC, "|") merged := email.MergeEmailLists(bugCC, jobCC) bugReporting.CC = strings.Join(merged, "|") @@ -150,12 +150,12 @@ func addTestJob(c context.Context, bug *Bug, bugKey *datastore.Key, bugReporting if len(jobs) != 0 { // The job is already present, update link. deletePatch = true - job, jobKey := jobs[0], keys[0] - if job.Link != "" || link == "" { + existingJob, jobKey := jobs[0], keys[0] + if existingJob.Link != "" || link == "" { return nil } - job.Link = link - if _, err := datastore.Put(c, jobKey, job); err != nil { + existingJob.Link = link + if _, err := datastore.Put(c, jobKey, existingJob); err != nil { return fmt.Errorf("failed to put job: %v", err) } return nil @@ -218,7 +218,7 @@ retry: stale := false tx := func(c context.Context) error { stale = false - job := new(Job) + job = new(Job) if err := datastore.Get(c, jobKey, job); err != nil { return fmt.Errorf("job %v: failed to get in tx: %v", jobID, err) } diff --git a/pkg/ast/parser.go b/pkg/ast/parser.go index d43011796..bd9d33657 100644 --- a/pkg/ast/parser.go +++ b/pkg/ast/parser.go @@ -262,11 +262,11 @@ func (p *parser) parseTypeDef() *TypeDef { } p.consume(tokRBrack) if p.tok == tokLBrace || p.tok == tokLBrack { - name := &Ident{ + emptyName := &Ident{ Pos: pos0, Name: "", } - str = p.parseStruct(name) + str = p.parseStruct(emptyName) } else { typ = p.parseType() } diff --git a/pkg/compiler/consts.go b/pkg/compiler/consts.go index 79bf84e70..d3556c047 100644 --- a/pkg/compiler/consts.go +++ b/pkg/compiler/consts.go @@ -293,7 +293,7 @@ func DeserializeConsts(data []byte, file string, eh ast.ErrorHandler) map[string ok = false continue } - if _, ok := consts[name]; ok { + if _, dup := consts[name]; dup { eh(pos, fmt.Sprintf("duplicate const %q", name)) ok = false continue diff --git a/pkg/email/parser.go b/pkg/email/parser.go index 7916a0e0e..1b2dbc747 100644 --- a/pkg/email/parser.go +++ b/pkg/email/parser.go @@ -247,11 +247,12 @@ func extractArgsLine(body []byte) string { return strings.TrimSpace(string(body[pos : pos+lineEnd])) } -func parseBody(r io.Reader, headers mail.Header) (body []byte, attachments [][]byte, err error) { +func parseBody(r io.Reader, headers mail.Header) ([]byte, [][]byte, error) { // git-send-email sends emails without Content-Type, let's assume it's text. mediaType := "text/plain" var params map[string]string if contentType := headers.Get("Content-Type"); contentType != "" { + var err error mediaType, params, err = mime.ParseMediaType(headers.Get("Content-Type")) if err != nil { return nil, nil, fmt.Errorf("failed to parse email header 'Content-Type': %v", err) @@ -281,6 +282,8 @@ func parseBody(r io.Reader, headers mail.Header) (body []byte, attachments [][]b if !strings.HasPrefix(mediaType, "multipart/") { return nil, nil, nil } + var body []byte + var attachments [][]byte mr := multipart.NewReader(r, params["boundary"]) for { p, err := mr.NextPart() diff --git a/pkg/ifuzz/ifuzz.go b/pkg/ifuzz/ifuzz.go index 985aa3501..2d51e0340 100644 --- a/pkg/ifuzz/ifuzz.go +++ b/pkg/ifuzz/ifuzz.go @@ -158,23 +158,23 @@ func Mutate(cfg *Config, r *rand.Rand, text []byte) []byte { switch x := r.Intn(100); { case x < 5 && len(text1) != 0: // delete byte - i := r.Intn(len(text1)) - copy(text1[i:], text1[i+1:]) + pos := r.Intn(len(text1)) + copy(text1[pos:], text1[pos+1:]) text1 = text1[:len(text1)-1] case x < 40 && len(text1) != 0: // replace a byte - i := r.Intn(len(text1)) - text1[i] = byte(r.Intn(256)) + pos := r.Intn(len(text1)) + text1[pos] = byte(r.Intn(256)) case x < 70 && len(text1) != 0: // flip a bit - i := r.Intn(len(text1)) - text1[i] ^= 1 << byte(r.Intn(8)) + pos := r.Intn(len(text1)) + text1[pos] ^= 1 << byte(r.Intn(8)) default: // insert a byte - i := r.Intn(len(text1) + 1) + pos := r.Intn(len(text1) + 1) text1 = append(text1, 0) - copy(text1[i+1:], text1[i:]) - text1[i] = byte(r.Intn(256)) + copy(text1[pos+1:], text1[pos:]) + text1[pos] = byte(r.Intn(256)) } } insns[i] = text1 diff --git a/pkg/repro/repro.go b/pkg/repro/repro.go index 44218f64b..5d6670a20 100644 --- a/pkg/repro/repro.go +++ b/pkg/repro/repro.go @@ -433,22 +433,24 @@ func (ctx *context) simplifyProg(res *Result) (*Result, error) { for _, simplify := range progSimplifies { opts := res.Opts - if simplify(&opts) { - crashed, err := ctx.testProg(res.Prog, res.Duration, opts) - if err != nil { - return nil, err - } - if crashed { - res.Opts = opts - // Simplification successful, try extracting C repro. - res, err := ctx.extractC(res) - if err != nil { - return nil, err - } - if res.CRepro { - return res, nil - } - } + if !simplify(&opts) { + continue + } + crashed, err := ctx.testProg(res.Prog, res.Duration, opts) + if err != nil { + return nil, err + } + if !crashed { + continue + } + res.Opts = opts + // Simplification successful, try extracting C repro. + res, err = ctx.extractC(res) + if err != nil { + return nil, err + } + if res.CRepro { + return res, nil } } @@ -614,30 +616,6 @@ func (ctx *context) bisectProgs(progs []*prog.LogEntry, pred func([]*prog.LogEnt []*prog.LogEntry, error) { ctx.reproLog(3, "bisect: bisecting %d programs", len(progs)) - compose := func(guilty1, guilty2 [][]*prog.LogEntry, chunk []*prog.LogEntry) []*prog.LogEntry { - progs := []*prog.LogEntry{} - for _, c := range guilty1 { - progs = append(progs, c...) - } - progs = append(progs, chunk...) - for _, c := range guilty2 { - progs = append(progs, c...) - } - return progs - } - - logGuilty := func(guilty [][]*prog.LogEntry) string { - log := "[" - for i, chunk := range guilty { - log += fmt.Sprintf("<%d>", len(chunk)) - if i != len(guilty)-1 { - log += ", " - } - } - log += "]" - return log - } - ctx.reproLog(3, "bisect: executing all %d programs", len(progs)) crashed, err := pred(progs) if err != nil { @@ -650,7 +628,7 @@ func (ctx *context) bisectProgs(progs []*prog.LogEntry, pred func([]*prog.LogEnt guilty := [][]*prog.LogEntry{progs} again: - ctx.reproLog(3, "bisect: guilty chunks: %v", logGuilty(guilty)) + ctx.reproLog(3, "bisect: guilty chunks: %v", chunksToStr(guilty)) for i, chunk := range guilty { if len(chunk) == 1 { continue @@ -658,14 +636,16 @@ again: guilty1 := guilty[:i] guilty2 := guilty[i+1:] - ctx.reproLog(3, "bisect: guilty chunks split: %v, <%v>, %v", logGuilty(guilty1), len(chunk), logGuilty(guilty2)) + ctx.reproLog(3, "bisect: guilty chunks split: %v, <%v>, %v", + chunksToStr(guilty1), len(chunk), chunksToStr(guilty2)) chunk1 := chunk[0 : len(chunk)/2] chunk2 := chunk[len(chunk)/2:] - ctx.reproLog(3, "bisect: chunk split: <%v> => <%v>, <%v>", len(chunk), len(chunk1), len(chunk2)) + ctx.reproLog(3, "bisect: chunk split: <%v> => <%v>, <%v>", + len(chunk), len(chunk1), len(chunk2)) ctx.reproLog(3, "bisect: triggering crash without chunk #1") - progs := compose(guilty1, guilty2, chunk2) + progs = flatenChunks(guilty1, guilty2, chunk2) crashed, err := pred(progs) if err != nil { return nil, err @@ -681,7 +661,7 @@ again: } ctx.reproLog(3, "bisect: triggering crash without chunk #2") - progs = compose(guilty1, guilty2, chunk1) + progs = flatenChunks(guilty1, guilty2, chunk1) crashed, err = pred(progs) if err != nil { return nil, err @@ -719,6 +699,30 @@ again: return progs, nil } +func flatenChunks(guilty1, guilty2 [][]*prog.LogEntry, chunk []*prog.LogEntry) []*prog.LogEntry { + var progs []*prog.LogEntry + for _, c := range guilty1 { + progs = append(progs, c...) + } + progs = append(progs, chunk...) + for _, c := range guilty2 { + progs = append(progs, c...) + } + return progs +} + +func chunksToStr(chunks [][]*prog.LogEntry) string { + log := "[" + for i, chunk := range chunks { + log += fmt.Sprintf("<%d>", len(chunk)) + if i != len(chunks)-1 { + log += ", " + } + } + log += "]" + return log +} + func reverseEntries(entries []*prog.LogEntry) []*prog.LogEntry { last := len(entries) - 1 for i := 0; i < len(entries)/2; i++ { diff --git a/sys/syz-extract/extract.go b/sys/syz-extract/extract.go index 612e045db..c1dce0105 100644 --- a/sys/syz-extract/extract.go +++ b/sys/syz-extract/extract.go @@ -48,13 +48,13 @@ type File struct { done chan bool } -type OS interface { +type Extractor interface { prepare(sourcedir string, build bool, arches []string) error prepareArch(arch *Arch) error processFile(arch *Arch, info *compiler.ConstInfo) (map[string]uint64, map[string]bool, error) } -var oses = map[string]OS{ +var extractors = map[string]Extractor{ "akaros": new(akaros), "linux": new(linux), "freebsd": new(freebsd), @@ -79,11 +79,11 @@ func main() { failf("%v", err) } - OS := oses[osStr] - if OS == nil { + extractor := extractors[osStr] + if extractor == nil { failf("unknown os: %v", osStr) } - if err := OS.prepare(*flagSourceDir, *flagBuild, archArray); err != nil { + if err := extractor.prepare(*flagSourceDir, *flagBuild, archArray); err != nil { failf("%v", err) } @@ -132,7 +132,7 @@ func main() { for job := range jobC { switch j := job.(type) { case *Arch: - infos, err := processArch(OS, j) + infos, err := processArch(extractor, j) j.err = err close(j.done) if j.err == nil { @@ -142,7 +142,7 @@ func main() { } } case *File: - j.consts, j.undeclared, j.err = processFile(OS, j.arch, j) + j.consts, j.undeclared, j.err = processFile(extractor, j.arch, j) close(j.done) } } @@ -244,7 +244,7 @@ func archFileList(os, arch string, files []string) (string, []string, []string, return os, arches, files, nil } -func processArch(OS OS, arch *Arch) (map[string]*compiler.ConstInfo, error) { +func processArch(extractor Extractor, arch *Arch) (map[string]*compiler.ConstInfo, error) { errBuf := new(bytes.Buffer) eh := func(pos ast.Pos, msg string) { fmt.Fprintf(errBuf, "%v: %v\n", pos, msg) @@ -257,13 +257,13 @@ func processArch(OS OS, arch *Arch) (map[string]*compiler.ConstInfo, error) { if infos == nil { return nil, fmt.Errorf("%v", errBuf.String()) } - if err := OS.prepareArch(arch); err != nil { + if err := extractor.prepareArch(arch); err != nil { return nil, err } return infos, nil } -func processFile(OS OS, arch *Arch, file *File) (map[string]uint64, map[string]bool, error) { +func processFile(extractor Extractor, arch *Arch, file *File) (map[string]uint64, map[string]bool, error) { inname := filepath.Join("sys", arch.target.OS, file.name) outname := strings.TrimSuffix(inname, ".txt") + "_" + arch.target.Arch + ".const" if file.info == nil { @@ -273,7 +273,7 @@ func processFile(OS OS, arch *Arch, file *File) (map[string]uint64, map[string]b os.Remove(outname) return nil, nil, nil } - consts, undeclared, err := OS.processFile(arch, file.info) + consts, undeclared, err := extractor.processFile(arch, file.info) if err != nil { return nil, nil, err } diff --git a/syz-ci/syzupdater.go b/syz-ci/syzupdater.go index 94cc2cf87..87f6c61ca 100644 --- a/syz-ci/syzupdater.go +++ b/syz-ci/syzupdater.go @@ -232,7 +232,7 @@ func (upd *SyzUpdater) build(commit *vcs.Commit) error { } for target := range upd.targets { parts := strings.Split(target, "/") - cmd := osutil.Command("make", "target") + cmd = osutil.Command("make", "target") cmd.Dir = upd.syzkallerDir cmd.Env = append([]string{}, os.Environ()...) cmd.Env = append(cmd.Env, -- cgit mrf-deployment