[ Added kbuild and Masahiro to CC ... ] On Tue, Aug 06, 2019 at 02:10:46PM -0700, Nathan Huckleberry wrote: > Signed-off-by: Nathan Huckleberry <nhuck@xxxxxxxxxx> > --- These two lines should be at the end of your commit log. :) > These patches add clang-tidy and the clang static-analyzer as make > targets. The goal of these patches is to make static analysis tools > usable and extendable by any developer or researcher who is familiar > with basic c++. > > The current static analysis tools require intimate knowledge of the internal > workings of the static analysis. Clang-tidy and the clang static analyzers > expose an easy to use api and allow users unfamiliar with clang to > write new checks with relative ease. > > ===Clang-tidy=== > > Clang-tidy is an easily extendable 'linter' that runs on the AST. > Clang-tidy checks are easy to write and understand. A check consists of > two parts, a matcher and a checker. The matcher is created using a > domain specific language that acts on the AST > (https://clang.llvm.org/docs/LibASTMatchersReference.html). When AST > nodes are found by the matcher a callback is made to the checker. The > checker can then execute additional checks and issue warnings. > > Here is an example clang-tidy check to report functions that have calls > to local_irq_disable without calls to local_irq_enable and vice-versa. > Functions flagged with __attribute((annotation("ignore_irq_balancing"))) > are ignored for analysis. > > The full patch can be found here (https://reviews.llvm.org/D65828) > > ``` > void IrqUnbalancedCheck::registerMatchers(MatchFinder *Finder) { > // finds calls to "arch_local_irq_disable" in a function body > auto disable = > forEachDescendant( > callExpr( > hasDeclaration( > namedDecl( > hasName("arch_local_irq_disable")))).bind("disable")); > > // finds calls to "arch_local_irq_enable" in a function body > auto enable = > forEachDescendant( > callExpr( > hasDeclaration( > namedDecl( > hasName("arch_local_irq_enable")))).bind("enable")); > > // Looks for function body that has the following property: > // ((disable && !enable) || (enable && !disable)) > auto matcher = functionDecl( > anyOf(allOf(disable, unless(enable)), allOf(enable, unless(disable)))); > > Finder->addMatcher(matcher.bind("func"), this); > } > > std::string annotation = "ignore_irq_balancing"; > void IrqUnbalancedCheck::check(const MatchFinder::MatchResult &Result) { > const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("func"); > const auto *DisableCall = Result.Nodes.getNodeAs<CallExpr>("disable"); > const auto *EnableCall = Result.Nodes.getNodeAs<CallExpr>("enable"); > > // If the function has __attribute((annotate("ignore_irq_balancing")) > for (const auto *Attr : MatchedDecl->attrs()) { > if (Attr->getKind() == clang::attr::Annotate) { > if(dyn_cast<AnnotateAttr>(Attr)->getAnnotation().str() == annotation) { > return; > } > } > } > > if(EnableCall) { > diag(MatchedDecl->getLocation(), "call to 'enable_local_irq' without 'disable_local_irq' in %0 ") > << MatchedDecl; > diag(EnableCall->getBeginLoc(), "call to 'enable_local_irq'", DiagnosticIDs::Note) > << MatchedDecl; > } > > if(DisableCall) { > diag(MatchedDecl->getLocation(), "call to 'disable_local_irq' without 'enable_local_irq' in %0 ") > << MatchedDecl; > diag(DisableCall->getBeginLoc(), "call to 'disable_local_irq'", DiagnosticIDs::Note) > << MatchedDecl; > } > } > ``` > > ===Clang static analyzer=== > > The clang static analyzer is a more powerful static analysis tool that > uses symbolic execution to find bugs. Currently there is a check that > looks for potential security bugs from invalid uses of kmalloc and > kfree. There are several more general purpose checks that are useful for > the kernel. > > The clang static analyzer is well documented and designed to be > extensible. > (https://clang-analyzer.llvm.org/checker_dev_manual.html) > (https://github.com/haoNoQ/clang-analyzer-guide/releases/download/v0.1/clang-analyzer-guide-v0.1.pdf) > > > Why add clang-tidy and the clang static analyzer when other static > analyzers are already in the kernel? > > The main draw of the clang tools is how accessible they are. The clang > documentation is very nice and these tools are built specifically to be > easily extendable by any developer. They provide an accessible method of > bug-finding and research to people who are not overly familiar with the > kernel codebase. > ^ i.e. they should be right here... > Makefile | 3 ++ > scripts/clang-tools/Makefile.clang-tools | 35 ++++++++++++++ > .../{ => clang-tools}/gen_compile_commands.py | 0 > scripts/clang-tools/parse_compile_commands.py | 47 +++++++++++++++++++ > 4 files changed, 85 insertions(+) > create mode 100644 scripts/clang-tools/Makefile.clang-tools > rename scripts/{ => clang-tools}/gen_compile_commands.py (100%) > create mode 100755 scripts/clang-tools/parse_compile_commands.py > > diff --git a/Makefile b/Makefile > index fabc127d127f..49f1d3fa48a8 100644 > --- a/Makefile > +++ b/Makefile > @@ -709,6 +709,7 @@ KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0) > > include scripts/Makefile.kcov > include scripts/Makefile.gcc-plugins > +include scripts/clang-tools/Makefile.clang-tools > > ifdef CONFIG_READABLE_ASM > # Disable optimizations that make assembler listings hard to read. > @@ -1470,6 +1471,8 @@ help: > @echo ' headers_check - Sanity check on exported headers' > @echo ' headerdep - Detect inclusion cycles in headers' > @echo ' coccicheck - Check with Coccinelle' > + @echo ' clang-analyzer - Check with clang static analyzer' > + @echo ' clang-tidy - Check with clang-tidy' I think this is great; more people will be able to run these tests. > @echo '' > @echo 'Kernel selftest:' > @echo ' kselftest - Build and run kernel selftest (run as root)' > diff --git a/scripts/clang-tools/Makefile.clang-tools b/scripts/clang-tools/Makefile.clang-tools > new file mode 100644 > index 000000000000..0adb6df20777 > --- /dev/null > +++ b/scripts/clang-tools/Makefile.clang-tools > @@ -0,0 +1,35 @@ > +# SPDX-License-Identifier: GPL-2.0 > +PHONY += clang-tidy > +HAS_PARALLEL := $(shell (parallel --version 2> /dev/null) | grep 'GNU parallel' 2> /dev/null) > +clang-tidy: > +ifdef CONFIG_CC_IS_CLANG > + $(PYTHON3) scripts/clang-tools/gen_compile_commands.py > +ifdef HAS_PARALLEL Is it worth falling back to xargs? Are there builders where clang-tidy or clang-analyzer are installed but parallel isn't? (i.e. it might just be better to simply require parallel.) Note that there's no test for python3 -- we just try to run it. :) > + #Xargs interleaves multiprocessed output. GNU Parallel does not. > + scripts/clang-tools/parse_compile_commands.py compile_commands.json \ > + | parallel -k -j $(shell nproc) 'echo {} && clang-tidy -p . "-checks=-*,linuxkernel-*" {}' > +else > + @echo "GNU parallel is not installed. Defaulting to non-parallelized runs" > + scripts/clang-tools/parse_compile_commands.py compile_commands.json \ > + | xargs -L 1 -I@ sh -c "echo '@' && clang-tidy -p . '-checks=-*,linuxkernel-*' @" > +endif > +else > + $(error Clang-tidy requires CC=clang) > +endif > + > +PHONY += clang-analyzer > +clang-analyzer: > +ifdef CONFIG_CC_IS_CLANG > + $(PYTHON3) scripts/clang-tools/gen_compile_commands.py > +ifdef HAS_PARALLEL > + #Xargs interleaves multiprocessed output. GNU Parallel does not. > + scripts/clang-tools/parse_compile_commands.py compile_commands.json \ > + | parallel -k -j $(shell nproc) 'echo {} && clang-tidy -p . "-checks=-*,clang-analyzer-*" {}' > +else > + @echo "GNU parallel is not installed. Defaulting to non-parallelized runs" > + scripts/clang-tools/parse_compile_commands.py compile_commands.json \ > + | xargs -L 1 -I@ sh -c "echo '@' && clang-tidy -p . '-checks=-*,clang-analyzer-*' @" > +endif > +else > + $(error Clang-analyzer requires CC=clang) > +endif > diff --git a/scripts/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py > similarity index 100% > rename from scripts/gen_compile_commands.py > rename to scripts/clang-tools/gen_compile_commands.py > diff --git a/scripts/clang-tools/parse_compile_commands.py b/scripts/clang-tools/parse_compile_commands.py > new file mode 100755 > index 000000000000..d6bc1bf9951e > --- /dev/null > +++ b/scripts/clang-tools/parse_compile_commands.py > @@ -0,0 +1,47 @@ > +#!/usr/bin/env python > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Copyright (C) Google LLC, 2019 > +# > +# Author: Nathan Huckleberry <nhuck@xxxxxxxxxx> > +# > +"""A helper routine for make clang-tidy to parse compile_commands.json.""" > + > +import argparse > +import json > +import logging > +import os > +import re > + > +def parse_arguments(): > + """Set up and parses command-line arguments. > + Returns: > + file_path: Path to compile_commands.json file > + """ > + usage = """Parse a compilation database and return a list of files > + included in the database""" > + parser = argparse.ArgumentParser(description=usage) > + > + file_path_help = ('Path to the compilation database to parse') > + parser.add_argument('file', type=str, help=file_path_help) > + > + args = parser.parse_args() > + > + return args.file > + > + > +def main(): > + filename = parse_arguments() > + > + #Read JSON data into the datastore variable > + if filename: > + with open(filename, 'r') as f: > + datastore = json.load(f) > + > + #Use the new datastore datastructure > + for entry in datastore: > + print(entry['file']) > + > + > +if __name__ == '__main__': > + main() And this is nice to have because now there is a real consumer of gen_compile_commands.py. -- Kees Cook
