Skip to content
Closed
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
33 changes: 25 additions & 8 deletions scripts/update_lit_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import glob
import os
import re
import shlex
import subprocess
import sys
import tempfile
Expand Down Expand Up @@ -109,30 +110,46 @@ def find_run_lines(test, lines):
def run_command(args, test, tmp, command):
env = dict(os.environ)
env['PATH'] = args.binaryen_bin + os.pathsep + env['PATH']
command = command.replace('%s', test)
command = command.replace('%S', os.path.dirname(test))
command = command.replace('%t', tmp)

match = re.match(r'^(.*)\s*foreach\s+(\S+)\s+(\S+)\s+(.*)$', command)
def replace_paths(text):
return text.replace('%s', test).replace('%S', os.path.dirname(test)).replace('%t', tmp)
Comment on lines +114 to +115

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using sequential .replace() calls can lead to unexpected behavior if one of the replacement values (such as test or tmp paths) contains another placeholder substring (e.g., if the test path contains %t or %S). In such cases, the subsequent .replace() calls will perform recursive replacements on the already-replaced path, corrupting the final command.

Using a single-pass regular expression replacement ensures that each placeholder is replaced exactly once and prevents recursive replacement bugs.

Suggested change
def replace_paths(text):
return text.replace('%s', test).replace('%S', os.path.dirname(test)).replace('%t', tmp)
def replace_paths(text):
replacements = {
'%s': test,
'%S': os.path.dirname(test),
'%t': tmp
}
return re.sub(r'%[sSt]', lambda m: replacements[m.group()], text)


def quote_path(path):
return subprocess.list2cmdline([path]) if os.name == 'nt' else shlex.quote(path)

def replace_shell_paths(text):
def replace(match):
token = match.group()
if not any(path in token for path in ('%s', '%S', '%t')):
return token
if token[0] in {'"', "'"}:
token = token[1:-1]
return quote_path(replace_paths(token))

return re.sub(r'''"[^"]*"|'[^']*'|%[sSt]''', replace, text)

# Parse foreach arguments before expanding paths, which can contain spaces.
argument = r'''(?:"[^"]*"|'[^']*'|\S+)'''
match = re.match(r'^(.*)\s*foreach\s+(' + argument + r')\s+(' + argument + r')\s+(.*)$', command)
if match:
prefix = match.group(1)
infile = match.group(2)
tempfile = match.group(3)
infile = replace_paths(match.group(2).strip('\'"'))
tempfile = replace_paths(match.group(3).strip('\'"'))
cmd_rest = match.group(4)

outputs = []
for i, (module, _asserts) in enumerate(support.split_wast(infile)):
tempname = tempfile + '.' + str(i)
with open(tempname, 'w') as temp:
print(module, file=temp)
new_command = prefix + ' ' + cmd_rest + ' ' + tempname
new_command = replace_shell_paths(prefix + ' ' + cmd_rest) + ' ' + quote_path(tempname)
out = subprocess.check_output(new_command, shell=True, env=env).decode('utf-8')
outputs.append(out)

return f"\n{INTERNAL_SEPARATOR}\n".join(outputs)
else:
assert 'foreach' not in command, 'bad foreach matching: ' + command
return subprocess.check_output(command, shell=True, env=env).decode('utf-8')
return subprocess.check_output(replace_shell_paths(command), shell=True, env=env).decode('utf-8')


def find_end(module, start):
Expand Down
38 changes: 38 additions & 0 deletions test/unit/test_update_lit_checks_paths.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import os
import subprocess
import sys
import tempfile
from pathlib import Path

from scripts.test import shared

from . import utils


class UpdateLitChecksPathsTest(utils.BinaryenTestCase):
def test_paths_with_spaces(self):
with tempfile.TemporaryDirectory(prefix='update lit checks ') as temp:
root = Path(temp)
# Exercise %s, %S, and %t, with both quoted and unquoted RUN lines.
for run in (
'wasm-opt %s -S -o -',
'wasm-opt "%s" -S -o -',
'wasm-opt "%S/input file.wat" -S -o %t.wat && wasm-opt %t.wat -S -o -',
'foreach %s %t wasm-opt -S -o -',
'foreach "%s" "%t" wasm-opt -S -o -',
):
with self.subTest(run=run):
test = root / 'input file.wat'
test.write_text(';; RUN: ' + run + ' | filecheck %s\n'
'(module\n'
' (func $answer (result i32) (i32.const 42))\n'
')\n')
env = dict(os.environ, TMPDIR=temp, TEMP=temp, TMP=temp)
command = [sys.executable,
os.path.join(shared.options.binaryen_root, 'scripts', 'update_lit_checks.py'),
'--binaryen-bin', shared.options.binaryen_bin,
'--force', '--dry-run', str(test)]
result = subprocess.run(command, env=env, capture_output=True, text=True)
self.assertEqual(result.returncode, 0, result.stderr)
self.assertIn(';; CHECK:', result.stdout)
self.assertIn('(i32.const 42)', result.stdout)