diff --git a/README.md b/README.md index c55acf7d..67eaf1c2 100644 --- a/README.md +++ b/README.md @@ -21,8 +21,8 @@ The main Bazel rule for CodeChecker is `codechecker_test()`. ### Clang-tidy -Clang-tidy is a fast static analyzer/linter for the C family of languages. This -repository provides Bazel aspect `clang_tidy_aspect()` and rule `clang_tidy_test()` +Clang-tidy is a fast static analyzer/linter for the C family of languages. +This repository provides Bazel rule `clang_tidy_test()` to run clang-tidy natively (without CodeChecker). Find more information about LLVM clang-tidy: @@ -313,9 +313,10 @@ codechecker_test( The following rules are _not_ using CodeChecker. -### Clang-tidy: `clang_tidy_aspect()` and `clang_tidy_test()` +### Clang-tidy: `clang_tidy_test()` -The Bazel rule `clang_tidy_test()` runs clang-tidy natively without CodeChecker. To use it, add the following to your BUILD file: +The Bazel rule `clang_tidy_test()` runs clang-tidy natively without CodeChecker. +To use it, add the following to your BUILD file: ```python load( @@ -331,15 +332,11 @@ clang_tidy_test( ) ``` -You can also run clang-tidy via the Bazel aspect `clang_tidy_aspect()` that can be invoked from the command line by passing the following parameter to Bazel build/test: `--aspects @codechecker_bazel//src:clang.bzl%clang_tidy_aspect`: - -```bash -bazel build ... --aspects @codechecker_bazel//src:clang.bzl%clang_tidy_aspect --output_groups=report -``` - ### Clang Static Analyzer: `clang_analyze_test()` -The Bazel rule `clang_analyze_test()` runs The Clang Static Analyzer natively without CodeChecker. To use it, add the following to your BUILD file: +The Bazel rule `clang_analyze_test()` runs The Clang Static Analyzer +natively without CodeChecker. +To use it, add the following to your BUILD file: ```python load( @@ -355,6 +352,9 @@ clang_analyze_test( ) ``` +> [!Note] +> Currently `clang_analyze_test()` rule does not support CTU (Cross Translation Unit) analysis. + ### Generating a compilation database: `compile_commands()` As generating a compilation database for C/C++ is a known pain point for bazel, this repository defines the Bazel rule `compile_commands()` rule which can be used independently of CodeChecker. The implementation is based on https://github.com/grailbio/bazel-compilation-database with some fixes on some tricky edge cases. To use it, include the following in your BUILD file: @@ -420,7 +420,3 @@ After that you can find all artifacts in `bazel-bin` directory: # compile_commands.json for compile_commands_pass cat bazel-bin/test/compile_commands_pass/compile_commands.json - -To run `clang_tidy_aspect()` on all C/C++ code: - - bazel build ... --aspects @codechecker_bazel//src:clang.bzl%clang_tidy_aspect --output_groups=report diff --git a/src/clang.bzl b/src/clang.bzl index 6436fdf8..51f3d4f5 100644 --- a/src/clang.bzl +++ b/src/clang.bzl @@ -18,30 +18,24 @@ Rulesets for running clang-tidy and the clang static analyzer. load("@bazel_tools//tools/build_defs/cc:action_names.bzl", "ACTION_NAMES") load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") +load("compile_commands.bzl", "platforms_transition") load("common.bzl", "SOURCE_ATTR", "version_specific_attributes") CLANG_TIDY_WRAPPER_SCRIPT = """#!/usr/bin/env bash -CLANG_TIDY=$1 -shift OUTPUT=$1 shift -CONFIG=$1 -shift # Make sure the output exists, and empty if there are no errors, # (clang-tidy doesn't create a patch file if there are no errors). -echo > $OUTPUT +touch $OUTPUT -$CLANG_TIDY --config-file=$CONFIG --export-fixes=$OUTPUT $@ 2>&1 +# echo "$@" +$@ """ CLANG_ANALYZE_WRAPPER_SCRIPT = """#!/usr/bin/env bash -CLANG=$1 -shift -OUTPUT=$1 -shift - -$CLANG --analyze -o $OUTPUT $@ 2>&1 +# echo "$@" +$@ """ def _run_tidy( @@ -49,7 +43,7 @@ def _run_tidy( exe, config, options, - compilation_context, + headers, infile, arguments, label, @@ -65,6 +59,9 @@ def _run_tidy( else: clang_tidy_bin = "clang-tidy" + # If config file is from a filegroup + if config and hasattr(config, "files"): + config = config.files.to_list()[0] # Create clang-tidy config file if not config: config = ctx.actions.declare_file(label + ".clang_tidy_config.yaml") @@ -81,14 +78,17 @@ def _run_tidy( # Prepare arguments args = ctx.actions.args() + # NOTE: we pass output file first and also as an argument below + args.add(outfile.path) + # Add clang-tidy binary args.add(clang_tidy_bin) - # Add output file - args.add(outfile.path) - # Add config file - args.add(config.path) + args.add("--config-file=" + config.path) + + # Add output file + args.add("--export-fixes=" + outfile.path) # Add clang-tidy options if options: @@ -112,7 +112,7 @@ def _run_tidy( input_files.extend(additional_deps.files.to_list()) inputs = depset( direct = input_files, - transitive = [compilation_context.headers], + transitive = [headers], ) ctx.actions.run( inputs = inputs, @@ -130,7 +130,7 @@ def _run_analyzer( exe, config, options, - compilation_context, + headers, infile, arguments, label, @@ -168,12 +168,14 @@ def _run_analyzer( # Add clang binary args.add(clang_bin) - # Add output file - args.add(outfile.path) - # # Add config file # args.add(config.path) + # Mandatory Clang Analyze options + args.add("--analyze") + args.add("-Xclang") + args.add("-analyzer-output=plist-multi-file") + # Add clang options if options: args.add_all(options) @@ -184,6 +186,10 @@ def _run_analyzer( # Add compiler flags -I -D etc args.add_all(arguments) + # Add output file + args.add("-o") + args.add(outfile.path) + input_files = [infile] if config: input_files.append(config) @@ -193,7 +199,7 @@ def _run_analyzer( input_files.extend(additional_deps.files.to_list()) inputs = depset( direct = input_files, - transitive = [compilation_context.headers], + transitive = [headers], ) ctx.actions.run( inputs = inputs, @@ -202,7 +208,7 @@ def _run_analyzer( arguments = [args], mnemonic = "ClangAnalyzer", use_default_shell_env = True, - progress_message = "Run clang -analyze on {}".format(infile.short_path), + progress_message = "Run clang --analyze on {}".format(infile.short_path), ) return outfile @@ -232,11 +238,12 @@ def check_valid_file_type(src): return False def _rule_sources(ctx): - srcs = [] if hasattr(ctx.rule.attr, "srcs"): for src in ctx.rule.attr.srcs: - srcs += [src for src in src.files.to_list() if src.is_source and check_valid_file_type(src)] + for file in src.files.to_list(): + if file.is_source and check_valid_file_type(file): + srcs.append(file) return srcs def _toolchain_flags(ctx, action_name = ACTION_NAMES.cpp_compile): @@ -255,7 +262,11 @@ def _toolchain_flags(ctx, action_name = ACTION_NAMES.cpp_compile): action_name = action_name, variables = compile_variables, ) - return flags + compiler = cc_common.get_tool_for_action( + feature_configuration = feature_configuration, + action_name = action_name, + ).split("/")[-1] + return flags + ["--driver-mode=" + compiler] def _compile_args(compilation_context): compile_args = [] @@ -280,6 +291,9 @@ def _safe_flags(flags): unsupported_flags = [ "-fno-canonical-system-headers", "-fstack-usage", + "-analyze-and-compile", + "-O3", + "-O4", ] return [flag for flag in flags if flag not in unsupported_flags] @@ -303,133 +317,86 @@ def _valid_for_clang_tidy(target, ctx): return False return True -def _clang_tidy_aspect_impl(target, ctx): - if not _valid_for_clang_tidy(target, ctx): - return [] - - exe = ctx.attr._clang_tidy_executable - additional_deps = ctx.attr._clang_tidy_additional_deps - if ctx.attr._clang_tidy_config.files: - config = ctx.attr._clang_tidy_config.files.to_list()[0] - default_options = [] - else: - config = None - default_options = ctx.attr._default_options - compilation_context = target[CcInfo].compilation_context - - rule_flags = ctx.rule.attr.copts if hasattr(ctx.rule.attr, "copts") else [] - c_flags = _safe_flags(_toolchain_flags(ctx, ACTION_NAMES.c_compile) + rule_flags) + ["-xc"] - cxx_flags = _safe_flags(_toolchain_flags(ctx, ACTION_NAMES.cpp_compile) + rule_flags) + ["-xc++"] - compile_args = _compile_args(compilation_context) - c_flags += compile_args - cxx_flags += compile_args - - srcs = _rule_sources(ctx) - - outputs = [ - _run_tidy( - ctx, - exe, - config, - default_options, - compilation_context, - src, - c_flags if src.extension in ["c", "C"] else cxx_flags, - target.label.name, - additional_deps, - ) - for src in srcs - ] - return [ - OutputGroupInfo(report = depset(direct = outputs)), - ] - -clang_tidy_aspect = aspect( - implementation = _clang_tidy_aspect_impl, - fragments = ["cpp"], - attrs = { - "_cc_toolchain": attr.label(default = Label("@bazel_tools//tools/cpp:current_cc_toolchain")), - "_clang_tidy_executable": attr.label(default = Label("@codechecker_bazel//src:clang_tidy_executable")), - "_clang_tidy_additional_deps": attr.label(default = Label("@codechecker_bazel//src:clang_tidy_additional_deps")), - "_clang_tidy_config": attr.label(default = Label("@codechecker_bazel//src:clang_tidy_config")), - "_default_options": attr.string_list(default = ["--use-color", "--warnings-as-errors=*"]), - }, - toolchains = ["@bazel_tools//tools/cpp:toolchain_type"], -) - CompileInfo = provider( doc = "Source files and corresponding compilation arguments", fields = { "arguments": "dict: file -> list of arguments", + "headers": "list: header files", }, ) -def _compile_info_sources(deps): - sources = [] - if type(deps) == "list": - for dep in deps: - if CompileInfo in dep: - if hasattr(dep[CompileInfo], "arguments"): - srcs = dep[CompileInfo].arguments.keys() - sources += srcs - return sources - -def _collect_all_sources(ctx): - sources = _rule_sources(ctx) +def _process_all_deps(ctx, arguments, headers): for attr in SOURCE_ATTR: if hasattr(ctx.rule.attr, attr): deps = getattr(ctx.rule.attr, attr) - sources += _compile_info_sources(deps) - - # Remove duplicates - sources = depset(sources).to_list() - return sources + for dep in deps: + if CompileInfo in dep: + if hasattr(dep[CompileInfo], "arguments"): + arguments.update(dep[CompileInfo].arguments) + if hasattr(dep[CompileInfo], "headers"): + headers += dep[CompileInfo].headers.to_list() + + # # NOTE: Alternative implementation + # for attr in dir(ctx.rule.attr): + # deps = getattr(ctx.rule.attr, attr) + # if type(deps) == "list": + # for dep in deps: + # if type(dep) == "Target" and CompileInfo in dep: + # if hasattr(dep[CompileInfo], "arguments"): + # arguments.update(dep[CompileInfo].arguments) + # if hasattr(dep[CompileInfo], "headers"): + # headers += dep[CompileInfo].headers.to_list() + +def _get_sources(ctx): + sources = _rule_sources(ctx) + arguments = { src: [] for src in sources } + headers = [] + _process_all_deps(ctx, arguments, headers) + return arguments.keys() def _compile_info_aspect_impl(target, ctx): - if not _valid_for_clang_tidy(target, ctx): - return [] - - compilation_context = target[CcInfo].compilation_context - - rule_flags = ctx.rule.attr.copts if hasattr(ctx.rule.attr, "copts") else [] - c_flags = _safe_flags(_toolchain_flags(ctx, ACTION_NAMES.c_compile) + rule_flags) # + ["-xc"] - cxx_flags = _safe_flags(_toolchain_flags(ctx, ACTION_NAMES.cpp_compile) + rule_flags) # + ["-xc++"] - - srcs = _collect_all_sources(ctx) - - compile_args = _compile_args(compilation_context) arguments = {} - for src in srcs: - flags = c_flags if src.extension in ["c", "C"] else cxx_flags - arguments[src] = compile_args + flags + headers = [] + _process_all_deps(ctx, arguments, headers) + + if _valid_for_clang_tidy(target, ctx): + compilation_context = target[CcInfo].compilation_context + headers += compilation_context.headers.to_list() + rule_flags = ctx.rule.attr.copts if hasattr(ctx.rule.attr, "copts") else [] + c_flags = _safe_flags(_toolchain_flags(ctx, ACTION_NAMES.c_compile) + rule_flags) # + ["-xc"] + cxx_flags = _safe_flags(_toolchain_flags(ctx, ACTION_NAMES.cpp_compile) + rule_flags) # + ["-xc++"] + srcs = _get_sources(ctx) + compile_args = _compile_args(compilation_context) + for src in srcs: + flags = c_flags if src.extension in ["c", "C"] else cxx_flags + arguments[src] = compile_args + flags + return [ CompileInfo( arguments = arguments, + headers = depset(headers), ), ] compile_info_aspect = aspect( implementation = _compile_info_aspect_impl, fragments = ["cpp"], + attr_aspects = SOURCE_ATTR, + toolchains = ["@bazel_tools//tools/cpp:toolchain_type"], attrs = { "_cc_toolchain": attr.label(default = Label("@bazel_tools//tools/cpp:current_cc_toolchain")), }, - attr_aspects = ["srcs", "deps", "data", "exports"], - toolchains = ["@bazel_tools//tools/cpp:toolchain_type"], ) def _clang_test(ctx, tool): all_files = [] - - # headers = depset() + all_headers = depset() for target in ctx.attr.targets: - if not CcInfo in target: - continue if CompileInfo in target: if hasattr(target[CompileInfo], "arguments"): srcs = target[CompileInfo].arguments.keys() + headers = target[CompileInfo].headers all_files += srcs - compilation_context = target[CcInfo].compilation_context for src in srcs: arguments = target[CompileInfo].arguments[src] report = tool( @@ -437,13 +404,13 @@ def _clang_test(ctx, tool): ctx.attr.executable, ctx.attr.config_file, ctx.attr.default_options + ctx.attr.options, - compilation_context, + headers, src, arguments, ctx.attr.name, ) all_files.append(report) - # headers = depset(transitive = [headers, compilation_context.headers]) + all_headers = depset(transitive = [all_headers, headers]) ctx.actions.write( output = ctx.outputs.test_script, @@ -452,7 +419,7 @@ def _clang_test(ctx, tool): ) files = depset( direct = all_files, - # transitive = [headers], + transitive = [all_headers], ) run_files = [ctx.outputs.test_script] + files.to_list() return [ @@ -469,10 +436,15 @@ def _clang_tidy_test_impl(ctx): clang_tidy_test = rule( implementation = _clang_tidy_test_impl, attrs = { + "platform": attr.string( + default = "", #"@platforms//os:linux", + doc = "Platform to build for", + ), "targets": attr.label_list( aspects = [ compile_info_aspect, ], + cfg = platforms_transition, doc = "List of compilable targets which should be checked.", ), "options": attr.string_list( @@ -481,6 +453,7 @@ clang_tidy_test = rule( ), "default_options": attr.string_list( default = [ + "--quiet", "--use-color", "--warnings-as-errors=*", # "--header-filter=.*", @@ -500,7 +473,7 @@ clang_tidy_test = rule( cfg = "exec", doc = "Clang-tidy executable", ), - }, + } | version_specific_attributes(), outputs = { "test_script": "%{name}.test_script.sh", }, @@ -513,10 +486,15 @@ def _clang_analyze_test_impl(ctx): clang_analyze_test = rule( implementation = _clang_analyze_test_impl, attrs = { + "platform": attr.string( + default = "", #"@platforms//os:linux", + doc = "Platform to build for", + ), "targets": attr.label_list( aspects = [ compile_info_aspect, ], + cfg = platforms_transition, doc = "List of compilable targets which should be checked.", ), "options": attr.string_list( @@ -526,7 +504,15 @@ clang_analyze_test = rule( "default_options": attr.string_list( default = [ "-fcolor-diagnostics", + "-Qunused-arguments", "-Xanalyzer -analyzer-werror", + "-Xclang -analyzer-opt-analyze-headers", + "-Xclang -analyzer-config -Xclang expand-macros=true", + "-Xclang -analyzer-config -Xclang aggressive-binary-operation-simplification=true", + "-Xclang -analyzer-config -Xclang crosscheck-with-z3=true", + # "-Xclang -analyzer-config -Xclang experimental-enable-naive-ctu-analysis=true", + # "-Xclang -analyzer-config -Xclang ctu-dir=.../data/ctu-dir/${platform}", + # "-Xclang -analyzer-config -Xclang display-ctu-progress=true", ], doc = "List of default clang options", ), @@ -542,7 +528,7 @@ clang_analyze_test = rule( cfg = "exec", doc = "Clang executable", ), - }, + } | version_specific_attributes(), outputs = { "test_script": "%{name}.test_script.sh", }, diff --git a/test/unit/legacy/test_legacy.py b/test/unit/legacy/test_legacy.py index 83986d50..f579b4f9 100644 --- a/test/unit/legacy/test_legacy.py +++ b/test/unit/legacy/test_legacy.py @@ -130,22 +130,6 @@ def test_bazel_compile_commands(self): self.grep_file(compile_commands, r"pass\.cc") self.grep_file(compile_commands, r"\/gcc") - def test_bazel_aspect_clang_tidy_pass(self): - """Test: bazel build :test_pass --aspects""" - command = "bazel build :test_pass " + \ - "--aspects @codechecker_bazel//src:clang.bzl%clang_tidy_aspect" + \ - " --output_groups=report" - self.check_command(command, exit_code=0) - - def test_bazel_aspect_clang_tidy_fail(self): - """Test: bazel build :test_lib --aspects""" - # NOTE: we should use :test_fail but transitive dependencies do not - # work - command = "bazel build :test_lib " + \ - "--aspects @codechecker_bazel//src:clang.bzl%clang_tidy_aspect" + \ - " --output_groups=report" - self.check_command(command, exit_code=1) - def test_bazel_build_clang_tidy_pass(self): """Test: bazel build :clang_tidy_pass""" self.check_command("bazel build :clang_tidy_pass", exit_code=0)