From 46a9c1b6b2267c32bd23b52813769b58401deccf Mon Sep 17 00:00:00 2001 From: Valerii Koval Date: Thu, 23 Jan 2020 12:57:54 +0200 Subject: [PATCH] Add initial support for PVS-Studio check tool (#3357) * Add initial support for PVS-Studio check tool * Enable all available PVS-Studio analyzers by default * Add tests for PVS-Studio check tool * Improve handling check tool extra flags that contain colon symbol --- platformio/commands/check/tools/__init__.py | 3 + platformio/commands/check/tools/base.py | 18 +- platformio/commands/check/tools/clangtidy.py | 2 +- platformio/commands/check/tools/cppcheck.py | 4 +- platformio/commands/check/tools/pvsstudio.py | 232 +++++++++++++++++++ platformio/managers/core.py | 1 + platformio/project/options.py | 2 +- tests/commands/test_check.py | 45 +++- 8 files changed, 294 insertions(+), 13 deletions(-) create mode 100644 platformio/commands/check/tools/pvsstudio.py diff --git a/platformio/commands/check/tools/__init__.py b/platformio/commands/check/tools/__init__.py index 4c8a5e72..9c4b1b7e 100644 --- a/platformio/commands/check/tools/__init__.py +++ b/platformio/commands/check/tools/__init__.py @@ -15,6 +15,7 @@ from platformio import exception from platformio.commands.check.tools.clangtidy import ClangtidyCheckTool from platformio.commands.check.tools.cppcheck import CppcheckCheckTool +from platformio.commands.check.tools.pvsstudio import PvsStudioCheckTool class CheckToolFactory(object): @@ -25,6 +26,8 @@ class CheckToolFactory(object): cls = CppcheckCheckTool elif tool == "clangtidy": cls = ClangtidyCheckTool + elif tool == "pvs-studio": + cls = PvsStudioCheckTool else: raise exception.PlatformioException("Unknown check tool `%s`" % tool) return cls(project_dir, config, envname, options) diff --git a/platformio/commands/check/tools/base.py b/platformio/commands/check/tools/base.py index 4c381a05..a2cf5940 100644 --- a/platformio/commands/check/tools/base.py +++ b/platformio/commands/check/tools/base.py @@ -27,10 +27,13 @@ class CheckToolBase(object): # pylint: disable=too-many-instance-attributes self.config = config self.envname = envname self.options = options - self.cpp_defines = [] - self.cpp_flags = [] + self.cc_flags = [] + self.cxx_flags = [] self.cpp_includes = [] - + self.cpp_defines = [] + self.toolchain_defines = [] + self.cc_path = None + self.cxx_path = None self._defects = [] self._on_defect_callback = None self._bad_input = False @@ -53,16 +56,19 @@ class CheckToolBase(object): # pylint: disable=too-many-instance-attributes data = load_project_ide_data(project_dir, envname) if not data: return - self.cpp_flags = data.get("cxx_flags", "").split(" ") + self.cc_flags = data.get("cc_flags", "").split(" ") + self.cxx_flags = data.get("cxx_flags", "").split(" ") self.cpp_includes = data.get("includes", []) self.cpp_defines = data.get("defines", []) - self.cpp_defines.extend(self._get_toolchain_defines(data.get("cc_path"))) + self.cc_path = data.get("cc_path") + self.cxx_path = data.get("cxx_path") + self.toolchain_defines = self._get_toolchain_defines(self.cc_path) def get_flags(self, tool): result = [] flags = self.options.get("flags") or [] for flag in flags: - if ":" not in flag: + if ":" not in flag or flag.startswith("-"): result.extend([f for f in flag.split(" ") if f]) elif flag.startswith("%s:" % tool): result.extend([f for f in flag.split(":", 1)[1].split(" ") if f]) diff --git a/platformio/commands/check/tools/clangtidy.py b/platformio/commands/check/tools/clangtidy.py index b7845f8b..4efc00a9 100644 --- a/platformio/commands/check/tools/clangtidy.py +++ b/platformio/commands/check/tools/clangtidy.py @@ -61,7 +61,7 @@ class ClangtidyCheckTool(CheckToolBase): cmd.extend(self.get_project_target_files()) cmd.append("--") - cmd.extend(["-D%s" % d for d in self.cpp_defines]) + cmd.extend(["-D%s" % d for d in self.cpp_defines + self.toolchain_defines]) cmd.extend(["-I%s" % inc for inc in self.cpp_includes]) return cmd diff --git a/platformio/commands/check/tools/cppcheck.py b/platformio/commands/check/tools/cppcheck.py index 3d92bc56..840568bd 100644 --- a/platformio/commands/check/tools/cppcheck.py +++ b/platformio/commands/check/tools/cppcheck.py @@ -112,12 +112,12 @@ class CppcheckCheckTool(CheckToolBase): cmd.append("--language=c++") if not self.is_flag_set("--std", flags): - for f in self.cpp_flags: + for f in self.cxx_flags + self.cc_flags: if "-std" in f: # Standards with GNU extensions are not allowed cmd.append("-" + f.replace("gnu", "c")) - cmd.extend(["-D%s" % d for d in self.cpp_defines]) + cmd.extend(["-D%s" % d for d in self.cpp_defines + self.toolchain_defines]) cmd.extend(flags) cmd.append("--file-list=%s" % self._generate_src_file()) diff --git a/platformio/commands/check/tools/pvsstudio.py b/platformio/commands/check/tools/pvsstudio.py new file mode 100644 index 00000000..bce4ae14 --- /dev/null +++ b/platformio/commands/check/tools/pvsstudio.py @@ -0,0 +1,232 @@ +# Copyright (c) 2020-present PlatformIO +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import tempfile +from xml.etree.ElementTree import fromstring + +import click + +from platformio import proc, util +from platformio.commands.check.defect import DefectItem +from platformio.commands.check.tools.base import CheckToolBase +from platformio.managers.core import get_core_package_dir + + +class PvsStudioCheckTool(CheckToolBase): # pylint: disable=too-many-instance-attributes + def __init__(self, *args, **kwargs): + self._tmp_dir = tempfile.mkdtemp(prefix="piocheck") + self._tmp_preprocessed_file = self._generate_tmp_file_path() + ".i" + self._tmp_output_file = self._generate_tmp_file_path() + ".pvs" + self._tmp_cfg_file = self._generate_tmp_file_path() + ".cfg" + self._tmp_cmd_file = self._generate_tmp_file_path() + ".cmd" + self.tool_path = os.path.join( + get_core_package_dir("tool-pvs-studio"), + "x64" if "windows" in util.get_systype() else "bin", + "pvs-studio", + ) + super(PvsStudioCheckTool, self).__init__(*args, **kwargs) + + with open(self._tmp_cfg_file, "w") as fp: + fp.write( + "exclude-path = " + + self.config.get_optional_dir("packages").replace("\\", "/") + ) + + with open(self._tmp_cmd_file, "w") as fp: + fp.write( + " ".join( + ['-I"%s"' % inc.replace("\\", "/") for inc in self.cpp_includes] + ) + ) + + def _process_defects(self, defects): + for defect in defects: + if not isinstance(defect, DefectItem): + return + if defect.severity not in self.options["severity"]: + return + self._defects.append(defect) + if self._on_defect_callback: + self._on_defect_callback(defect) + + def _demangle_report(self, output_file): + converter_tool = os.path.join( + get_core_package_dir("tool-pvs-studio"), + "HtmlGenerator" + if "windows" in util.get_systype() + else os.path.join("bin", "plog-converter"), + ) + + cmd = ( + converter_tool, + "-t", + "xml", + output_file, + "-m", + "cwe", + "-m", + "misra", + "-a", + # Enable all possible analyzers and defect levels + "GA:1,2,3;64:1,2,3;OP:1,2,3;CS:1,2,3;MISRA:1,2,3", + "--cerr", + ) + + result = proc.exec_command(cmd) + if result["returncode"] != 0: + click.echo(result["err"]) + self._bad_input = True + + return result["err"] + + def parse_defects(self, output_file): + defects = [] + + report = self._demangle_report(output_file) + if not report: + self._bad_input = True + return [] + + try: + defects_data = fromstring(report) + except: # pylint: disable=bare-except + click.echo("Error: Couldn't decode generated report!") + self._bad_input = True + return [] + + for table in defects_data.iter("PVS-Studio_Analysis_Log"): + message = table.find("Message").text + category = table.find("ErrorType").text + line = table.find("Line").text + file_ = table.find("File").text + defect_id = table.find("ErrorCode").text + cwe = table.find("CWECode") + cwe_id = None + if cwe is not None: + cwe_id = cwe.text.lower().replace("cwe-", "") + misra = table.find("MISRA") + if misra is not None: + message += " [%s]" % misra.text + + severity = DefectItem.SEVERITY_LOW + if category == "error": + severity = DefectItem.SEVERITY_HIGH + elif category == "warning": + severity = DefectItem.SEVERITY_MEDIUM + + defects.append( + DefectItem( + severity, category, message, file_, line, id=defect_id, cwe=cwe_id + ) + ) + + return defects + + def configure_command(self, src_file): # pylint: disable=arguments-differ + if os.path.isfile(self._tmp_output_file): + os.remove(self._tmp_output_file) + + if not os.path.isfile(self._tmp_preprocessed_file): + click.echo( + "Error: Missing preprocessed file '%s'" % (self._tmp_preprocessed_file) + ) + return "" + + cmd = [ + self.tool_path, + "--skip-cl-exe", + "yes", + "--language", + "C" if src_file.endswith(".c") else "C++", + "--preprocessor", + "gcc", + "--cfg", + self._tmp_cfg_file, + "--source-file", + src_file, + "--i-file", + self._tmp_preprocessed_file, + "--output-file", + self._tmp_output_file, + ] + + flags = self.get_flags("pvs-studio") + if not self.is_flag_set("--platform", flags): + cmd.append("--platform=arm") + cmd.extend(flags) + + return cmd + + def _generate_tmp_file_path(self): + # pylint: disable=protected-access + return os.path.join(self._tmp_dir, next(tempfile._get_candidate_names())) + + def _prepare_preprocessed_file(self, src_file): + flags = self.cxx_flags + compiler = self.cxx_path + if src_file.endswith(".c"): + flags = self.cc_flags + compiler = self.cc_path + + cmd = [compiler, src_file, "-E", "-o", self._tmp_preprocessed_file] + cmd.extend([f for f in flags if f]) + cmd.extend(["-D%s" % d for d in self.cpp_defines]) + cmd.append('@"%s"' % self._tmp_cmd_file) + + result = proc.exec_command(" ".join(cmd), shell=True) + if result["returncode"] != 0: + if self.options.get("verbose"): + click.echo(" ".join(cmd)) + click.echo(result["err"]) + self._bad_input = True + + def clean_up(self): + temp_files = ( + self._tmp_output_file, + self._tmp_preprocessed_file, + self._tmp_cfg_file, + self._tmp_cmd_file, + ) + for f in temp_files: + if os.path.isfile(f): + os.remove(f) + + def check(self, on_defect_callback=None): + self._on_defect_callback = on_defect_callback + src_files = [ + f for f in self.get_project_target_files() if not f.endswith((".h", ".hpp")) + ] + + for src_file in src_files: + self._prepare_preprocessed_file(src_file) + cmd = self.configure_command(src_file) + if self.options.get("verbose"): + click.echo(" ".join(cmd)) + if not cmd: + self._bad_input = True + continue + + result = proc.exec_command(cmd) + # pylint: disable=unsupported-membership-test + if result["returncode"] != 0 or "License was not entered" in result["err"]: + self._bad_input = True + click.echo(result["err"]) + continue + + self._process_defects(self.parse_defects(self._tmp_output_file)) + + self.clean_up() + + return self._bad_input diff --git a/platformio/managers/core.py b/platformio/managers/core.py index 6166d8fe..1d09474b 100644 --- a/platformio/managers/core.py +++ b/platformio/managers/core.py @@ -31,6 +31,7 @@ CORE_PACKAGES = { "tool-scons": "~2.20501.7" if PY2 else "~3.30102.0", "tool-cppcheck": "~1.189.0", "tool-clangtidy": "^1.80000.0", + "tool-pvs-studio": "~7.5.0", } PIOPLUS_AUTO_UPDATES_MAX = 100 diff --git a/platformio/project/options.py b/platformio/project/options.py index f8a50a76..d38ed566 100644 --- a/platformio/project/options.py +++ b/platformio/project/options.py @@ -531,7 +531,7 @@ ProjectOptions = OrderedDict( group="check", name="check_tool", description="A list of check tools used for analysis", - type=click.Choice(["cppcheck", "clangtidy"]), + type=click.Choice(["cppcheck", "clangtidy", "pvs-studio"]), multiple=True, default=["cppcheck"], ), diff --git a/tests/commands/test_check.py b/tests/commands/test_check.py index 64e4602e..2078acf1 100644 --- a/tests/commands/test_check.py +++ b/tests/commands/test_check.py @@ -239,21 +239,30 @@ int main() { def test_check_individual_flags_passed(clirunner, tmpdir): - config = DEFAULT_CONFIG + "\ncheck_tool = cppcheck, clangtidy" - config += "\ncheck_flags = cppcheck: --std=c++11 \n\tclangtidy: --fix-errors" + config = DEFAULT_CONFIG + "\ncheck_tool = cppcheck, clangtidy, pvs-studio" + config += """\ncheck_flags = + cppcheck: --std=c++11 + clangtidy: --fix-errors + pvs-studio: --analysis-mode=4 +""" tmpdir.join("platformio.ini").write(config) tmpdir.mkdir("src").join("main.cpp").write(TEST_CODE) result = clirunner.invoke(cmd_check, ["--project-dir", str(tmpdir), "-v"]) - clang_flags_found = cppcheck_flags_found = False + clang_flags_found = cppcheck_flags_found = pvs_flags_found = False for l in result.output.split("\n"): if "--fix" in l and "clang-tidy" in l and "--std=c++11" not in l: clang_flags_found = True elif "--std=c++11" in l and "cppcheck" in l and "--fix" not in l: cppcheck_flags_found = True + elif ( + "--analysis-mode=4" in l and "pvs-studio" in l.lower() and "--fix" not in l + ): + pvs_flags_found = True assert clang_flags_found assert cppcheck_flags_found + assert pvs_flags_found def test_check_cppcheck_misra_addon(clirunner, check_dir): @@ -344,3 +353,33 @@ int main() { assert high_result.exit_code == 0 assert low_result.exit_code != 0 + + +def test_check_pvs_studio_free_license(clirunner, tmpdir): + config = """ +[env:test] +platform = teensy +board = teensy35 +framework = arduino +check_tool = pvs-studio +""" + code = ( + """// This is an open source non-commercial project. Dear PVS-Studio, please check it. +// PVS-Studio Static Code Analyzer for C, C++, C#, and Java: http://www.viva64.com +""" + + TEST_CODE + ) + + tmpdir.join("platformio.ini").write(config) + tmpdir.mkdir("src").join("main.c").write(code) + + result = clirunner.invoke( + cmd_check, ["--project-dir", str(tmpdir), "--fail-on-defect=high", "-v"] + ) + + errors, warnings, style = count_defects(result.output) + + assert result.exit_code != 0 + assert errors != 0 + assert warnings != 0 + assert style == 0