From c992206a1d8b6e537fe1782d025c25ea77bce06d Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Thu, 2 Jul 2020 18:38:27 +0200 Subject: tools/syz-linter: add custom linter For now we have 2 simple checks: 1. for multiline comments: /* */ -> // 2. for string len comparison with 0: len(str) != 0 -> str != "" Update #1876 --- tools/syz-linter/linter.go | 98 ++++++++++++++++++++++ tools/syz-linter/linter_test.go | 15 ++++ .../testdata/src/lintertest/lintertest.go | 19 +++++ 3 files changed, 132 insertions(+) create mode 100644 tools/syz-linter/linter.go create mode 100644 tools/syz-linter/linter_test.go create mode 100644 tools/syz-linter/testdata/src/lintertest/lintertest.go (limited to 'tools') diff --git a/tools/syz-linter/linter.go b/tools/syz-linter/linter.go new file mode 100644 index 000000000..c4b6837b2 --- /dev/null +++ b/tools/syz-linter/linter.go @@ -0,0 +1,98 @@ +// Copyright 2020 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +// This is our linter with custom checks for the project. +// See the following tutorial on writing Go analyzers: +// https://disaev.me/p/writing-useful-go-analysis-linter/ +// See the following tutorial on adding custom golangci-lint linters: +// https://golangci-lint.run/contributing/new-linters/ +// See comments below and testdata/src/lintertest/lintertest.go for the actual checks we do. +package main + +import ( + "go/ast" + "go/token" + "strings" + + "golang.org/x/tools/go/analysis" +) + +var AnalyzerPlugin analyzerPlugin + +type analyzerPlugin struct{} + +func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer { + return []*analysis.Analyzer{ + SyzAnalyzer, + } +} + +var SyzAnalyzer = &analysis.Analyzer{ + Name: "lint", + Doc: "custom syzkaller project checks", + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + for _, file := range pass.Files { + ast.Inspect(file, func(n ast.Node) bool { + switch n := n.(type) { + case *ast.Comment: + checkMulitlineComments(pass, n) + case *ast.BinaryExpr: + checkStringLenCompare(pass, n) + } + return true + }) + } + return nil, nil +} + +// checkMulitlineComments warns about C++-style multiline comments. +// We don't use them in the codebase. +func checkMulitlineComments(pass *analysis.Pass, n *ast.Comment) { + if !strings.HasPrefix(n.Text, "/*") { + return + } + pass.Report(analysis.Diagnostic{ + Pos: n.Pos(), + Message: "Use C-style comments // instead of /* */", + }) +} + +// checkStringLenCompare checks for string len comparisons with 0. +// E.g.: if len(str) == 0 {} should be if str == "" {}. +func checkStringLenCompare(pass *analysis.Pass, n *ast.BinaryExpr) { + if n.Op != token.EQL && n.Op != token.NEQ && n.Op != token.LSS && + n.Op != token.GTR && n.Op != token.LEQ && n.Op != token.GEQ { + return + } + if isStringLenCall(pass, n.X) && isIntZeroLiteral(n.Y) || + isStringLenCall(pass, n.Y) && isIntZeroLiteral(n.X) { + pass.Report(analysis.Diagnostic{ + Pos: n.Pos(), + Message: "Compare string with \"\", don't compare len with 0", + }) + } +} + +func isStringLenCall(pass *analysis.Pass, n ast.Expr) bool { + call, ok := n.(*ast.CallExpr) + if !ok || len(call.Args) != 1 { + return false + } + fun, ok := call.Fun.(*ast.Ident) + if !ok || fun.Name != "len" { + return false + } + return pass.TypesInfo.Types[call.Args[0]].Type.String() == "string" +} + +func isIntZeroLiteral(n ast.Expr) bool { + lit, ok := n.(*ast.BasicLit) + return ok && lit.Kind == token.INT && lit.Value == "0" +} + +func main() { + _ = AnalyzerPlugin +} diff --git a/tools/syz-linter/linter_test.go b/tools/syz-linter/linter_test.go new file mode 100644 index 000000000..a356f5cf5 --- /dev/null +++ b/tools/syz-linter/linter_test.go @@ -0,0 +1,15 @@ +// Copyright 2020 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +package main + +import ( + "testing" + + "github.com/google/syzkaller/pkg/osutil" + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestLinter(t *testing.T) { + analysistest.Run(t, osutil.Abs("testdata"), SyzAnalyzer, "lintertest") +} diff --git a/tools/syz-linter/testdata/src/lintertest/lintertest.go b/tools/syz-linter/testdata/src/lintertest/lintertest.go new file mode 100644 index 000000000..b507bc1ee --- /dev/null +++ b/tools/syz-linter/testdata/src/lintertest/lintertest.go @@ -0,0 +1,19 @@ +// Copyright 2020 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +package lintertest + +/* some comment */ // want "Use C-style comments // instead of /* */" +var comment = 1 /* some comment */ // want "Use C-style comments // instead of /* */" + +func stringComparison() { + str := "" + if len(str) == 0 { // want "Compare string with \"\", don't compare len with 0" + } + if 0 != len(str) { // want "Compare string with \"\", don't compare len with 0" + } + if len(returnString()+"foo") > 0 { // want "Compare string with \"\", don't compare len with 0" + } +} + +func returnString() string { return "foo" } -- cgit mrf-deployment