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 Sep 8, 2023
1 parent f2949d2 commit 2c5a326
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 25 deletions.
47 changes: 41 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 @@ -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):
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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:
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
12 changes: 3 additions & 9 deletions analyzer/tests/unit/test_checker_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,9 @@ def test_no_disabled_checks(self):
"""
Test that ClangSA only uses enable lists.
"""
# TODO: This test is currently removed, because checkers that are not
# enabled are explicitly disabled. In a next commit ClangSA reports
# will be hidden instead of disabled. In that commit this test could be
# re-enabled.
pass

# self.assertFalse(
# any(arg.startswith('-analyzer-disable-checker')
# for arg in self.__class__.cmd))
self.assertFalse(
any(arg.startswith('-analyzer-disable-checker')
for arg in self.__class__.cmd))

def test_checker_initializer(self):
"""
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 @@ -160,6 +160,11 @@ def setup_class_common(workspace_name):
# -----------------------------------------------------

# 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 @@ -183,6 +188,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 2c5a326

Please sign in to comment.