Repository navigation
fix(ci): make host-asan test jobs find and use the ASAN runtime #8268
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,6 +29,7 @@ | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import argparse | ||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||
| import platform | ||||||||||||||||||||||||||
| import subprocess | ||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||||||
|
|
@@ -58,7 +59,14 @@ | |||||||||||||||||||||||||
| "HSA_XNACK": "1", | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ASAN_RUNTIME_LIB = "libclang_rt.asan.so" | ||||||||||||||||||||||||||
| # Which of these exists depends on LLVM_ENABLE_PER_TARGET_RUNTIME_DIR: ON | ||||||||||||||||||||||||||
| # installs <resource-dir>/lib/<triple>/libclang_rt.asan.so, OFF installs | ||||||||||||||||||||||||||
| # <resource-dir>/lib/linux/libclang_rt.asan-<arch>.so. TheRock has flipped | ||||||||||||||||||||||||||
| # between the two (#8077), so try both rather than pinning one. | ||||||||||||||||||||||||||
| ASAN_RUNTIME_LIBS = ( | ||||||||||||||||||||||||||
| "libclang_rt.asan.so", | ||||||||||||||||||||||||||
| f"libclang_rt.asan-{platform.machine()}.so", | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def _resolve_asan_runtime( | ||||||||||||||||||||||||||
|
|
@@ -72,20 +80,23 @@ def _resolve_asan_runtime( | |||||||||||||||||||||||||
| if not os.access(clang, os.X_OK): | ||||||||||||||||||||||||||
| return None, f"clang not found at {clang}, ASAN runtime path not resolved" | ||||||||||||||||||||||||||
|
Comment on lines
62
to
81
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have some other code across the project that is also checking for the asan runtime path, such as TheRock/build_tools/github_actions/test_executable_scripts/test_hiptests.py Lines 109 to 120 in 8438943
I also want our tests to match our documentation in more places, e.g. https://github.com/ROCm/TheRock/blob/main/docs/development/sanitizers.md#step-2-locate-the-asan-runtime-directory I think this is fine for now though. |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||
| result = subprocess.run( | ||||||||||||||||||||||||||
| [str(clang), f"-print-file-name={ASAN_RUNTIME_LIB}"], | ||||||||||||||||||||||||||
| capture_output=True, | ||||||||||||||||||||||||||
| text=True, | ||||||||||||||||||||||||||
| check=True, | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| except (subprocess.CalledProcessError, OSError) as e: | ||||||||||||||||||||||||||
| return None, f"could not query {clang} for the ASAN runtime: {e}" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| runtime = Path(result.stdout.strip()) | ||||||||||||||||||||||||||
| if not runtime.is_file(): | ||||||||||||||||||||||||||
| return None, f"ASAN runtime not found at {runtime}" | ||||||||||||||||||||||||||
| return runtime.resolve(), None | ||||||||||||||||||||||||||
| for lib in ASAN_RUNTIME_LIBS: | ||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||
| result = subprocess.run( | ||||||||||||||||||||||||||
| [str(clang), f"-print-file-name={lib}"], | ||||||||||||||||||||||||||
| capture_output=True, | ||||||||||||||||||||||||||
| text=True, | ||||||||||||||||||||||||||
| check=True, | ||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||
| except (subprocess.CalledProcessError, OSError) as e: | ||||||||||||||||||||||||||
| return None, f"could not query {clang} for the ASAN runtime: {e}" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| # clang echoes the name back unchanged when it cannot locate the file. | ||||||||||||||||||||||||||
| runtime = Path(result.stdout.strip()) | ||||||||||||||||||||||||||
| if runtime.is_file(): | ||||||||||||||||||||||||||
| return runtime.resolve(), None | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return None, f"ASAN runtime not found, tried {', '.join(ASAN_RUNTIME_LIBS)}" | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| def _resolve_symbolizer(artifacts_dir: Path) -> tuple[Optional[Path], Optional[str]]: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
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.
Expanding this is also going to skip our test for
rocminfohere:TheRock/tests/test_rocm_sanity.py
Lines 58 to 63 in 8438943
We may want to differentiate between host asan and device asan in test skipping.
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.
Agree!
I'll get a new PR for it after merging, and update this comment as well.