Skip to content

Commit

Permalink
[feat] Hide disabled ClangSA checkers in post-processing
Browse files Browse the repository at this point in the history
ClangSA checkers may rely on each other. For example unix.Malloc checker
may prevent a report from unix.MismatchedDeallocator. It is possible that
after disabling unix.Malloc the underlying analysis continues giving a
chance for unix.MismatchedDeallocator to emit a report.

In order to avoid this noisy behavior, ClangSA checkers are never
disabled during the analysis invocation. If CodeChecker wants to disable
a checker then its result should be hidden in a post-processing step.
  • Loading branch information
bruntib committed Jul 20, 2023
1 parent eef7035 commit bc2aa30
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 16 deletions.
41 changes: 35 additions & 6 deletions analyzer/codechecker_analyzer/analyzers/clangsa/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"""

import os
import plistlib
import re
import shlex
import subprocess
Expand Down Expand Up @@ -106,6 +107,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):
Expand Down Expand Up @@ -288,6 +290,37 @@ 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 rely on each other under the hood. The disabled checkers' reports
are removed in this post-processing step.
"""
if not os.path.isfile(result_handler.analyzer_result_file):
return

try:
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. For further details see analyzer_action_str() in
# ResultHandler. 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.
Expand Down Expand Up @@ -339,23 +372,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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ def analyzer_action_str(self):
# build command, because its hash is used for identification in the
# plist file name.
#
# TODO: This solution is considered a workaround, because the real
# question is why such a subprocess is logged? Can cc1build be always
# used as a traditional g++ command?
# The proper logging of this "make 4.3" version has been done in
# bf140d6, so it is unlikely happen that two build actions differ only
# in their "-o" flags. This workaround is still kept for any case.
#
# Note that some information loss occurs during the following algorithm
# because ' '.join(shlex.split(cmd)) is not necessarily equal to cmd:
Expand Down
6 changes: 6 additions & 0 deletions web/tests/functional/report_viewer_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ def setup_class_common(workspace_name):
test_project_name_third = project_info['name'] + uuid.uuid4().hex

# Let's run the third analysis.
codechecker_cfg['checkers'] = ['-e', 'core.CallAndMessage',
'-d', 'core.StackAddressEscape',
'-d', 'clang-diagnostic',
'-e', 'clang-diagnostic-division-by-zero'
]
ret = codechecker.check_and_store(codechecker_cfg,
test_project_name_third,
project.path(test_project))
Expand All @@ -119,6 +124,7 @@ def setup_class_common(workspace_name):

# Let's run the second analysis and updat the same run.
codechecker_cfg['checkers'] = ['-d', 'core.StackAddressEscape',
'-d', 'unix.Malloc',
'-d', 'clang-diagnostic',
'-e', 'clang-diagnostic-division-by-zero',
'-e', 'clang-diagnostic-return-type']
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,22 +88,21 @@ def setup_method(self, method):
'core.NullDereference': 4,
'cplusplus.NewDelete': 5,
'deadcode.DeadStores': 6,
'misc-definitions-in-headers': 2,
'unix.MismatchedDeallocator': 1}
'misc-definitions-in-headers': 2}

self.run1_sev_counts = {Severity.MEDIUM: 6,
Severity.LOW: 6,
Severity.HIGH: 32}

self.run2_sev_counts = {Severity.MEDIUM: 6,
self.run2_sev_counts = {Severity.MEDIUM: 5,
Severity.LOW: 6,
Severity.HIGH: 24}

self.run1_detection_counts = \
{DetectionStatus.NEW: 44}

self.run2_detection_counts = \
{DetectionStatus.NEW: 36}
{DetectionStatus.NEW: 35}

self.run1_files = \
{'new_delete.cpp': 6,
Expand All @@ -125,7 +124,7 @@ def setup_method(self, method):

self.run2_files = \
{'call_and_message.cpp': 5,
'new_delete.cpp': 6,
'new_delete.cpp': 5,
'divide_zero.cpp': 5,
'divide_zero_duplicate.cpp': 2,
'null_dereference.cpp': 5,
Expand Down
4 changes: 2 additions & 2 deletions web/tests/functional/report_viewer_api/test_report_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ def test_run1_run2_all_results(self):
ReportFilter(),
None)

self.assertEqual(run_result_count, 80)
self.assertEqual(run_result_count, 79)

run_results = self._cc_client.getRunResults(self._runids,
run_result_count,
Expand Down Expand Up @@ -382,7 +382,7 @@ def test_detection_date_filters(self):
def test_fix_date_filters(self):
""" Filter by fix dates. """
report_filter = ReportFilter(
detectionStatus=[DetectionStatus.RESOLVED])
detectionStatus=[DetectionStatus.OFF])
run_results = self._cc_client.getRunResults(None, None, 0,
None, report_filter, None,
False)
Expand Down

0 comments on commit bc2aa30

Please sign in to comment.