From 2fcb2b5c16e21d2a94741e19bbe7443d88127273 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Thu, 4 Jun 2020 22:35:04 +0200 Subject: .golangci.yml: enable gocognit checker Finds too complex functions. Similar to gocyclo, but uses somewhat different metric. --- .golangci.yml | 7 +++ pkg/ifuzz/decode.go | 2 +- pkg/ifuzz/encode.go | 2 +- pkg/ifuzz/gen/gen.go | 4 +- pkg/report/linux.go | 2 +- pkg/runtest/run.go | 127 +++++++++++++++++++++++++------------------------ syz-manager/manager.go | 2 +- 7 files changed, 79 insertions(+), 67 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 1b042eb4c..553b6ff78 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -38,6 +38,7 @@ linters: - nestif - goprintffuncname - godot + - gocognit disable: - bodyclose - depguard @@ -68,16 +69,22 @@ linters-settings: lll: line-length: 120 gocyclo: + # TODO: consider reducing this value. min-complexity: 24 dupl: + # TODO: consider reducing this value. threshold: 60 goconst: min-len: 7 min-occurrences: 4 nestif: + # TODO: consider reducing this value. min-complexity: 12 godot: check-all: true + gocognit: + # TODO: consider reducing this value. + min-complexity: 70 issues: exclude-use-default: false diff --git a/pkg/ifuzz/decode.go b/pkg/ifuzz/decode.go index b16c9f0bd..327fad82e 100644 --- a/pkg/ifuzz/decode.go +++ b/pkg/ifuzz/decode.go @@ -10,7 +10,7 @@ import ( // Decode decodes instruction length for the given mode. // It can have falsely decode incorrect instructions, // but should not fail to decode correct instructions. -// nolint: gocyclo, nestif +// nolint: gocyclo, nestif, gocognit func Decode(mode int, text []byte) (int, error) { if len(text) == 0 { return 0, fmt.Errorf("zero-length instruction") diff --git a/pkg/ifuzz/encode.go b/pkg/ifuzz/encode.go index ecabcfcd9..dd884bb61 100644 --- a/pkg/ifuzz/encode.go +++ b/pkg/ifuzz/encode.go @@ -11,7 +11,7 @@ import ( "math/rand" ) -// nolint: gocyclo, nestif +// nolint: gocyclo, nestif, gocognit func (insn *Insn) Encode(cfg *Config, r *rand.Rand) []byte { if !insn.isCompatible(cfg) { panic("instruction is not suitable for this mode") diff --git a/pkg/ifuzz/gen/gen.go b/pkg/ifuzz/gen/gen.go index 86c04617d..72bc1fa0a 100644 --- a/pkg/ifuzz/gen/gen.go +++ b/pkg/ifuzz/gen/gen.go @@ -17,7 +17,7 @@ import ( "github.com/google/syzkaller/pkg/serializer" ) -// nolint: gocyclo +// nolint: gocyclo, gocognit func main() { if len(os.Args) != 2 { failf("usage: gen instructions.txt") @@ -180,7 +180,7 @@ func (err errSkip) Error() string { return string(err) } -// nolint: gocyclo +// nolint: gocyclo, gocognit func parsePattern(insn *ifuzz.Insn, vals []string) error { if insn.Opcode != nil { return fmt.Errorf("PATTERN is already parsed for the instruction") diff --git a/pkg/report/linux.go b/pkg/report/linux.go index 760c9949c..6f968ada9 100644 --- a/pkg/report/linux.go +++ b/pkg/report/linux.go @@ -196,7 +196,7 @@ func (ctx *linux) findFirstOops(output []byte) (oops *oops, startPos int, contex } // Yes, it is complex, but all state and logic are tightly coupled. It's unclear how to simplify it. -// nolint: gocyclo +// nolint: gocyclo, gocognit func (ctx *linux) findReport(output []byte, oops *oops, startPos int, context string, useQuestionable bool) ( endPos, reportEnd int, report []byte, prefix [][]byte) { // Prepend 5 lines preceding start of the report, diff --git a/pkg/runtest/run.go b/pkg/runtest/run.go index fa85563ee..1ded72e5a 100644 --- a/pkg/runtest/run.go +++ b/pkg/runtest/run.go @@ -166,87 +166,92 @@ func (ctx *Context) generatePrograms(progs chan *RunRequest) error { sandboxes = append(sandboxes, sandbox) } sort.Strings(sandboxes) - sysTarget := targets.Get(ctx.Target.OS, ctx.Target.Arch) for _, file := range files { if strings.HasSuffix(file.Name(), "~") || strings.HasSuffix(file.Name(), ".swp") || !strings.HasPrefix(file.Name(), ctx.Tests) { continue } - p, requires, results, err := ctx.parseProg(file.Name()) - if err != nil { + if err := ctx.generateFile(progs, sandboxes, cover, file.Name()); err != nil { return err } - nextSandbox: - for _, sandbox := range sandboxes { - name := fmt.Sprintf("%v %v", file.Name(), sandbox) - for _, call := range p.Calls { - if !ctx.EnabledCalls[sandbox][call.Meta] { - progs <- &RunRequest{ - name: name, - skip: fmt.Sprintf("unsupported call %v", call.Meta.Name), - } - continue nextSandbox + } + return nil +} + +func (ctx *Context) generateFile(progs chan *RunRequest, sandboxes []string, cover []bool, filename string) error { + p, requires, results, err := ctx.parseProg(filename) + if err != nil { + return err + } + sysTarget := targets.Get(ctx.Target.OS, ctx.Target.Arch) +nextSandbox: + for _, sandbox := range sandboxes { + name := fmt.Sprintf("%v %v", filename, sandbox) + for _, call := range p.Calls { + if !ctx.EnabledCalls[sandbox][call.Meta] { + progs <- &RunRequest{ + name: name, + skip: fmt.Sprintf("unsupported call %v", call.Meta.Name), } + continue nextSandbox } - properties := map[string]bool{ - "arch=" + ctx.Target.Arch: true, - "sandbox=" + sandbox: true, + } + properties := map[string]bool{ + "arch=" + ctx.Target.Arch: true, + "sandbox=" + sandbox: true, + } + for _, threaded := range []bool{false, true} { + name := name + if threaded { + name += "/thr" } - for _, threaded := range []bool{false, true} { - name := name - if threaded { - name += "/thr" + properties["threaded"] = threaded + for _, times := range []int{1, 3} { + properties["repeat"] = times > 1 + properties["norepeat"] = times <= 1 + if times > 1 { + name += "/repeat" } - properties["threaded"] = threaded - for _, times := range []int{1, 3} { - properties["repeat"] = times > 1 - properties["norepeat"] = times <= 1 - if times > 1 { - name += "/repeat" - } - for _, cov := range cover { - if sandbox == "" { - break // executor does not support empty sandbox - } - name := name - if cov { - name += "/cover" - } - properties["cover"] = cov - properties["C"] = false - properties["executor"] = true - req, err := ctx.createSyzTest(p, sandbox, threaded, cov, times) - if err != nil { - return err - } - ctx.produceTest(progs, req, name, properties, requires, results) - } - - if sysTarget.HostFuzzer { - // For HostFuzzer mode, we need to cross-compile - // and copy the binary to the target system. - continue + for _, cov := range cover { + if sandbox == "" { + break // executor does not support empty sandbox } - name := name - properties["C"] = true - properties["executor"] = false - name += " C" - if !sysTarget.ExecutorUsesForkServer && times > 1 { - // Non-fork loop implementation does not support repetition. - progs <- &RunRequest{ - name: name, - broken: "non-forking loop", - } - continue + if cov { + name += "/cover" } - req, err := ctx.createCTest(p, sandbox, threaded, times) + properties["cover"] = cov + properties["C"] = false + properties["executor"] = true + req, err := ctx.createSyzTest(p, sandbox, threaded, cov, times) if err != nil { return err } ctx.produceTest(progs, req, name, properties, requires, results) } + if sysTarget.HostFuzzer { + // For HostFuzzer mode, we need to cross-compile + // and copy the binary to the target system. + continue + } + name := name + properties["C"] = true + properties["executor"] = false + name += " C" + if !sysTarget.ExecutorUsesForkServer && times > 1 { + // Non-fork loop implementation does not support repetition. + progs <- &RunRequest{ + name: name, + broken: "non-forking loop", + } + continue + } + req, err := ctx.createCTest(p, sandbox, threaded, times) + if err != nil { + return err + } + ctx.produceTest(progs, req, name, properties, requires, results) } } } diff --git a/syz-manager/manager.go b/syz-manager/manager.go index 8b3ede955..e13747236 100644 --- a/syz-manager/manager.go +++ b/syz-manager/manager.go @@ -289,7 +289,7 @@ type ReproResult struct { } // Manager needs to be refactored (#605). -// nolint: gocyclo +// nolint: gocyclo, gocognit func (mgr *Manager) vmLoop() { log.Logf(0, "booting test machines...") log.Logf(0, "wait for the connection from test machine...") -- cgit mrf-deployment