Skip to content

Commit

Permalink
[analyzer] Add the 'executable' config to all analyzers
Browse files Browse the repository at this point in the history
By default, we look for clang, clang-tidy, gcc, etc. These binaries (and
compilers in general) are notorious for having their version number in
the binary name, making it difficult to run clang++-16 instead of
clang++. Its not impossible, but really inconvenient, hence this patch.

We debated a few approaches to this (including a CodeChecker-wide option
called --analyzer-binary, that could havve been used like this:
  --analyzer-binary clangsa:/usr/bin/clang-16

We decided that since not all analyzers might have an executable binary
(like pylint), we shouldn't commit to this naming. We also decided that
this is a flag that should be implemented by individual analyzer
plugins.

The PR adds the 'executable' config to all currently supported
analyzers.

I also needed to make adjustments to `CodeChecker analyzers`, since the
new config must be visible, even if we couldn't get hold of an analyzer
binary that we support. The list of changes:
* Deprecate both `--all` and `--details`, because they were more
  confusing than useful
* We list all supported (not available) analyzers by default, but print
  a warning message when its not available.
* We used to query the version of each analyzer binary in a uniform way,
  but clang, cppcheck, gcc prints their version using different flags,
  and in different formats. I changed this to call the analyzer plugins'
  get_binary_version method (which I also implemented in this patch).

I don't like enlarging patches too much, but this is how the PR turned
out. Its honestly not my proudest work (you can only do so much in so
little time), but I think the interface is fine, even if the
implementation can use a little improvement.

Closes #3898.
  • Loading branch information
Szelethus committed Oct 20, 2023
1 parent fcd5f23 commit 2713597
Show file tree
Hide file tree
Showing 8 changed files with 142 additions and 109 deletions.
3 changes: 2 additions & 1 deletion analyzer/codechecker_analyzer/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,8 @@ def perform_analysis(args, skip_handlers, actions, metadata_tool,

# TODO: cppcheck may require a different environment than clang.
version = analyzer_types.supported_analyzers[analyzer] \
.get_version(context.analyzer_env)
.get_binary_version(context.analyzer_binaries[analyzer],
context.analyzer_env)
metadata_info['analyzer_statistics']['version'] = version

metadata_tool['analyzers'][analyzer] = metadata_info
Expand Down
20 changes: 20 additions & 0 deletions analyzer/codechecker_analyzer/analyzers/analyzer_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@
LOG = get_logger('analyzer')


def handle_analyzer_executable_from_config(analyzer_name, path):
context = analyzer_context.get_context()
if not os.path.isfile(path):
LOG.error(f"'{path}' is not a path to an analyzer binary "
f"given to --analyzer-config={analyzer_name}:executable!")
sys.exit(1)
context.analyzer_binaries[analyzer_name] = path


class SourceAnalyzer(metaclass=ABCMeta):
"""
Base class for different source analyzers.
Expand Down Expand Up @@ -56,6 +65,17 @@ def resolve_missing_binary(cls, configured_binary, environ):
"""
raise NotImplementedError("Subclasses should implement this!")

@abstractmethod
def get_binary_version(self, configured_binary, environ, details=False) \
-> str:
"""
Return the version number of the binary that CodeChecker found, even
if its incompatible. If details is true, additional version information
is provided. If details is false, the return value should be
convertible to a distutils.version.StrictVersion type.
"""
raise NotImplementedError("Subclasses should implement this!")

@classmethod
def is_binary_version_incompatible(cls, configured_binary, environ) \
-> Optional[str]:
Expand Down
3 changes: 1 addition & 2 deletions analyzer/codechecker_analyzer/analyzers/analyzer_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,7 @@ def construct_analyzer(buildaction,
LOG.error('Unsupported analyzer type: %s', analyzer_type)
return analyzer

except Exception as ex:
LOG.debug_analyzer(ex)
except Exception:
# We should've detected well before this point that something is off
# with the analyzer. We can't recover here.
raise
Expand Down
29 changes: 22 additions & 7 deletions analyzer/codechecker_analyzer/analyzers/clangsa/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@

def parse_clang_help_page(
command: List[str],
start_label: str
start_label: str,
starting_value: (str, str) = None
) -> List[str]:
"""
Parse the clang help page starting from a specific label.
Expand Down Expand Up @@ -75,6 +76,8 @@ def parse_clang_help_page(
re.compile(r"^\s{3,}(?P<desc>[^\n]+)$")

res = []
if starting_value:
res.append(starting_value)

flag = None
desc = []
Expand Down Expand Up @@ -171,16 +174,19 @@ def __add_plugin_load_flags(cls, analyzer_cmd: List[str]):
analyzer_cmd.extend(["-load", plugin])

@classmethod
def get_version(cls, env=None):
""" Get analyzer version information. """
version = [cls.analyzer_binary(), '--version']
def get_binary_version(self, configured_binary, environ, details=False) \
-> str:
if details:
version = [configured_binary, '--version']
else:
version = [configured_binary, '-dumpversion']
try:
output = subprocess.check_output(version,
env=env,
env=environ,
universal_newlines=True,
encoding="utf-8",
errors="ignore")
return output
return output.strip()
except (subprocess.CalledProcessError, OSError) as oerr:
LOG.warning("Failed to get analyzer version: %s",
' '.join(version))
Expand Down Expand Up @@ -305,7 +311,11 @@ def get_analyzer_config(cls) -> List[str]:

command.append("-analyzer-config-help")

return parse_clang_help_page(command, 'OPTIONS:')
return parse_clang_help_page(
command, 'OPTIONS:',
("executable", "Use the specified analyzer binary. This "
"supersedes any other method CodeChecker might use "
"to get hold of one."))

def post_analyze(self, result_handler):
"""
Expand Down Expand Up @@ -700,6 +710,11 @@ def construct_config_handler(cls, args):
isinstance(args.analyzer_config, list):
for cfg in args.analyzer_config:
if cfg.analyzer == cls.ANALYZER_NAME:
if cfg.option == 'executable':
analyzer_base.handle_analyzer_executable_from_config(
cfg.analyzer, cfg.value)
LOG.info(f"Using clangsa binary '{cfg.value}'")
continue
handler.checker_config.append(f"{cfg.option}={cfg.value}")

return handler
37 changes: 30 additions & 7 deletions analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,16 @@ def need_asterisk(checker: str) -> bool:
return result


def parse_version(tidy_output):
"""
Parse clang-tidy version output and return the version number.
"""
version_re = re.compile(r'.*version (?P<version>[\d\.]+)', re.S)
match = version_re.match(tidy_output)
if match:
return match.group('version')


class ClangTidy(analyzer_base.SourceAnalyzer):
"""
Constructs the clang tidy analyzer commands.
Expand All @@ -220,16 +230,18 @@ def analyzer_binary(cls):
.analyzer_binaries[cls.ANALYZER_NAME]

@classmethod
def get_version(cls, env=None):
""" Get analyzer version information. """
version = [cls.analyzer_binary(), '--version']
def get_binary_version(self, configured_binary, environ, details=False) \
-> str:
version = [configured_binary, '--version']
try:
output = subprocess.check_output(version,
env=env,
env=environ,
universal_newlines=True,
encoding="utf-8",
errors="ignore")
return output
if details:
return output.strip()
return parse_version(output)
except (subprocess.CalledProcessError, OSError) as oerr:
LOG.warning("Failed to get analyzer version: %s",
' '.join(version))
Expand Down Expand Up @@ -289,16 +301,22 @@ def get_analyzer_config(cls):
"""
Return the analyzer configuration with all checkers enabled.
"""

tidy_configs = [
("executable", "Use the specified analyzer binary. This "
"supersedes any other method CodeChecker might use "
"to get hold of one.")]
try:
result = subprocess.check_output(
[cls.analyzer_binary(), "-dump-config", "-checks=*"],
env=analyzer_context.get_context().analyzer_env,
universal_newlines=True,
encoding="utf-8",
errors="ignore")
return parse_analyzer_config(result)
tidy_configs.extend(parse_analyzer_config(result))
except (subprocess.CalledProcessError, OSError):
return []
pass
return tidy_configs

def get_checker_list(self, config) -> Tuple[List[str], List[str]]:
"""
Expand Down Expand Up @@ -583,6 +601,11 @@ def construct_config_handler(cls, args):
isinstance(args.analyzer_config, list):
for cfg in args.analyzer_config:
if cfg.analyzer == cls.ANALYZER_NAME:
if cfg.option == 'executable':
analyzer_base.handle_analyzer_executable_from_config(
cfg.analyzer, cfg.value)
LOG.info(f"Using clang-tidy binary '{cfg.value}'")
continue
analyzer_config[cfg.option] = cfg.value

# Reports in headers are hidden by default in clang-tidy. Re-enable it
Expand Down
47 changes: 20 additions & 27 deletions analyzer/codechecker_analyzer/analyzers/cppcheck/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def parse_version(cppcheck_output):
version_re = re.compile(r'^Cppcheck (?P<version>[\d\.]+)')
match = version_re.match(cppcheck_output)
if match:
return StrictVersion(match.group('version'))
return match.group('version')


class Cppcheck(analyzer_base.SourceAnalyzer):
Expand All @@ -83,16 +83,19 @@ def analyzer_binary(cls):
.analyzer_binaries[cls.ANALYZER_NAME]

@classmethod
def get_version(cls, env=None):
def get_binary_version(self, configured_binary, environ, details=False) \
-> str:
""" Get analyzer version information. """
version = [cls.analyzer_binary(), '--version']
version = [configured_binary, '--version']
try:
output = subprocess.check_output(version,
env=env,
env=environ,
universal_newlines=True,
encoding="utf-8",
errors="ignore")
return output
if details:
return output.strip()
return parse_version(output)
except (subprocess.CalledProcessError, OSError) as oerr:
LOG.warning("Failed to get analyzer version: %s",
' '.join(version))
Expand Down Expand Up @@ -257,11 +260,13 @@ def get_analyzer_config(cls):
"""
Config options for cppcheck.
"""
return [("addons", "A list of cppcheck addon files."),
return [("executable", "Use the specified analyzer binary. This "
"supersedes any other method CodeChecker might "
"use to get hold of one."),
("addons", "A list of cppcheck addon files."),
("libraries", "A list of cppcheck library definiton files."),
("platform", "The platform configuration .xml file."),
("inconclusive", "Enable inconclusive reports.")
]
("inconclusive", "Enable inconclusive reports.")]

@classmethod
def get_checker_config(cls):
Expand Down Expand Up @@ -333,34 +338,17 @@ def resolve_missing_binary(cls, configured_binary, env):
LOG.debug("Using '%s' for Cppcheck!", cppcheck)
return cppcheck

@classmethod
def __get_analyzer_version(cls, analyzer_binary, env):
"""
Return the analyzer version.
"""
command = [analyzer_binary, "--version"]

try:
result = subprocess.check_output(
command,
env=env,
encoding="utf-8",
errors="ignore")
return parse_version(result)
except (subprocess.CalledProcessError, OSError):
return []

@classmethod
def is_binary_version_incompatible(cls, configured_binary, environ):
"""
Check the version compatibility of the given analyzer binary.
"""
analyzer_version = \
cls.__get_analyzer_version(configured_binary, environ)
cls.get_binary_version(configured_binary, environ)

# The analyzer version should be above 1.80 because '--plist-output'
# argument was introduced in this release.
if analyzer_version >= StrictVersion("1.80"):
if StrictVersion(analyzer_version) >= StrictVersion("1.80"):
return None

return "CppCheck binary found is too old at " \
Expand Down Expand Up @@ -389,6 +377,11 @@ def construct_config_handler(cls, args):
isinstance(args.analyzer_config, list):
for cfg in args.analyzer_config:
if cfg.analyzer == cls.ANALYZER_NAME:
if cfg.option == 'executable':
analyzer_base.handle_analyzer_executable_from_config(
cfg.analyzer, cfg.value)
LOG.info(f"Using cppcheck binary '{cfg.value}'")
continue
analyzer_config[cfg.option].append(cfg.value)

handler.analyzer_config = analyzer_config
Expand Down
25 changes: 13 additions & 12 deletions analyzer/codechecker_analyzer/analyzers/gcc/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@ def analyzer_binary(cls):
return analyzer_context.get_context() \
.analyzer_binaries[cls.ANALYZER_NAME]

@classmethod
def get_version(cls, env=None):
""" Get analyzer version information. """
return cls.__get_analyzer_version(cls.analyzer_binary(), env)

def add_checker_config(self, checker_cfg):
# TODO
pass
Expand Down Expand Up @@ -132,7 +127,11 @@ def get_analyzer_config(cls):
Config options for gcc.
"""
# TODO
return []
gcc_configs = [
("executable", "Use the specified analyzer binary. This "
"supersedes any other method CodeChecker might use "
"to get hold of one.")]
return gcc_configs

@classmethod
def get_checker_config(cls):
Expand Down Expand Up @@ -174,19 +173,21 @@ def resolve_missing_binary(cls, configured_binary, env):
pass

@classmethod
def __get_analyzer_version(cls, analyzer_binary, env):
def get_binary_version(self, configured_binary, env, details=False) \
-> str:
"""
Return the analyzer version.
"""
# --version outputs a lot of garbage as well (like copyright info),
# this only contains the version info.
version = [analyzer_binary, '-dumpfullversion']
if details:
version = [configured_binary, '--version']
else:
version = [configured_binary, '-dumpfullversion']
try:
output = subprocess.check_output(version,
env=env,
encoding="utf-8",
errors="ignore")
return output
return output.strip()
except (subprocess.CalledProcessError, OSError) as oerr:
LOG.warning("Failed to get analyzer version: %s",
' '.join(version))
Expand All @@ -200,7 +201,7 @@ def is_binary_version_incompatible(cls, configured_binary, environ):
Check the version compatibility of the given analyzer binary.
"""
analyzer_version = \
cls.__get_analyzer_version(configured_binary, environ)
cls.get_binary_version(configured_binary, environ)

# The analyzer version should be above 13.0.0 because the
# '-fdiagnostics-format=sarif-file' argument was introduced in this
Expand Down
Loading

0 comments on commit 2713597

Please sign in to comment.