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

[fix] Quote command line segment using shlex #3578

Merged
merged 1 commit into from
Jan 21, 2022
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
5 changes: 4 additions & 1 deletion analyzer/codechecker_analyzer/analysis_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,10 @@ def handle_reproducer(source_analyzer, rh, zip_file, actions_map):

LOG.debug("[ZIP] Writing extra information...")
archive.writestr("build-action", action.original_command)
archive.writestr("analyzer-command", ' '.join(rh.analyzer_cmd))
archive.writestr(
"analyzer-command",
' '.join([shlex.quote(x) for x in rh.analyzer_cmd]),
)
archive.writestr("return-code", str(rh.analyzer_returncode))

toolchain = gcc_toolchain.toolchain_in_args(
Expand Down
4 changes: 3 additions & 1 deletion analyzer/codechecker_analyzer/analyzers/analyzer_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import signal
import subprocess
import sys
import shlex

from codechecker_common.logger import get_logger

Expand Down Expand Up @@ -82,7 +83,8 @@ def analyze(self, analyzer_cmd, res_handler, env=None, proc_callback=None):
"""
LOG.debug('Running analyzer ...')

LOG.debug_analyzer('\n%s', ' '.join(analyzer_cmd))
LOG.debug_analyzer('\n%s',
' '.join([shlex.quote(x) for x in analyzer_cmd]))

res_handler.analyzer_cmd = analyzer_cmd
try:
Expand Down
3 changes: 2 additions & 1 deletion analyzer/codechecker_analyzer/buildlog/build_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import os
import pickle
import platform
import shlex
import subprocess
import sys
from uuid import uuid4
Expand Down Expand Up @@ -81,7 +82,7 @@ def perform_build_command(logfile, command, context, keep_link, silent=False,
final_command = command
command = ' '.join(["intercept-build",
"--cdb", logfile,
"sh -c \"" + final_command + "\""])
"sh -c", shlex.quote(final_command)])
steakhal marked this conversation as resolved.
Show resolved Hide resolved
log_env = original_env
LOG.debug_analyzer(command)

Expand Down
7 changes: 4 additions & 3 deletions analyzer/codechecker_analyzer/buildlog/log_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -968,7 +968,8 @@ def parse_options(compilation_db_entry,

if 'arguments' in compilation_db_entry:
gcc_command = compilation_db_entry['arguments']
details['original_command'] = ' '.join(gcc_command)
details['original_command'] = \
' '.join([shlex.quote(x) for x in gcc_command])
elif 'command' in compilation_db_entry:
details['original_command'] = compilation_db_entry['command']
gcc_command = shlex.split(compilation_db_entry['command'])
Expand Down Expand Up @@ -1172,10 +1173,10 @@ def extend_compilation_database_entries(compilation_database):
continue

opts, sources = process_response_file(response_file)
cmd.extend(opts)
cmd.extend([shlex.quote(x) for x in opts])
source_files.extend(sources)
else:
cmd.append(opt)
cmd.append(shlex.quote(opt))

entry['command'] = ' '.join(cmd)

Expand Down
5 changes: 3 additions & 2 deletions analyzer/tests/libtest/codechecker.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def show(out, err):
print("\nTEST execute stderr:\n")
print(err)

cmd_log = ' '.join([shlex.quote(x) for x in cmd])
try:
proc = subprocess.Popen(
cmd,
Expand All @@ -41,13 +42,13 @@ def show(out, err):
out, err = proc.communicate()
if proc.returncode != 0:
show(out, err)
print('Unsuccessful run: "' + ' '.join(cmd) + '"')
print(f'Unsuccessful run: {cmd_log}')
print(proc.returncode)
return out, err, proc.returncode
except OSError as oerr:
print(oerr)
show(out, err)
print('Failed to run: "' + ' '.join(cmd) + '"')
print(f'Failed to run: {cmd_log}')
raise


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
'-DVARIABLE="some value"' '-DVARIABLE2="me@domain.com"'
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"directory": "/tmp",
"command": "g++ -DVARIABLE=\\\"me@domain.com\\\" /tmp/a.cpp -o /tmp/a.out",
"file": "/tmp/a.cpp"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[
{
"directory": "PLACEHOLDER",
"command": "g++ @ldlogger\\ response.rsp /tmp/a.cpp -o /tmp/a.out",
"file": "/tmp/a.cpp"
}
]
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[
{
"directory": "/tmp",
"command": "g++ /tmp/a b.cpp",
"command": "g++ /tmp/a\\ b.cpp",
"file": "/tmp/a b.cpp"
}
]
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[
{
"directory": "/tmp",
"command": "g++ -DVARIABLE=\"some value\" /tmp/a.cpp -o /tmp/a.out",
"command": "g++ -DVARIABLE=\\\"some\\ value\\\" /tmp/a.cpp -o /tmp/a.out",
"file": "/tmp/a.cpp"
}
]
50 changes: 41 additions & 9 deletions analyzer/tests/unit/test_log_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def test_new_ldlogger(self):
self.assertEqual(len(build_action.analyzer_options), 1)
self.assertTrue(len(build_action.target) > 0)
self.assertEqual(build_action.analyzer_options[0],
r'-DVARIABLE=some value')
r'-DVARIABLE="some value"')

# Test source file with spaces.
logfile = os.path.join(self.__test_files, "ldlogger-new-space.json")
Expand All @@ -115,16 +115,42 @@ def test_new_ldlogger(self):
self.assertEqual(build_action.source, r'/tmp/a b.cpp')
self.assertEqual(build_action.lang, 'c++')

# Test @ sign in variable definition.
logfile = os.path.join(self.__test_files, "ldlogger-new-at.json")

build_actions, _ = log_parser.\
parse_unique_log(load_json_or_empty(logfile), self.__this_dir)
build_action = build_actions[0]

self.assertEqual(len(build_action.analyzer_options), 1)
self.assertEqual(build_action.analyzer_options[0],
r'-DVARIABLE="me@domain.com"')

# Test the same stuff with response files.
logfile = os.path.join(self.__test_files, "ldlogger-new-response.json")
logjson = load_json_or_empty(logfile)
# Make it relative to the response file.
logjson[0]['directory'] = self.__test_files

build_actions, _ = log_parser.\
parse_unique_log(logjson, self.__this_dir)
build_action = build_actions[0]

self.assertEqual(len(build_action.analyzer_options), 2)
self.assertEqual(build_action.analyzer_options[0],
r'-DVARIABLE="some value"')
self.assertEqual(build_action.analyzer_options[1],
r'-DVARIABLE2="me@domain.com"')

def test_old_intercept_build(self):
"""
Test log file parsing escape behaviour with clang-5.0 intercept-build.
"""
logfile = os.path.join(self.__test_files, "intercept-old.json")

# FIXME: Yes, the json is actually bad! The space should have been
# escaped by intercept-build along with the backslash.
# Scan-build-py shipping with clang-5.0 makes a logfile that contains:
# -DVARIABLE=\"some value\" and --target=x86_64-linux-gnu
#
# The define is passed to the analyzer properly.
logfile = os.path.join(self.__test_files, "intercept-old.json")

build_actions, _ = log_parser.\
parse_unique_log(load_json_or_empty(logfile), self.__this_dir)
Expand All @@ -133,6 +159,8 @@ def test_old_intercept_build(self):
self.assertEqual(build_action.source, r'/tmp/a.cpp')
self.assertEqual(len(build_action.analyzer_options), 1)
self.assertTrue(len(build_action.target) > 0)
# FIXME: We should expect r'-DVARIABLE="some value"' with a fixed
# intercept-build.
self.assertEqual(build_action.analyzer_options[0],
r'-DVARIABLE="some')

Expand Down Expand Up @@ -454,7 +482,8 @@ def test_response_file_simple(self):
parse_unique_log(load_json_or_empty(logfile), self.__this_dir)
build_action = build_actions[0]
self.assertEqual(len(build_action.analyzer_options), 1)
self.assertEqual(build_action.analyzer_options[0], '-DVARIABLE=some')
self.assertEqual(build_action.analyzer_options[0],
'-DVARIABLE=some value')

def test_response_file_contains_source_file(self):
"""
Expand All @@ -481,7 +510,8 @@ def test_response_file_contains_source_file(self):

self.assertEqual(len(build_action.analyzer_options), 1)
self.assertEqual(build_action.source, self.src_file_path)
self.assertEqual(build_action.analyzer_options[0], '-DVARIABLE=some')
self.assertEqual(build_action.analyzer_options[0],
'-DVARIABLE=some value')

def test_response_file_contains_multiple_source_files(self):
"""
Expand Down Expand Up @@ -521,12 +551,14 @@ def test_response_file_contains_multiple_source_files(self):
a_build_action = [b for b in build_actions
if b.source == a_file_path][0]
self.assertEqual(len(a_build_action.analyzer_options), 1)
self.assertEqual(a_build_action.analyzer_options[0], '-DVARIABLE=some')
self.assertEqual(a_build_action.analyzer_options[0],
'-DVARIABLE=some value')

b_build_action = [b for b in build_actions
if b.source == b_file_path][0]
self.assertEqual(len(b_build_action.analyzer_options), 1)
self.assertEqual(b_build_action.analyzer_options[0], '-DVARIABLE=some')
self.assertEqual(b_build_action.analyzer_options[0],
'-DVARIABLE=some value')

def test_source_file_contains_at_sign(self):
"""
Expand Down