-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[ASan] Make dyld_insert_libraries_reexec work with internal shell #168655
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
[ASan] Make dyld_insert_libraries_reexec work with internal shell #168655
Conversation
Created using spr 1.3.7 [skip ci]
Created using spr 1.3.7
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Aiden Grossman (boomanaiden154) ChangesThis test was doing some feature checks within the test itself. This patch Full diff: https://github.com/llvm/llvm-project/pull/168655.diff 2 Files Affected:
diff --git a/compiler-rt/test/asan/TestCases/Darwin/dyld_insert_libraries_reexec.cpp b/compiler-rt/test/asan/TestCases/Darwin/dyld_insert_libraries_reexec.cpp
index 145e162a21c0e..89ee7a178525a 100644
--- a/compiler-rt/test/asan/TestCases/Darwin/dyld_insert_libraries_reexec.cpp
+++ b/compiler-rt/test/asan/TestCases/Darwin/dyld_insert_libraries_reexec.cpp
@@ -14,23 +14,12 @@
// RUN: %run %t/a.out 2>&1 \
// RUN: | FileCheck %s
-// RUN: MACOS_MAJOR=$(sw_vers -productVersion | cut -d'.' -f1)
-// RUN: MACOS_MINOR=$(sw_vers -productVersion | cut -d'.' -f2)
-
-// RUN: IS_MACOS_10_11_OR_HIGHER=$([ $MACOS_MAJOR -eq 10 ] && [ $MACOS_MINOR -lt 11 ]; echo $?)
-
// On OS X 10.10 and lower, if the dylib is not DYLD-inserted, ASan will re-exec.
-// RUN: if [ $IS_MACOS_10_11_OR_HIGHER == 0 ]; then \
-// RUN: %env_asan_opts=verbosity=1 %run %t/a.out 2>&1 \
-// RUN: | FileCheck --check-prefix=CHECK-NOINSERT %s; \
-// RUN: fi
+// RUN: %if mac-os-10-11-or-higher %{ %env_asan_opts=verbosity=1 %run %t/a.out 2>&1 | FileCheck --check-prefix=CHECK-NOINSERT %s %}
// On OS X 10.11 and higher, we don't need to DYLD-insert anymore, and the interceptors
// still installed correctly. Let's just check that things work and we don't try to re-exec.
-// RUN: if [ $IS_MACOS_10_11_OR_HIGHER == 1 ]; then \
-// RUN: %env_asan_opts=verbosity=1 %run %t/a.out 2>&1 \
-// RUN: | FileCheck %s; \
-// RUN: fi
+// RUN: %if mac-os-10-10-or-lower %{ %env_asan_opts=verbosity=1 %run %t/a.out 2>&1 | FileCheck %s %}
#include <stdio.h>
diff --git a/compiler-rt/test/asan/TestCases/Darwin/lit.local.cfg.py b/compiler-rt/test/asan/TestCases/Darwin/lit.local.cfg.py
index af82d30cf4de9..b09c1f7cd3daa 100644
--- a/compiler-rt/test/asan/TestCases/Darwin/lit.local.cfg.py
+++ b/compiler-rt/test/asan/TestCases/Darwin/lit.local.cfg.py
@@ -1,3 +1,6 @@
+import subprocess
+
+
def getRoot(config):
if not config.parent:
return config
@@ -8,3 +11,25 @@ def getRoot(config):
if root.target_os not in ["Darwin"]:
config.unsupported = True
+
+
+def get_product_version():
+ try:
+ version_process = subprocess.run(
+ ["sw_vers", "-productVersion"],
+ check=True,
+ stdout=subprocess.PIPE,
+ stderr=subprocess.PIPE,
+ )
+ version_string = version_process.stdout.decode("utf-8").split("\n")[0]
+ version_split = version_string.split(".")
+ return (int(version_split[0]), int(version_split[1]))
+ except:
+ return (0, 0)
+
+
+macos_version_major, macos_version_minor = get_product_version()
+if macos_version_major > 10 and macos_version_minor > 11:
+ config.available_features.add("mac-os-10-11-or-higher")
+else:
+ config.available_features.add("mac-os-10-10-or-lower")
|
This test was doing some feature checks within the test itself. This patch rewrites the feature checks to be done in a fashion more idiomatic to lit, as the internal shell does not support the features needed for the previous feature checks. Pull Request: llvm#168655
|
|
||
| macos_version_major, macos_version_minor = get_product_version() | ||
| if macos_version_major > 10 and macos_version_minor > 11: | ||
| config.available_features.add("mac-os-10-11-or-higher") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should only add this feature when config.apple_platform == "osx" (ref). This particular test has // UNSUPPORTED: ios so it does not break anything right now, but it's not ideal to have a feature set based on the host OS if we are running on a simulator/device.
Created using spr 1.3.7 [skip ci]
This test was doing some feature checks within the test itself. This patch rewrites the feature checks to be done in a fashion more idiomatic to lit, as the internal shell does not support the features needed for the previous feature checks. Pull Request: llvm#168655
🐧 Linux x64 Test Results
|
This test was doing some feature checks within the test itself. This patch rewrites the feature checks to be done in a fashion more idiomatic to lit, as the internal shell does not support the features needed for the previous feature checks. Pull Request: llvm#168655
…l shell This test was doing some feature checks within the test itself. This patch rewrites the feature checks to be done in a fashion more idiomatic to lit, as the internal shell does not support the features needed for the previous feature checks. Reviewers: ndrewh, DanBlackwell, fmayer Reviewed By: ndrewh Pull Request: llvm/llvm-project#168655
This test was doing some feature checks within the test itself. This patch
rewrites the feature checks to be done in a fashion more idiomatic to lit,
as the internal shell does not support the features needed for the previous
feature checks.