Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always pass absolute paths to skip handler #4227

Merged
merged 8 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions analyzer/codechecker_analyzer/buildlog/build_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,11 @@ def __init__(self, **kwargs):
super(BuildAction, self).__setattr__(slot, kwargs[slot])

def __str__(self):
# For debugging.
return ('\nOriginal command: {0},\n'
'Analyzer type: {1},\n Analyzer options: {2},\n'
'Directory: {3},\nOutput: {4},\nLang: {5},\nTarget: {6},\n'
'Source: {7}'). \
format(self.original_command,
self.analyzer_type, self.analyzer_options,
self.directory, self.output, self.lang, self.target,
self.source)
"""
Return all the members of the __slots__ list.
"""
info = [(member, getattr(self, member)) for member in self.__slots__]
return ('\n'.join([f'{key}: {value}' for key, value in info]))

def __setattr__(self, attr, value):
if hasattr(self, attr) and getattr(self, attr) != value:
Expand Down
32 changes: 19 additions & 13 deletions analyzer/codechecker_analyzer/buildlog/log_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# pylint: disable=no-name-in-module
from distutils.spawn import find_executable
from enum import Enum
from pathlib import Path

import glob
import json
Expand Down Expand Up @@ -1048,6 +1049,11 @@ def parse_options(compilation_db_entry,
details['action_type'] = BuildAction.COMPILE

details['source'] = compilation_db_entry['file']
# Calculate absolute path to file if not already abs
if not Path(details['source']).is_absolute():
details['source'] = str(Path(
Path(compilation_db_entry['directory']).absolute(),
compilation_db_entry['file']).resolve())

# In case the file attribute in the entry is empty.
if details['source'] == '.':
Expand Down Expand Up @@ -1281,32 +1287,32 @@ def parse_unique_log(compilation_database,
skipped_cmp_cmd_count = 0

for entry in extend_compilation_database_entries(compilation_database):
# Normalization needs to be done here, because the skip regex
# won't match properly in the skiplist handler.
entry['file'] = os.path.normpath(
os.path.join(entry['directory'], entry['file']))
# Parse compilation db entry into an action object
action = parse_options(entry,
compiler_info_file,
keep_gcc_include_fixed,
keep_gcc_intrin,
clangsa.version.get,
analyzer_clang_version)

# Skip parsing the compilaton commands if it should be skipped
# at both analysis phases (pre analysis and analysis).
# Skipping of the compile commands is done differently if no
# CTU or statistics related feature was enabled.
if analysis_skip_handlers \
and analysis_skip_handlers.should_skip(entry['file']) \
and analysis_skip_handlers.should_skip(action.source) \
and (not ctu_or_stats_enabled or pre_analysis_skip_handlers
and pre_analysis_skip_handlers.should_skip(
entry['file'])):
action.source)):
skipped_cmp_cmd_count += 1
LOG.debug("skipping:", action.source)
continue

action = parse_options(entry,
compiler_info_file,
keep_gcc_include_fixed,
keep_gcc_intrin,
clangsa.version.get,
analyzer_clang_version)

if not action.lang:
skipped_cmp_cmd_count += 1
continue
if action.action_type != BuildAction.COMPILE:
skipped_cmp_cmd_count += 1
continue
if build_action_uniqueing == CompileActionUniqueingType.NONE:
if action not in uniqued_build_actions:
Expand Down
90 changes: 85 additions & 5 deletions analyzer/tests/functional/skip/test_skip.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import tempfile
import unittest

from pathlib import Path

from libtest import env


Expand Down Expand Up @@ -63,9 +65,14 @@ def setup_method(self, method):
self.report_dir = os.path.join(self.test_workspace, "reports")
self.test_dir = os.path.join(os.path.dirname(__file__), 'test_files')

def __analyze_simple(self, build_json, analyzer_extra_options=None):
def __analyze_simple(self,
build_json,
analyzer_extra_options=None,
cwd=None):
""" Analyze the 'simple' project. """
test_dir = os.path.join(self.test_dir, "simple")
if not cwd:
cwd = os.path.join(self.test_dir, "simple")

analyze_cmd = [
self._codechecker_cmd, "analyze", "-c", build_json,
"--analyzers", "clangsa", "-o", self.report_dir]
Expand All @@ -77,7 +84,7 @@ def __analyze_simple(self, build_json, analyzer_extra_options=None):
analyze_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
cwd=test_dir,
cwd=cwd,
encoding="utf-8",
errors="ignore")
out, err = process.communicate()
Expand Down Expand Up @@ -267,6 +274,79 @@ def test_analyze_header_with_file_option(self):
self.assertTrue(plist_files)
self.assertTrue(all('skip_header.cpp' in f for f in plist_files))

def test_analyze_relative_with_file_option(self):
"""
Analyze a file with the --file option with a compilation database
in which the file entry is relative to the directory.
"""
def check_analyze_out(out):
# Checks if analyze command was successful
self.assertNotIn("There were no compilation commands in the" +
"provided compilation database or all of" +
"them were skipped.",
out)
self.assertIn("Starting static analysis", out)
# file_to_be_skipped.cpp must not be in the list of analyzed files
self.assertNotIn("file_to_be_skipped.cpp", out)
# skip_header.cpp must be in the list of analyzed files
self.assertIn("skip_header.cpp", out)

def check_parse_out(out):
# Checks if the parse command was successful
reports = out[0]
self.assertNotIn("file_to_be_skipped.cpp", reports)
# skip_header.cpp must be in the reports
self.assertIn("skip_header.cpp", reports)

# Copy test files to the test workspace
shutil.copytree(Path(self.test_dir, "simple"),
Path(self.test_workspace, "rel_simple"))
# Obtain a compilation database, copy it to the prepared test workspace
self.__log_and_analyze_simple()
build_json = Path(self.test_workspace, "rel_simple", "build.json")
shutil.copy(Path(self.test_workspace, "build.json"), build_json)

compilation_commands = None
with open(build_json, 'r') as f:
compilation_commands = json.load(f)

for entry in compilation_commands:
entry["directory"] = str(Path(self.test_workspace))
entry["file"] = str(Path("rel_simple", entry["file"]))

with open(build_json, 'w') as f:
json.dump(compilation_commands, f)

# Do the CodeChecker Analyze with --file
out, _ = self.__analyze_simple(build_json, ["--clean", "--file", Path(
self.test_workspace, "rel_simple", "skip_header.cpp").absolute()])
check_analyze_out(out)

out = self.__run_parse()
check_parse_out(out)

# Also test where the directory is a ".".
for entry in compilation_commands:
entry["directory"] = "."

with open(build_json, 'w') as f:
json.dump(compilation_commands, f)

# Do the CodeChecker Analyze with --file
# We also need to set cwd to test_workspace
out, _ = self.__analyze_simple(build_json,
["--clean",
"--file",
Path(
self.test_workspace,
"rel_simple",
"skip_header.cpp").absolute()],
cwd=self.test_workspace)
check_analyze_out(out)

out = self.__run_parse()
check_parse_out(out)

def test_analyze_header_with_file_option_and_intercept_json(self):
"""
Analyze a header file with the --file option and a compilation database
Expand Down Expand Up @@ -340,7 +420,7 @@ def test_analyze_file_option(self):
"--ignore", skip_file.name,
"--file", "*/skip_header.cpp"])
print(glob.glob(
os.path.join(self.report_dir, '*.plist')))
os.path.join(self.report_dir, '*.plist')))
self.assertFalse(
any('skip_header.cpp' not in f for f in glob.glob(
os.path.join(self.report_dir, '*.plist'))))
Expand All @@ -357,7 +437,7 @@ def test_analyze_file_option(self):
"--ignore", skip_file.name,
"--file", "*/skip_header.cpp"])
print(glob.glob(
os.path.join(self.report_dir, '*.plist')))
os.path.join(self.report_dir, '*.plist')))
self.assertFalse(
any('skip_header.cpp' not in f for f in glob.glob(
os.path.join(self.report_dir, '*.plist'))))
Expand Down
32 changes: 16 additions & 16 deletions analyzer/tests/unit/test_option_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ def test_build_multiplefiles(self):
action = {
'file': 'main.cpp',
'command': "g++ -o main main.cpp lib.cpp",
'directory': ''}
'directory': '/tmp'}

res = log_parser.parse_options(action)
print(res)
self.assertTrue('main.cpp' == res.source)
self.assertTrue('/tmp/main.cpp' == res.source)
self.assertEqual(BuildAction.COMPILE, res.action_type)

def test_compile_onefile(self):
Expand All @@ -72,11 +72,11 @@ def test_compile_onefile(self):
action = {
'file': 'main.cpp',
'command': "g++ -c main.cpp",
'directory': ''}
'directory': '/tmp'}

res = log_parser.parse_options(action)
print(res)
self.assertTrue('main.cpp' == res.source)
self.assertTrue('/tmp/main.cpp' == res.source)
self.assertEqual(BuildAction.COMPILE, res.action_type)

def test_nasm_action(self):
Expand All @@ -86,12 +86,12 @@ def test_nasm_action(self):
action = {
'file': 'main.asm',
'command': "nasm -f elf64 main.asm",
'directory': ''}
'directory': '/tmp'}

res = log_parser.parse_options(action)
print(res)
self.assertIsNone(res.lang)
self.assertEqual(res.source, 'main.asm')
self.assertEqual(res.source, '/tmp/main.asm')
self.assertEqual(res.analyzer_type, -1)

def test_preprocess_onefile(self):
Expand All @@ -101,12 +101,12 @@ def test_preprocess_onefile(self):
action = {
'file': 'main.c',
'command': "gcc -E main.c",
'directory': ''}
'directory': '/tmp'}

res = log_parser.parse_options(action)
print(res)

self.assertTrue('main.c' == res.source)
self.assertTrue('/tmp/main.c' == res.source)
self.assertEqual(BuildAction.PREPROCESS, res.action_type)

def test_compile_lang(self):
Expand All @@ -117,12 +117,12 @@ def test_compile_lang(self):
action = {
'file': 'main.c',
'command': "gcc -c -x c main.c",
'directory': ''}
'directory': '/tmp'}

res = log_parser.parse_options(action)
print(res)

self.assertTrue('main.c' == res.source)
self.assertTrue('/tmp/main.c' == res.source)
self.assertEqual('c', res.lang)
self.assertEqual(BuildAction.COMPILE, res.action_type)

Expand All @@ -142,13 +142,13 @@ def test_compile_arch(self):
action = {
'file': 'main.c',
'command': "gcc -c -arch " + arch['c'] + ' main.c',
'directory': ''
'directory': '/tmp'
}

res = log_parser.parse_options(action)
print(res)

self.assertTrue('main.c' == res.source)
self.assertTrue('/tmp/main.c' == res.source)
self.assertEqual(arch['c'], res.arch)
self.assertEqual(BuildAction.COMPILE, res.action_type)

Expand Down Expand Up @@ -279,12 +279,12 @@ def test_preprocess_and_compile_with_extra_file(self):
action = {
'file': 'main.cpp',
'command': 'g++ -c -MF deps.txt main.cpp',
'directory': ''}
'directory': '/tmp'}

res = log_parser.parse_options(action)
print(res)
self.assertEqual(res.analyzer_options, [])
self.assertEqual(res.source, 'main.cpp')
self.assertEqual(res.source, '/tmp/main.cpp')
self.assertEqual(BuildAction.COMPILE, res.action_type)

@unittest.skipUnless(
Expand Down Expand Up @@ -381,7 +381,7 @@ def test_preprocess_and_compile_with_extra_file_clang(self):
action = {
'file': 'main.cpp',
'command': 'clang++ -c -MF deps.txt main.cpp',
'directory': ''}
'directory': '/tmp'}

class FakeClangVersion:
installed_dir = "/tmp/clang/install"
Expand All @@ -400,7 +400,7 @@ def fake_clangsa_version_func(compiler, env):
action, get_clangsa_version_func=fake_clangsa_version_func)
print(res)
self.assertEqual(res.analyzer_options, [])
self.assertEqual(res.source, 'main.cpp')
self.assertEqual(res.source, '/tmp/main.cpp')
self.assertEqual(BuildAction.COMPILE, res.action_type)

def test_keep_clang_flags(self):
Expand Down
Loading