Skip to content

Conversation

@mgehre-amd
Copy link
Contributor

@mgehre-amd mgehre-amd commented Jan 17, 2025

Running MLIR python tests unders asan currently fails with

executed command: 'LD_PRELOAD=$(/usr/bin/clang++-17' '-print-file-name=libclang_rt.asan-x86_64.so)' /scratch/slx-llvm/.venv-3.10/bin/python3.10 /scratch/slx-llvm/mlir/test/python/ir/context_lifecycle.py

| 'LD_PRELOAD=$(/usr/bin/clang++-17': command not found

because lit doesn't quite understand the syntax.
To fix, we resolve the path to libclang_rt.asan-x86_64.so within the lit configuration. This has the additional benefit that we don't have to call clang++ for every test.

@mgehre-amd mgehre-amd requested a review from joker-eph January 17, 2025 08:42
@llvmbot llvmbot added the mlir label Jan 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 17, 2025

@llvm/pr-subscribers-mlir

Author: Matthias Gehre (mgehre-amd)

Changes

Running MLIR python tests unders asan currently fails with


because lit doesn't quite understand the syntax.
To fix, we resolve the path to libclang_rt.asan-x86_64.so within the lit configuration. This has the additional benefit that we don't have to call clang++ for every test.


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

1 Files Affected:

  • (modified) mlir/test/lit.cfg.py (+28-11)
diff --git a/mlir/test/lit.cfg.py b/mlir/test/lit.cfg.py
index c28623123d9991..3122214be73f18 100644
--- a/mlir/test/lit.cfg.py
+++ b/mlir/test/lit.cfg.py
@@ -82,17 +82,32 @@ def add_runtime(name):
 # available. This is darwin specific since it's currently only needed on darwin.
 # Stolen from llvm/test/lit.cfg.py with a few modifications
 def get_asan_rtlib():
-    if not "asan" in config.available_features or not "Darwin" in config.host_os:
+    if not "asan" in config.available_features:
         return ""
-    # Find the asan rt lib
-    resource_dir = (
-        subprocess.check_output([config.host_cc.strip(), "-print-resource-dir"])
-        .decode("utf-8")
-        .strip()
-    )
-    return os.path.join(
-        resource_dir, "lib", "darwin", "libclang_rt.asan_osx_dynamic.dylib"
-    )
+
+    if "Darwin" in config.host_os:
+        # Find the asan rt lib
+        resource_dir = (
+            subprocess.check_output([config.host_cc.strip(), "-print-resource-dir"])
+            .decode("utf-8")
+            .strip()
+        )
+        return os.path.join(
+            resource_dir, "lib", "darwin", "libclang_rt.asan_osx_dynamic.dylib"
+        )
+    if "Linux" in config.host_os:
+        return (
+            subprocess.check_output(
+                [
+                    config.host_cxx.strip(),
+                    f"-print-file-name=libclang_rt.asan-{config.host_arch}.so",
+                ]
+            )
+            .decode("utf-8")
+            .strip()
+        )
+
+    return ""
 
 
 # On macOS, we can't do the DYLD_INSERT_LIBRARIES trick with a shim python
@@ -247,7 +262,9 @@ def find_real_python_interpreter():
 # TODO: detect Windows situation (or mark these tests as unsupported on these platforms).
 if "asan" in config.available_features:
     if "Linux" in config.host_os:
-        python_executable = f"LD_PRELOAD=$({config.host_cxx} -print-file-name=libclang_rt.asan-{config.host_arch}.so) {config.python_executable}"
+        python_executable = (
+            f"env LD_PRELOAD={find_asan_runtime()} {config.python_executable}"
+        )
     if "Darwin" in config.host_os:
         # Ensure we use a non-shim Python executable, for the `DYLD_INSERT_LIBRARIES`
         # env variable to take effect

Running MLIR python tests unders asan currently fails with
```

```
because lit doesn't quite understand the syntax.
To fix, we resolve the path to `libclang_rt.asan-x86_64.so` within the lit configuration.
This has the additional benefit that we don't have to call `clang++` for every test.
@mgehre-amd mgehre-amd force-pushed the matthias.fix_python_asan branch from 4372bf9 to 62400c8 Compare January 17, 2025 08:57
@makslevental makslevental self-requested a review January 28, 2025 17:47
Copy link
Contributor

@makslevental makslevental left a comment

Choose a reason for hiding this comment

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

Cool thanks - this is a very useful improvement because I'm always wanting to run under ASAN but never had the patience to figure out this failure.

@mgehre-amd mgehre-amd merged commit d344624 into llvm:main Jan 29, 2025
8 checks passed
@mgehre-amd mgehre-amd deleted the matthias.fix_python_asan branch January 29, 2025 07:36
mgehre-amd added a commit to Xilinx/torch-mlir that referenced this pull request Feb 24, 2025
mgehre-amd added a commit to Xilinx/torch-mlir that referenced this pull request Feb 25, 2025
mgehre-amd added a commit to llvm/torch-mlir that referenced this pull request Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants