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

[DanBlackwell]

I have been bringing up sanitizer_common tests to run on iOS simulators, and found that a clang argument LSAN_OPTIONS=suppressions=lsan.supp was incorrectly split - this patch fixes up the environment handling behaviour in the python scripts.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Dan Blackwell (DanBlackwell)

Changes

I have been bringing up sanitizer_common tests to run on iOS simulators, and found that a clang argument LSAN_OPTIONS=suppressions=lsan.supp was incorrectly split - this patch fixes up the environment handling behaviour in the python scripts.

---

Full diff: https://github.com/llvm/llvm-project/pull/146721.diff

3 Files Affected:

• (modified) compiler-rt/test/sanitizer_common/ios_commands/iossim_env.py (+1-1)
• (modified) compiler-rt/test/sanitizer_common/ios_commands/iossim_run.py (+7-5)
• (modified) compiler-rt/test/sanitizer_common/lit.common.cfg.py (+1-1)

diff --git a/compiler-rt/test/sanitizer_common/ios_commands/iossim_env.py b/compiler-rt/test/sanitizer_common/ios_commands/iossim_env.py
index 57a01f3852b06..1bcd8e05033a5 100755
--- a/compiler-rt/test/sanitizer_common/ios_commands/iossim_env.py
+++ b/compiler-rt/test/sanitizer_common/ios_commands/iossim_env.py
@@ -8,7 +8,7 @@
   if not "=" in arg:
     break
   idx += 1
-  (argname, argval) = arg.split("=")
+  (argname, argval) = arg.split("=", maxsplit=1)
   os.environ["SIMCTL_CHILD_" + argname] = argval
 
 exitcode = subprocess.call(sys.argv[idx:])
diff --git a/compiler-rt/test/sanitizer_common/ios_commands/iossim_run.py b/compiler-rt/test/sanitizer_common/ios_commands/iossim_run.py
index 5e977ea5ed908..ddb76f0b18d63 100755
--- a/compiler-rt/test/sanitizer_common/ios_commands/iossim_run.py
+++ b/compiler-rt/test/sanitizer_common/ios_commands/iossim_run.py
@@ -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")
@@ -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 = (
@@ -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.
diff --git a/compiler-rt/test/sanitizer_common/lit.common.cfg.py b/compiler-rt/test/sanitizer_common/lit.common.cfg.py
index c3c1336bacd53..88d3ea9bc5ad2 100644
--- a/compiler-rt/test/sanitizer_common/lit.common.cfg.py
+++ b/compiler-rt/test/sanitizer_common/lit.common.cfg.py
@@ -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"]

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[padriff]

Looks good from my perspective. We'll still need someone who can approve the PR

[thurstond]

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!

[DanBlackwell]

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?

[thurstond]

Done, thank you!