diff --git a/Makefile b/Makefile index 05adda0e7..8bcd4b2f6 100644 --- a/Makefile +++ b/Makefile @@ -170,6 +170,7 @@ help: ' ANALYZER_FORMAT=text|json policy output format (default: text)' \ ' sparse, smatch analyze configured modules through Kbuild' \ ' coccicheck run kernel Coccinelle report rules' \ + ' cppcheck analyze fresh kernel and userspace build contexts' \ ' DIFFBR=commit compare with one commit; select relevant inputs' \ ' lint-test test shared analyzer tooling (no modules)' \ ' all make all' \ @@ -279,6 +280,19 @@ coccicheck: export SCST_LINT_KDIR := $(KDIR) coccicheck: +@$(PYTHON) -B scripts/coccinelle/coccicheck.py +.PHONY: cppcheck lint-userspace +cppcheck: export SCST_LINT_KDIR := $(KDIR) +cppcheck: export SCST_LINT_CC := $(CC) +cppcheck: + +@$(PYTHON) -B scripts/cppcheck/cppcheck.py + +lint-userspace: + +$(MAKE) -C scst/src ../include/scst_itf_ver.h + +$(MAKE) -C $(ISCSI_DIR) progs + +$(MAKE) -C usr/fileio + +$(MAKE) -C usr/stpgd + +$(MAKE) -C usr/events + # Preserve dependency order. The newer QLogic base is compiled for its target # module, but only sources selected by analyzer policy reach the checker. lint-kbuild: diff --git a/scripts/README.md b/scripts/README.md index 9fdac9e12..14f85ef4f 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -14,6 +14,7 @@ direct callers of a relocated script must use its new directory below. | `policy/` | Reviewed shared analyzer input policy; exclusions carry their reasons. | | `sparse_smatch/` | Native Kbuild analyzers (`make sparse`, `make smatch`) and CHECK records. | | `coccinelle/` | Kernel semantic-patch report rules (`make coccicheck`). | +| `cppcheck/` | Native analysis using captured compiler commands (`make cppcheck`). | | `tests/` | Isolated regression fixtures for the shared analyzer helpers (`make lint-test`). | The existing `checkpatch`, `checkpatch_commits`, `checkpatch_diff`, and @@ -79,7 +80,7 @@ python3 -B scripts/analyzer_policy.py validate ``` The Python commands also accept `--format=json` and a policy name: `common`, -`sparse`, `smatch` or `coccicheck`. Use `make analyzer-policy ANALYZER=smatch` to see the +`sparse`, `smatch`, `coccicheck` or `cppcheck`. Use `make analyzer-policy ANALYZER=smatch` to see the effective input scope and all diagnostic exceptions. A file inventory does not establish analysis or build coverage. @@ -208,6 +209,29 @@ incomplete analysis, even when native `spatch` returns zero. Findings return 1; incomplete analysis returns 2 (Make maps either failure to 2). Full output and per-rule coverage are in `.cache/lint/coccicheck.{log,json}`. +## Cppcheck + +```sh +make -j8 cppcheck KDIR=/path/to/kernel/build KUNIT_TEST=y +make -j8 cppcheck KDIR=/path/to/kernel/build KUNIT_TEST=y DIFFBR=master +``` + +The target requires Cppcheck with `--check-level` and `--checkers-report` +support, a prepared kernel and SCST build dependencies. It captures fresh +Kbuild `CHECK` commands and rebuilds userspace programs (including the event +example) to collect their compiler commands. It then runs kernel and userspace +analysis separately with all native checks enabled. Nothing is installed or +loaded. KUnit coverage follows `KUNIT_TEST` and the kernel configuration. + +A relevant diff triggers the full configured analysis, including header +consumers. Explicit or changed C files absent from the build cause a failure. +Real includes and definitions are preserved; no project models, header +substitutions, inline suppressions or diagnostic exceptions are enabled. +Native parser failures and missing includes remain visible as incomplete +analysis. Source findings return 1 and incomplete analysis returns 2; Make +maps both to 2. Full logs and the summary are in `.cache/lint/cppcheck.{log,json}`; +the summary links to captured commands, native XML and checker coverage. + ## Dependencies and tooling tests Shared analyzer tooling requires Linux, Git and Python 3.10+, using only the diff --git a/scripts/analyzer_policy.py b/scripts/analyzer_policy.py index 373bd86d0..159cb2ce1 100644 --- a/scripts/analyzer_policy.py +++ b/scripts/analyzer_policy.py @@ -24,8 +24,8 @@ KBUILD_SCHEMA = { "kernel_exceptions": [{"header": str, "message": str, "reason": str}], } BASIC_SCHEMA = {"version": int, "notes": [str]} -POLICIES = ("common", "sparse", "smatch", "coccicheck") -BASIC_POLICIES = ("coccicheck",) +POLICIES = ("common", "sparse", "smatch", "coccicheck", "cppcheck") +BASIC_POLICIES = ("coccicheck", "cppcheck") def relative_path(value: str, location: str): diff --git a/scripts/cppcheck/capture.py b/scripts/cppcheck/capture.py new file mode 100644 index 000000000..ab9c37fbe --- /dev/null +++ b/scripts/cppcheck/capture.py @@ -0,0 +1,55 @@ +#!/usr/bin/env python3 +"""Record Kbuild CHECK arguments or successful userspace compiler invocations.""" + +import json +import os +from pathlib import Path +import shlex +import sys +import uuid + +sys.dont_write_bytecode = True +ROOT = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(ROOT / "scripts")) +from lint_common import entrypoint, run # noqa: E402 +from lint_scope import in_scope # noqa: E402 + + +def source_files(arguments, profile, root): + if profile == "kernel": + candidates = arguments[-1:] + elif "-c" in arguments: + candidates = [arg for arg in arguments if arg.endswith(".c") and not arg.startswith("-")] + else: + return [] + return [Path(arg).resolve() for arg in candidates + if Path(arg).is_file() and in_scope(Path(arg), root, "cppcheck")] + + +def main(): + profile, *arguments = sys.argv[1:] + if profile not in {"kernel", "userspace"}: + raise ValueError(f"unknown capture profile: {profile}") + sources = source_files(arguments, profile, ROOT) + compiler = shlex.split(os.environ.get("SCST_LINT_CC", "cc")) + if not compiler: + raise ValueError("empty userspace compiler command") + # Kernel CHECK receives the real c_flags. It only records; Kbuild still + # invokes its own compiler. Do not run Sparse or add __CHECKER__ definitions. + command = (["cc"] if profile == "kernel" else compiler) + arguments + records = [] + for source in sources: + path = Path(os.environ["SCST_LINT_RECORDS"]) / f"{uuid.uuid4()}.json" + record = {"file": str(source), "directory": str(Path.cwd()), "arguments": command, + "profile": profile, "returncode": 2} + path.write_text(json.dumps(record) + "\n") + records.append((path, record)) + status = run(command).returncode if profile == "userspace" else 0 + for path, record in records: + record["returncode"] = status + path.write_text(json.dumps(record) + "\n") + return status + + +if __name__ == "__main__": + sys.exit(entrypoint(main)) diff --git a/scripts/cppcheck/cppcheck.py b/scripts/cppcheck/cppcheck.py new file mode 100644 index 000000000..e9befa278 --- /dev/null +++ b/scripts/cppcheck/cppcheck.py @@ -0,0 +1,208 @@ +#!/usr/bin/env python3 +"""Run Cppcheck on fresh kernel and userspace commands without project models.""" + +import argparse +import json +import os +from pathlib import Path +import re +import shlex +import shutil +import subprocess +import sys +import tempfile +import xml.etree.ElementTree as ET + +sys.dont_write_bytecode = True +HERE = Path(__file__).resolve().parent +ROOT = HERE.parents[1] +sys.path.insert(0, str(ROOT / "scripts")) +from lint_common import copy_output, entrypoint, log_path, run, show_log # noqa: E402 +from lint_kbuild import build_lock, collect, unchecked_invocations # noqa: E402 +from lint_scope import select_sources # noqa: E402 + +INCOMPLETE = {"syntaxError", "unknownMacro", "internalAstError", "internalError", + "preprocessorErrorDirective", "noValidConfiguration", "cppcheckError", + "directiveAsMacroParameter", "missingInclude", "missingIncludeSystem", + "toomanyconfigs"} + + +def native_result(xml, progress, checkers, status, expected, root): + failures = [] + diagnostics = [] + try: + document = ET.fromstring(xml) + if document.tag != "results" or document.get("version") != "2" or ( + document.find("cppcheck") is None or document.find("errors") is None): + raise ValueError("unexpected Cppcheck XML structure") + for error in document.findall("errors/error"): + if not error.get("id") or not error.get("severity") or error.get("msg") is None: + raise ValueError("incomplete Cppcheck diagnostic") + diagnostics.append({**error.attrib, + "locations": [location.attrib for location in error.findall("location")]}) + if error.get("id") in INCOMPLETE: + failures.append(f"{error.get('id')}: {error.get('msg')}") + except (ET.ParseError, ValueError) as error: + failures.append(f"invalid or incomplete XML report: {error}") + if status not in {0, 3} or (status == 3 and not diagnostics): + failures.append(f"unexpected native status: {status}") + if ("No critical errors encountered." not in checkers or + "There were critical errors" in checkers or + not re.search(r"^Yes\s+\S+", checkers, re.MULTILINE)): + failures.append("missing checker coverage or critical native errors") + checked = set() + for line in progress.splitlines(): + match = re.match(r"^Checking (.+?\.c)(?: |:).*\.\.\.$", line) + if match: + checked.add((root / match[1]).resolve()) + if re.match(r"^(?:cppcheck: (?:error|warning):|Failed to|Internal error)", line): + failures.append(line) + missing = sorted(str(path) for path in set(expected) - checked) + failures.extend(f"no native checking record for {path}" for path in missing) + return {"native_status": status, "diagnostics": diagnostics, "failures": failures, + "checked_files": sorted(str(path) for path in checked & set(expected)), + "remaining": len(diagnostics), "complete": not failures} + + +def capture_userspace(directory, log): + env = dict(os.environ, SCST_LINT_RECORDS=str(directory)) + compiler = shlex.join([sys.executable, str(HERE / "capture.py"), "userspace"]) + command = ["make", "--no-silent", "-k", "-B", "-C", str(ROOT), "lint-userspace", + "CC=" + compiler] + with log.open("ab") as stream: + result = run(command, env=env, stdout=stream, stderr=subprocess.STDOUT, + progress="Cppcheck userspace build context") + records = [json.loads(path.read_text()) for path in sorted(directory.glob("*.json"))] + return result.returncode, records + + +def project_entry(record): + """Resolve paths from the compiler cwd, not the analyzer's cwd. + + Native project import leaves forced includes relative and does not use + -isystem paths in its own frontend. Pass the same real headers via -I. + Keep the original command untouched in the capture record. + """ + directory = Path(record["directory"]) + arguments = iter(record["arguments"]) + normalized = [] + system_includes = [] + for argument in arguments: + option = next((flag for flag in ("-isystem", "-include", "-I") + if argument.startswith(flag)), None) + if option is None: + normalized.append(argument) + continue + value = argument[len(option):] or next(arguments, "") + if not value: + raise ValueError(f"missing operand for compiler option {option}") + path = directory / value + if option != "-include" or path.is_file(): + value = str(path.resolve()) + if option == "-isystem": + system_includes.extend(["-I", value]) + else: + normalized.extend([option, value]) + # GCC searches explicit system directories after every ordinary -I path. + normalized.extend(system_includes) + return {"file": record["file"], "directory": str(directory), "arguments": normalized} + + +def analyze(profile, records, directory, log): + project = directory / f"{profile}.json" + # Keep each distinct compiler context, including repeated files with + # different definitions. Do not strip include paths or rewrite headers. + commands = {json.dumps(project_entry(row), sort_keys=True) for row in records} + project.write_text("[\n" + ",\n".join(sorted(commands)) + "\n]\n") + stdout = directory / f"{profile}.stdout" + xml = directory / f"{profile}.xml" + checkers = directory / f"{profile}.checkers" + checkers.write_text("") + command = ["cppcheck", "--project=" + str(project), "--enable=all", "--inconclusive", + "--check-level=exhaustive", "--force", "--xml", "--xml-version=2", + "--error-exitcode=3", "--checkers-report=" + str(checkers)] + try: + with stdout.open("wb") as output, xml.open("wb") as errors: + result = run(command, cwd=ROOT, stdout=output, stderr=errors, + env=dict(os.environ, NO_COLOR="1"), progress=f"Cppcheck {profile}") + finally: + with log.open("ab") as stream: + stream.write(f"\nCppcheck {profile}: {shlex.join(command)}\n".encode()) + for path in (stdout, xml, checkers): + if path.exists(): + copy_output(path, stream) + record = native_result(xml.read_text(errors="replace"), stdout.read_text(errors="replace"), + checkers.read_text(errors="replace"), result.returncode, + [Path(row["file"]) for row in records], ROOT) + return {"profile": profile, "translation_units": len(commands), **record} + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--base", default=os.environ.get("DIFFBR") or None) + parser.add_argument("files", nargs="*", type=Path) + args = parser.parse_args() + with build_lock(ROOT): + log = log_path(ROOT, "cppcheck.log") + report = log.with_suffix(".json") + summary = {"tool": "cppcheck", "run_id": os.environ.get("SCST_LINT_RUN_ID"), + "outcome": "incomplete", "exit_code": 2, "profiles": [], + "remaining": 0, "checked_files": []} + report.write_text(json.dumps(summary, indent=2) + "\n") + show_log(log) + try: + selected = select_sources(ROOT, args.files, args.base, "cppcheck") + summary["selected_files"] = [p.relative_to(ROOT).as_posix() for p in selected] + if not selected: + print("No eligible Cppcheck inputs; analysis skipped.") + summary.update(outcome="skipped", exit_code=0) + return 0 + if not shutil.which("cppcheck"): + raise ValueError("cppcheck is not installed") + with log.open("ab") as stream: + result = run(["cppcheck", "--version"], stdout=stream, stderr=subprocess.STDOUT) + if result.returncode: + raise ValueError(f"cppcheck version probe failed: {result.returncode}") + # Fresh records prevent stale incremental-build state from implying + # coverage. Retain commands and native reports for diagnosis. + directory = Path(tempfile.mkdtemp(prefix="cppcheck-", dir=log.parent)) + summary["records_directory"] = str(directory) + kernel_records = directory / "kernel" + user_records = directory / "userspace" + kernel_records.mkdir() + user_records.mkdir() + checker = [sys.executable, str(HERE / "capture.py"), "kernel"] + kernel_status, kernel = collect(ROOT, "cppcheck", kernel_records, log, checker=checker) + missing_checks = unchecked_invocations(ROOT, "cppcheck", log, kernel) + user_status, userspace = capture_userspace(user_records, log) + records = kernel + userspace + captured = {Path(row["file"]) for row in records} + missing = ([str(path) for path in selected if path.suffix == ".c" and path not in captured] + if args.files or args.base else []) + summary.update(kernel_make_status=kernel_status, userspace_make_status=user_status, + unchecked_files=missing, unrecorded_checks=missing_checks) + if (kernel_status or user_status or not kernel or not userspace or missing or + missing_checks or any(row["returncode"] for row in records)): + raise ValueError("incomplete Cppcheck build context; inspect the build log and summary") + for profile, rows in (("kernel", kernel), ("userspace", userspace)): + result = analyze(profile, rows, directory, log) + summary["profiles"].append(result) + summary["remaining"] += result["remaining"] + report.write_text(json.dumps(summary, indent=2) + "\n") + if any(not row["complete"] for row in summary["profiles"]): + print("Cppcheck analysis is incomplete; inspect the full native reports.", file=sys.stderr) + return 2 + summary["checked_files"] = sorted(path.relative_to(ROOT).as_posix() for path in captured) + status = int(bool(summary["remaining"])) + summary.update(outcome="findings" if status else "clean", exit_code=status) + print(f"Cppcheck: {len(captured)} files, {summary['remaining']} diagnostic(s).") + return status + finally: + report.write_text(json.dumps(summary, indent=2) + "\n") + if os.environ.get("LINT_DEFER_REPORTS") != "1": + copy_output(log) + show_log(log) + + +if __name__ == "__main__": + sys.exit(entrypoint(main)) diff --git a/scripts/lint_kbuild.py b/scripts/lint_kbuild.py index 993b6f094..cea2cd573 100644 --- a/scripts/lint_kbuild.py +++ b/scripts/lint_kbuild.py @@ -54,10 +54,11 @@ def validate_checker(root, tool, log): raise ValueError(f"{tool} is incompatible with {probe}; module build was not started") -def collect(root, tool, directory, log): +def collect(root, tool, directory, log, *, checker=None): """Retain CHECK exits even when the enclosing Kbuild discards them.""" env = dict(os.environ, SCST_LINT_RECORDS=str(directory)) - checker = shlex.join([sys.executable, str(root / "scripts/sparse_smatch/check.py"), tool]) + checker = shlex.join(checker or + [sys.executable, str(root / "scripts/sparse_smatch/check.py"), tool]) command = ["make", "--no-silent", "-k", "-C", str(root), "lint-kbuild", "C=2", "V=0", "CHECK=" + checker] with log.open("ab") as output: diff --git a/scripts/policy/cppcheck.json b/scripts/policy/cppcheck.json new file mode 100644 index 000000000..c27d91c51 --- /dev/null +++ b/scripts/policy/cppcheck.json @@ -0,0 +1,9 @@ +{ + "version": 1, + "notes": [ + "Capture fresh kernel CHECK and userspace compiler commands.", + "Analyze configured translation units with native headers and definitions.", + "KUNIT_TEST=y includes KUnit sources through their actual Kbuild context.", + "No project models, inline suppressions or diagnostic exceptions." + ] +} diff --git a/scripts/tests/test_analyzer_policy.py b/scripts/tests/test_analyzer_policy.py index 1b1532da8..c5d76bc14 100644 --- a/scripts/tests/test_analyzer_policy.py +++ b/scripts/tests/test_analyzer_policy.py @@ -156,7 +156,8 @@ class PolicyCliTests(unittest.TestCase): (self.scripts / "policy").mkdir(parents=True) source = analyzer_policy.POLICY_PATH.parent.parent for name in ("analyzer_policy.py", "lint_scope.py", "policy/common.json", - "policy/sparse.json", "policy/smatch.json"): + "policy/sparse.json", "policy/smatch.json", "policy/coccicheck.json", + "policy/cppcheck.json"): shutil.copyfile(source / name, self.scripts / name) def run_cli(self, *args): @@ -211,7 +212,7 @@ class PolicyCliTests(unittest.TestCase): "usr", "iscsi-scst/usr"}) def test_configuration_and_inventory_errors_fail_visibly(self): - unknown = self.run_cli("show", "cppcheck") + unknown = self.run_cli("show", "unknown-tool") self.assertEqual(unknown.returncode, 2) no_git = self.run_cli("files", "--format=json") self.assertEqual(no_git.returncode, 2) @@ -230,6 +231,16 @@ class PolicyCliTests(unittest.TestCase): self.assertEqual(broken.stdout, "") self.assertIn("analyzer policy:", broken.stderr) + def test_basic_analyzers_inherit_common_scope_without_exceptions(self): + common = json.loads((self.scripts / "policy/common.json").read_text()) + for tool in ("coccicheck", "cppcheck"): + with self.subTest(tool=tool): + result = self.run_cli("show", tool, "--format=json") + self.assertEqual(result.returncode, 0, result.stderr) + report = json.loads(result.stdout) + self.assertEqual(report["effective_source_policy"], common) + self.assertEqual(set(report["configured_policy"]), {"version", "notes"}) + if __name__ == "__main__": unittest.main() diff --git a/scripts/tests/test_cppcheck.py b/scripts/tests/test_cppcheck.py new file mode 100644 index 000000000..25892520c --- /dev/null +++ b/scripts/tests/test_cppcheck.py @@ -0,0 +1,99 @@ +"""Cppcheck report/capture fixtures without native analysis or compilation.""" + +import json +from pathlib import Path +import sys +import tempfile +import unittest + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from analyzer_policy import read_policy +from cppcheck.capture import source_files +from cppcheck.cppcheck import native_result, project_entry + + +class CppcheckTests(unittest.TestCase): + def setUp(self): + self.source = Path("/tmp/scst/scst/src/file.c") + self.progress = f"Checking {self.source} ...\n" + self.checkers = "No critical errors encountered.\nYes CheckOther::checkSomething\n" + + def report(self, diagnostics="", status=0, progress=None, checkers=None): + xml = ('' + f'{diagnostics}') + return native_result(xml, self.progress if progress is None else progress, + self.checkers if checkers is None else checkers, + status, [self.source], Path("/tmp/scst")) + + def test_valid_empty_report_requires_native_coverage(self): + self.assertTrue(self.report()["complete"]) + for kwargs in ({"progress": ""}, {"checkers": ""}, {"status": 1}, {"status": 3}, + {"checkers": "There were critical errors (syntaxError).\nYes Check\n"}): + with self.subTest(kwargs=kwargs): + self.assertFalse(self.report(**kwargs)["complete"]) + + def test_relative_progress_and_configuration_suffix(self): + self.assertTrue(self.report(progress="Checking scst/src/file.c: CONFIG_X=1...\n")["complete"]) + + def test_findings_are_preserved(self): + diagnostic = '' + result = self.report(diagnostic, 3) + self.assertTrue(result["complete"]) + self.assertEqual(result["remaining"], 1) + self.assertEqual(result["diagnostics"][0]["id"], "nullPointer") + + def test_native_zero_with_parser_failure_is_incomplete(self): + for identifier in ("syntaxError", "unknownMacro", "missingIncludeSystem", "internalError"): + with self.subTest(identifier=identifier): + diagnostic = f'' + self.assertFalse(self.report(diagnostic)["complete"]) + + def test_malformed_and_partial_xml_are_incomplete(self): + for xml in ("", "", '', + ''): + with self.subTest(xml=xml): + self.assertFalse(native_result(xml, self.progress, self.checkers, 0, + [self.source], Path("/tmp/scst"))["complete"]) + + def test_capture_requires_compile_and_shared_scope(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + source = root / "usr/fileio/file.c" + source.parent.mkdir(parents=True) + source.touch() + self.assertEqual(source_files(["-c", str(source)], "userspace", root), [source]) + self.assertEqual(source_files(["-M", str(source)], "userspace", root), []) + self.assertEqual(source_files(["-Iinclude", str(source)], "kernel", root), [source]) + outside = root / "qla2x00t/legacy.c" + outside.parent.mkdir() + outside.touch() + self.assertEqual(source_files([str(outside)], "kernel", root), []) + + def test_basic_policy_rejects_models_and_suppressions(self): + with tempfile.TemporaryDirectory() as temporary: + path = Path(temporary) / "policy.json" + policy = {"version": 1, "notes": ["Native commands."]} + path.write_text(json.dumps(policy)) + self.assertEqual(read_policy(path, "cppcheck"), policy) + for field in ("models", "suppressions", "source_exceptions"): + path.write_text(json.dumps({**policy, field: []})) + with self.subTest(field=field), self.assertRaises(ValueError): + read_policy(path, "cppcheck") + + def test_project_retains_real_headers_relative_to_compiler_directory(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + (root / "include").mkdir() + (root / "include/config.h").touch() + arguments = ["cc", "-Iinclude", "-isystem", "system", "-include", "include/config.h", + "-D__KERNEL__", "-c", "file.c"] + record = {"directory": str(root), "file": str(root / "file.c"), "arguments": arguments} + entry = project_entry(record) + self.assertEqual(entry["arguments"], ["cc", "-I", str(root / "include"), + "-include", str(root / "include/config.h"), + "-D__KERNEL__", "-c", "file.c", "-I", str(root / "system")]) + self.assertEqual(record["arguments"], arguments) + + +if __name__ == "__main__": + unittest.main()