Merge pull request #3949 from bruntib/no_unknown_checker_state

Eliminate "unknown" checker state

analyzer/codechecker_analyzer/analysis_manager.py

@@ -329,8 +329,8 @@ def handle_failure(
     # from the standard output by this postprocess phase so we can present them
     # as CodeChecker reports.
     checks = source_analyzer.config_handler.checks()
-    state = checks.get('clang-diagnostic-error', (CheckerState.default, ''))[0]
-    if state != CheckerState.disabled:
+    state = checks.get('clang-diagnostic-error', (CheckerState.enabled, ''))[0]
+    if state == CheckerState.enabled:
         rh.postprocess_result(skip_handlers)
 
     # Remove files that successfully analyzed earlier on.

analyzer/codechecker_analyzer/analyzers/clangsa/analyzer.py

@@ -10,6 +10,7 @@
 """
 
 import os
+import plistlib
 import re
 import shlex
 import subprocess
@@ -123,6 +124,7 @@ def __init__(self, cfg_handler, buildaction):
         super(ClangSA, self).__init__(cfg_handler, buildaction)
         self.__disable_ctu = False
         self.__checker_configs = []
+        self.__disabled_checkers = []
 
     @classmethod
     def analyzer_binary(cls):
@@ -305,6 +307,43 @@ def get_analyzer_config(cls) -> List[str]:
 
         return parse_clang_help_page(command, 'OPTIONS:')
 
+    def post_analyze(self, result_handler):
+        """
+        Disabled checkers are not actually disabled during analysis, because
+        they might rely on each other under the hood. The disabled checkers'
+        reports are removed in this post-processing step.
+        """
+        try:
+            if not os.path.isfile(result_handler.analyzer_result_file):
+                # This check has the same race-condition reason as the
+                # exception, so see its description below.
+                return
+
+            with open(result_handler.analyzer_result_file, 'rb') as f:
+                plist = plistlib.load(f)
+        except plistlib.InvalidFileException:
+            # It may happen that a compilation database contains a build action
+            # multiple times (because it is compiled for several target
+            # architectures), or at least they differ in a so minor part that
+            # the same .plist file belongs to them. (For further details see
+            # analyzer_action_str() in ResultHandler about the strange behavior
+            # of make 4.3 in its -o flag.)
+            # If this happens then the analysis of the same build action is
+            # analyzed in parallel several times, so the same output .plist
+            # file is changed by several threads. This may result an invalid
+            # .plist which fails plistlib.load(). This is not a big problem,
+            # because the second thread will also execute this post-processing
+            # and it happens rarely anyways. test_compile_uniqueing() fails
+            # undeterministically without this.
+            return
+
+        plist['diagnostics'] = list(filter(
+            lambda diag: diag['check_name'] not in self.__disabled_checkers,
+            plist.get('diagnostics', [])))
+
+        with open(result_handler.analyzer_result_file, 'wb') as f:
+            plistlib.dump(plist, f)
+
     def construct_analyzer_cmd(self, result_handler):
         """
         Called by the analyzer method.
@@ -356,23 +395,19 @@ def construct_analyzer_cmd(self, result_handler):
                     ['-Xclang', '-analyzer-config', '-Xclang', cfg])
 
             # Config handler stores which checkers are enabled or disabled.
-            disabled_checkers = []
+            self.__disabled_checkers = []
             enabled_checkers = []
             for checker_name, value in config.checks().items():
                 state, _ = value
                 if state == CheckerState.enabled:
                     enabled_checkers.append(checker_name)
                 elif state == CheckerState.disabled:
-                    disabled_checkers.append(checker_name)
+                    self.__disabled_checkers.append(checker_name)
 
             if enabled_checkers:
                 analyzer_cmd.extend(['-Xclang',
                                      '-analyzer-checker=' +
                                      ','.join(enabled_checkers)])
-            if disabled_checkers:
-                analyzer_cmd.extend(['-Xclang',
-                                     '-analyzer-disable-checker=' +
-                                     ','.join(disabled_checkers)])
             # Enable aggressive-binary-operation-simplification option.
             version_info = ClangSA.version_info()
             if version_info and version_info.major_version >= 8:

analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py

@@ -16,7 +16,7 @@
 import re
 import shlex
 import subprocess
-from typing import List, Tuple
+from typing import Iterable, List, Set, Tuple
 
 import yaml
 
@@ -165,13 +165,55 @@ def get_warnings(env=None):
         raise
 
 
+def _add_asterisk_for_group(
+    subset_checkers: Iterable[str],
+    all_checkers: Set[str]
+) -> List[str]:
+    """
+    Since CodeChecker interprets checker name prefixes as checker groups, they
+    have to be added a '*' joker character when using them at clang-tidy
+    -checks flag. This function adds a '*' for each item in "checkers" if it's
+    a checker group, i.e. identified as a prefix for any checker name in
+    "all_checkers".
+    For example "readability-container" is a prefix of multiple checkers, so
+    this is converted to "readability-container-*". On the other hand
+    "performance-trivially-destructible" is a full checker name, so it remains
+    as is.
+    """
+    def is_group_prefix_of(prefix: str, long: str) -> bool:
+        """
+        Returns True if a checker(-group) name is prefix of another
+        checker name. For example bugprone-string is prefix of
+        bugprone-string-constructor but not of
+        bugprone-stringview-nullptr.
+        """
+        prefix_split = prefix.split('-')
+        long_split = long.split('-')
+        return prefix_split == long_split[:len(prefix_split)]
+
+    def need_asterisk(checker: str) -> bool:
+        return any(
+            is_group_prefix_of(checker, long) and checker != long
+            for long in all_checkers)
+
+    result = []
+
+    for checker in subset_checkers:
+        result.append(checker + ('*' if need_asterisk(checker) else ''))
+
+    return result
+
+
 class ClangTidy(analyzer_base.SourceAnalyzer):
     """
     Constructs the clang tidy analyzer commands.
     """
 
     ANALYZER_NAME = 'clang-tidy'
 
+    # Cache object for get_analyzer_checkers().
+    __analyzer_checkers = None
+
     @classmethod
     def analyzer_binary(cls):
         return analyzer_context.get_context() \
@@ -204,6 +246,9 @@ def get_analyzer_checkers(cls):
         Return the list of the all of the supported checkers.
         """
         try:
+            if cls.__analyzer_checkers:
+                return cls.__analyzer_checkers
+
             environ = analyzer_context.get_context().analyzer_env
             result = subprocess.check_output(
                 [cls.analyzer_binary(), "-list-checks", "-checks=*"],
@@ -217,6 +262,8 @@ def get_analyzer_checkers(cls):
                 ("clang-diagnostic-" + warning, "")
                 for warning in get_warnings(environ))
 
+            cls.__analyzer_checkers = checker_description
+
             return checker_description
         except (subprocess.CalledProcessError, OSError):
             return []
@@ -266,30 +313,43 @@ def get_checker_list(self, config) -> Tuple[List[str], List[str]]:
         clang-tidy and in some cases that would cause analysis error due to
         some ClangSA bug.
         """
-        checkers = []
-        # Usage of a set will remove compiler warnings and clang-diagnostics
-        # which are the same.
+        # clang-tidy emits reports from its check in the same format as
+        # compiler diagnostics (like unused variables, etc). This makes it a
+        # little difficult to distinguish compiler warnings and clang-tidy
+        # check warnings. The only clue is that compiler warnings are emitted
+        # as if they came from a check called clang-diagnostic- (e.g.
+        # -Wunused-variable will emit a warning under the name
+        # clang-diagnostic-unused-variable).
+
+        # There are two ways to disable a compiler warning in clang-tidy,
+        # either by -Wno- or -checks=-clang-diagnostic- (note the dash before
+        # clang-diagnostic!). However, there is only one way to enable them:
+        # through -W. Using -checks=clang-diagnostic- does not enable the
+        # warning, but undoes -checks=-clang-diagnostic-.
+
+        # Since we disable all checks by default via -checks=-*, in order to
+        # enable a compiler warning, we first have to undo the -checks level
+        # disable and then enable it, so we need both
+        # -checks=compiler-diagnostic- and -W.
         compiler_warnings = set()
+        enabled_checkers = set()
 
         has_checker_config = \
             config.checker_config and config.checker_config != '{}'
 
-        # Do not disable any clang-tidy checks explicitly, but don't run
-        # ClangSA checkers. ClangSA checkers are driven by an other
-        # analyzer in CodeChecker.
-        checkers.append('-clang-analyzer-*')
-
-        # For clang compiler warnings a correspoding
-        # clang-diagnostic error is generated by Clang tidy.
-        # They can be disabled by this glob -clang-diagnostic-*
-        checkers.append('clang-diagnostic-*')
-
         # Config handler stores which checkers are enabled or disabled.
         for checker_name, value in config.checks().items():
             state, _ = value
 
             warning_name, warning_type = \
                 get_compiler_warning_name_and_type(checker_name)
+
+            # This warning must be given a parameter separated by either '=' or
+            # space. This warning is not supported as a checker name so its
+            # special usage is avoided.
+            if warning_name and warning_name.startswith('frame-larger-than'):
+                continue
+
             if warning_name is not None:
                 # -W and clang-diagnostic- are added as compiler warnings.
                 if warning_type == CheckerType.compiler:
@@ -300,28 +360,34 @@ def get_checker_list(self, config) -> Tuple[List[str], List[str]]:
                                 "instead.")
                     if state == CheckerState.enabled:
                         compiler_warnings.add('-W' + warning_name)
+                        enabled_checkers.add(checker_name)
                     elif state == CheckerState.disabled:
                         if config.enable_all:
                             LOG.warning("Disabling compiler warning with "
                                         f"compiler flag '-d W{warning_name}' "
                                         "is not supported.")
-                        compiler_warnings.add('-Wno-' + warning_name)
                 # If a clang-diagnostic-... is enabled add it as a compiler
                 # warning as -W..., if it is disabled, tidy can suppress when
                 # specified in the -checks parameter list, so we add it there
                 # as -clang-diagnostic-... .
                 elif warning_type == CheckerType.analyzer:
                     if state == CheckerState.enabled:
                         compiler_warnings.add('-W' + warning_name)
-                    elif state == CheckerState.disabled:
-                        checkers.append('-' + checker_name)
+                        enabled_checkers.add(checker_name)
 
                 continue
 
             if state == CheckerState.enabled:
-                checkers.append(checker_name)
-            elif state == CheckerState.disabled:
-                checkers.append('-' + checker_name)
+                enabled_checkers.add(checker_name)
+
+        # By default all checkers are disabled and the enabled ones are added
+        # explicitly.
+        checkers = ['-*']
+
+        checkers += _add_asterisk_for_group(
+            enabled_checkers,
+            set(x[0] for x in ClangTidy.get_analyzer_checkers()))
+
         # -checks=-clang-analyzer-* option is added to the analyzer command by
         # default except when all analyzer config options come from .clang-tidy
         # file. The content of this file overrides every other custom config
@@ -378,7 +444,8 @@ def construct_analyzer_cmd(self, result_handler):
             analyzer_cmd.append('-Qunused-arguments')
 
             # Enable these compiler warnings by default.
-            analyzer_cmd.extend(['-Wall', '-Wextra'])
+            if not config.enable_all:
+                analyzer_cmd.extend(['-Wno-everything'])
 
             compile_lang = self.buildaction.lang
 

analyzer/codechecker_analyzer/analyzers/clangtidy/config_handler.py

@@ -33,7 +33,7 @@ def __init__(self):
         super(ClangTidyConfigHandler, self).__init__()
 
     def add_checker(self, checker_name, description='',
-                    state=CheckerState.default):
+                    state=CheckerState.disabled):
         """
         Add additional checker if the 'take-config-from-directory'
         analyzer configuration option is not set.

analyzer/codechecker_analyzer/analyzers/config_handler.py

@@ -21,18 +21,13 @@
 LOG = get_logger('system')
 
 
-# The baseline handling of checks in every analyzer is to let the analysis
-# engine decide which checks are worthwhile run. Checks handled this way
-# (implicitly by the analyzer) are considered to have a CheckerState of
-# default. If the check however appears in profiles, and such a profile is
-# enabled explicitly on the command-line or implicitly as in case of the
-# default profile, then they are considered to have a CheckerState of enabled.
-# Likewise for individually enabled checks. If a check is however explicitly
-# disabled on the command-line, or belongs to a profile explicitly disabled
-# on the command-line, then it is considered to have a CheckerState of
-# disabled.
+# If the check appears in profiles, and such a profile is enabled explicitly on
+# the command-line or implicitly as in case of the default profile, then they
+# are considered to have a CheckerState of enabled. Likewise for individually
+# enabled checks. If a check is however explicitly disabled on the
+# command-line, or belongs to a profile explicitly disabled on the
+# command-line, then it is considered to have a CheckerState of disabled.
 class CheckerState(Enum):
-    default = 0
     disabled = 1
     enabled = 2
 
@@ -77,17 +72,14 @@ def __init__(self):
         self.report_hash = None
         self.enable_all = None
 
-        # The key is the checker name, the value is a tuple.
-        # False if disabled (should be by default).
-        # True if checker is enabled.
-        # (False/True, 'checker_description')
+        # The key is the checker name, the value is a tuple of CheckerState and
+        # checker description.
         self.__available_checkers = collections.OrderedDict()
 
     def add_checker(self, checker_name, description='',
-                    state=CheckerState.default):
+                    state=CheckerState.disabled):
         """
-        Add additional checker. If no state argument is given, the actual usage
-        of the checker is handled by the analyzer.
+        Add a checker to the available checkers' list.
         """
         self.__available_checkers[checker_name] = (state, description)
 
@@ -171,8 +163,7 @@ def initialize_checkers(self,
 
         checker_labels = analyzer_context.get_context().checker_labels
 
-        # Add all checkers marked as default. This means the analyzer should
-        # manage whether it is enabled or disabled.
+        # Add all checkers marked as disabled.
         for checker_name, description in checkers:
             self.add_checker(checker_name, description)
 

analyzer/codechecker_analyzer/analyzers/cppcheck/config_handler.py

@@ -9,39 +9,10 @@
 Config handler for Cppcheck analyzer.
 """
 from .. import config_handler
-from ..config_handler import CheckerState
 
 
 class CppcheckConfigHandler(config_handler.AnalyzerConfigHandler):
     """
     Configuration handler for Cppcheck analyzer.
     """
-    def initialize_checkers(self,
-                            checkers,
-                            cmdline_enable=None,
-                            enable_all=False):
-        if not cmdline_enable:
-            cmdline_enable = list()
-        """
-        Set all the default checkers to disabled. This will ensure that
-        --enable=all will not run with all the possible checkers
-        """
-        super().initialize_checkers(
-                            checkers,
-                            cmdline_enable,
-                            enable_all)
-
-        # Set all the checkers with default CheckerState checkers to
-        # disabled. This will ensure that --enable=all will not run with
-        # all the possible checkers. All the checkers that are in the default
-        # profile (or configured othewise, eg.: from the cli) should be
-        # already enabled at this point.
-        # This happens in two phases in order to avoid iterator invalidation.
-        # (self.set_checker_enabled() removes elements, so we can't use it
-        # while iterating over the checker list.)
-        default_state_checkers = [
-            checker_name for checker_name, data in
-            self.checks().items() if data[0] == CheckerState.default]
-
-        for checker_name in default_state_checkers:
-            self.set_checker_enabled(checker_name, enabled=False)
+    pass