Skip to content

[compiler-rt][sanitizer_common] Improve handling of env vars for iOS simulator tests #146721

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 3, 2025
Merged

[compiler-rt][sanitizer_common] Improve handling of env vars for iOS simulator tests #146721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions compiler-rt/test/sanitizer_common/ios_commands/iossim_env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

idx = 1
for arg in sys.argv[1:]:
if not "=" in arg:
break
idx += 1
(argname, argval) = arg.split("=")
os.environ["SIMCTL_CHILD_" + argname] = argval
if not "=" in arg:
break
idx += 1
(argname, argval) = arg.split("=", maxsplit=1)
Copy link
Contributor

@thurstond thurstond Jul 2, 2025

Choose a reason for hiding this comment

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

The commit message mentions "a clang argument LSAN_OPTIONS=suppressions=lsan.supp was incorrectly split" and it looks like this line is the core fix, but there's also some other minor non-whitespace fixups (e.g., iossim_run.py avoids overwriting os.environ[simctl_version] if already set) - please mention them in the commit message too.

Other than that, looks good to me; thanks for the patch!

Copy link
Contributor Author

@DanBlackwell DanBlackwell Jul 3, 2025

Choose a reason for hiding this comment

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

Thanks for the review! I've squashed in the formatting commit and updated the message to reflect all of the changes that have been made. Could you please hit merge for me if it looks ok?

Copy link
Contributor

@thurstond thurstond Jul 3, 2025

Choose a reason for hiding this comment

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

Done, thank you!

os.environ["SIMCTL_CHILD_" + argname] = argval

exitcode = subprocess.call(sys.argv[idx:])
if exitcode > 125:
exitcode = 126
exitcode = 126
sys.exit(exitcode)
12 changes: 7 additions & 5 deletions compiler-rt/test/sanitizer_common/ios_commands/iossim_run.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env python3

import glob, os, pipes, sys, subprocess
import glob, os, shlex, sys, subprocess


device_id = os.environ.get("SANITIZER_IOSSIM_TEST_DEVICE_IDENTIFIER")
Expand All @@ -21,8 +21,11 @@
"ASAN_ACTIVATION_OPTIONS",
"MallocNanoZone",
]:
if e in os.environ:
os.environ["SIMCTL_CHILD_" + e] = os.environ[e]
simctl_version = "SIMCTL_CHILD_" + e
# iossim_env.py might have already set these using arguments it was given
# (and that we can't see from inside this script). Don't overwrite them!
if e in os.environ and simctl_version not in os.environ:
os.environ[simctl_version] = os.environ[e]

find_atos_cmd = "xcrun -sdk iphonesimulator -f atos"
atos_path = (
Expand All @@ -49,8 +52,7 @@
# Don't quote glob pattern
rm_args.append(arg)
else:
# FIXME(dliew): pipes.quote() is deprecated
rm_args.append(pipes.quote(arg))
rm_args.append(shlex.quote(arg))
rm_cmd_line = ["/bin/rm"] + rm_args
rm_cmd_line_str = " ".join(rm_cmd_line)
# We use `shell=True` so that any wildcard globs get expanded by the shell.
Expand Down
2 changes: 1 addition & 1 deletion compiler-rt/test/sanitizer_common/lit.common.cfg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def build_invocation(compile_flags):
config.substitutions.append(("%tool_name", config.tool_name))
config.substitutions.append(("%tool_options", tool_options))
config.substitutions.append(
("%env_tool_opts=", "env " + tool_options + "=" + default_tool_options_str)
("%env_tool_opts=", "%env " + tool_options + "=" + default_tool_options_str)
)

config.suffixes = [".c", ".cpp"]
Expand Down
Loading