Skip to content

test(amdsmi): cover loader resilience under a restrictive import guard - #9879

Merged
marifamd merged 3 commits into
developfrom
users/marifamd/amdsmi-loader-import-guard-test
Aug 31, 2026
Merged

marifamd merged 3 commits into
developfrom
users/marifamd/amdsmi-loader-import-guard-test

Conversation

@marifamd

@marifamd marifamd commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Motivation

The Python wrapper loader must survive a hostile import environment set up by a downstream consumer. rocprofiler-compute runs amd-smi under a profile-mode import guard that raises a bare ImportError (the parent of ModuleNotFoundError) for any non-whitelisted top-level package. A past loader change imported a third-party package in its search path but caught only (ModuleNotFoundError, FileNotFoundError), so the guard's ImportError escaped, aborted import amdsmi, and broke 21 tests in TheRock CI, forcing a revert. Nothing in amd-smi's own suite catches that failure class today.

Technical Details

Stacked on #9695. Adds one test to projects/amdsmi/tests/python/test_abi_compat.py:

  • _ProfileModeLikeGuard: a minimal clone of the downstream guard. Allows stdlib plus a small ROCm whitelist and raises ImportError for every other top-level package, from both a meta_path finder and a builtins.__import__ hook. rocm_sdk is deliberately left off the whitelist.
  • RestrictiveImportGuardTest.test_loader_survives_restrictive_import_guard: reimports the wrapper under that guard, reusing the AMDSMI_LIB_OVERRIDE escape hatch and the existing _Patch / STABLE_SYMBOLS fake-CDLL harness, then asserts the loader still resolves the override path.
  • Skipped below Python 3.10 because it relies on sys.stdlib_module_names.

If any unguarded third-party import creeps back into the load path, this fails here in amd-smi CI instead of downstream in TheRock.

Issue Tracking

JIRA ID: ROCM-3941

Test Plan

  • cd projects/amdsmi/tests/python && python3 -m unittest test_abi_compat -v
  • pre-commit run --config projects/amdsmi/.pre-commit-config.yaml --files projects/amdsmi/tests/python/test_abi_compat.py

Test Result

  • Ran 11 tests in 0.166s / OK (10 existing + the new RestrictiveImportGuardTest).
  • pre-commit: all hooks pass (codespell, ruff-format, and the rest).

Submission Checklist

@marifamd
marifamd requested a review from a team as a code owner August 7, 2026 21:12
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/dgalants/amdsmi-packaging-scriptlet-scope branch from f1ece37 to 997ec8a Compare August 10, 2026 16:50
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/marifamd/amdsmi-loader-import-guard-test branch from ee64282 to 3d72487 Compare August 10, 2026 16:50
@therock-pr-bot

therock-pr-bot Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/dgalants/amdsmi-packaging-scriptlet-scope branch from 997ec8a to d055df1 Compare August 11, 2026 22:01
@dmitrii-galantsev
dmitrii-galantsev requested a review from a team as a code owner August 11, 2026 22:01
Base automatically changed from users/dgalants/amdsmi-packaging-scriptlet-scope to develop August 12, 2026 21:21
@github-actions github-actions Bot added the github actions Pull requests that update GitHub Actions code label Aug 28, 2026
@dmitrii-galantsev
dmitrii-galantsev force-pushed the users/marifamd/amdsmi-loader-import-guard-test branch from 092ad8a to c2dcef8 Compare August 28, 2026 16:26

@dmitrii-galantsev dmitrii-galantsev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

claude:

Rebased onto current develop and now a clean single-file change to test_abi_compat.py, with the already-merged #9695 content, the SPDX-header reverts, and the configure-breaking CMakeLists.txt changes all dropped. The mutation test was re-run after the rebase and still fails without the guarded branch, so the coverage is real — LGTM.

marifamd and others added 2 commits August 29, 2026 15:32
The wrapper loader must not abort import when a downstream consumer
installs a restrictive import guard. rocprofiler-compute runs amd-smi
under a profile-mode guard that raises a bare ImportError (the parent of
ModuleNotFoundError) for any non-whitelisted top-level package. A prior
loader change imported a third-party package in its search path and
caught only (ModuleNotFoundError, FileNotFoundError), so the guard's
ImportError escaped and broke import amdsmi in downstream CI.

Add RestrictiveImportGuardTest, which reimports the wrapper under a
minimal clone of that guard with the offending package left off the
whitelist, and asserts the loader still resolves the override path. Any
unguarded third-party import that returns to the load path now fails in
amd-smi CI instead of downstream.

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
AMDSMI_LIB_OVERRIDE short-circuits _load_library() at step 1, so the guard
test never entered the relocatable-ROCm-tree branch -- the one place a
rocm_sdk import would plausibly live, and the exact regression this test
was added for. A mutation injected there passed unnoticed. Add a case with
no override so the loader walks steps 2-4, and assert on the resolved path
so the test reports more than "the import did not raise".

Drop the 3.10-only sys.stdlib_module_names dependency: it was incidental
but skipped 4 of the 9 CI matrix legs, while pyproject declares >=3.6.

Short-circuit the guard on already-imported modules; the guard window is
process-wide, and failing a cached re-import is hostile to any in-process
tracer or plugin rather than a property worth asserting.
@marifamd
marifamd force-pushed the users/marifamd/amdsmi-loader-import-guard-test branch from c2dcef8 to 88d7a94 Compare August 29, 2026 20:32
@marifamd
marifamd merged commit b3616e4 into develop Aug 31, 2026
64 of 65 checks passed
@marifamd
marifamd deleted the users/marifamd/amdsmi-loader-import-guard-test branch August 31, 2026 22:04
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

dmitrii-galantsev added a commit that referenced this pull request Nov 6, 2026
…#14510)

## Motivation

TheRock's `rocm-sdk-core` wheel carries the `amdsmi` Python module only
at `_rocm_sdk_core/share/amd_smi` (since #9340). Copied into
site-packages, `import amdsmi; amdsmi.amdsmi_init()` fails with
`libamd_smi.so.27: cannot open shared object file` unless `torch` was
imported first, because only PyTorch's preload makes the library
findable.

## Technical Details

`_load_library()` in `amdsmi_wrapper.py` (mirrored in
`tools/generator.py`):

- **`_rocm_sdk_core` lookup.** When no system library is found or the
one found is refused, the loader takes `lib/libamd_smi.so.27` from the
`_rocm_sdk_core` package (`sys.modules`, else `sys.path`). It checks
paths only and imports nothing, so import guards like
rocprofiler-compute's cannot break it (#9879).
- **`_AMDSMI_ROCM_SDK_CORE` flag.** TheRock sets it in the copy it ships
in `rocm-sdk-core` (ROCm/TheRock#9578). That copy then uses the
relocatable path and `_rocm_sdk_core` only, never a system
`libamd_smi.so` from another release. The rpm/deb copy keeps its order
and only gains the fallback.
- **Reuse before load.** Each candidate is first opened with
`RTLD_NOLOAD`. PyTorch 2.14's `_amdsmi_cdll_hook` (in the ROCm 10.1.0
wheels; narrowed upstream in pytorch/pytorch#197482) redirects any
`libamd_smi.so*` load to `$ROCM_PATH/lib/libamd_smi.so` first. With
`ROCM_PATH` set and a system ROCm installed, the module loaded that
second copy and both `torch.cuda.device_count()` and amdsmi reported 0
GPUs. Asked with `RTLD_NOLOAD`, the hook can only return a copy already
in the process.
- **Library version check.** SONAME 27 changed struct sizes between 27.1
and 27.2: `amdsmi_fabric_info_t` grew from 320 to 552 bytes when its
union gained `v2`, and 10.2 nightlies built before #13926 have a
1792-byte `amdsmi_driver_info_t` that `amdsmi_get_gpu_driver_info()`
zeroes in full, against the 768 bytes a 27.1 module allocates. A library
newer than the module's `_version.py` is closed and skipped, so it
cannot write past the module's buffers. An older one warns.
`AMDSMI_LIB_OVERRIDE` still loads anything.
- **Version stays in sync.** `get_version_from_file` makes CMake
reconfigure when `amdsmi.h` changes, so `_version.py` cannot lag the
library in an incremental build.
- **Docs.** `docs/packaging.md`, `docs/install/install.md` and
`py-interface/README.md` describe the new loader order, the flag,
`RTLD_NOLOAD` reuse and the version check.

This does not make `share/amd_smi` pip-installable again. A module
installed from there would not depend on `rocm-sdk-core` and would go
stale on the next ROCm upgrade.

## Issue Tracking

JIRA ID: ROCM-33336
Related: ROCM-31613, ROCm/TheRock#9226 (with #14409), ROCm/TheRock#3054

## Test Plan

- Python loader tests (`tests/python/test_abi_compat.py`): flag on and
off, the fallback, `RTLD_NOLOAD` reuse, newer and older libraries,
closing a refused library, unusable `sys.path` entries. Each new check
was disabled once to confirm its test fails.
- The 10.1.0 `rocm-sdk-core` module patched with this loader, on the
shipped wheels (`rocm-sdk-core` 10.1.0, `torch 2.14.0+rocm10.1.0`), 2×
MI210, host with a 10.2 ROCm in `ld.so.cache` and `ROCM_PATH` set:
  - copied into site-packages and via `PYTHONPATH`;
  - imported alone, before torch and after torch;
  - also with `/opt` and `ld.so.cache` hidden;
  - also with the 10.2 library loaded first.
- The `amd-smi` CLI from an install tree and from `rocm-sdk-core`'s
entry point.
- A version bump in `amdsmi.h` followed by `ninja`: reconfigures and
updates `_version.py`.
- The `DESTDIR` install: the site-packages copy matches `share/amd_smi`,
`_version.py` included.

## Test Result

- Python tests: 55 passed.
- Module, every case: one `libamd_smi` mapped, torch and amdsmi each
report 2 GPUs.
- With a newer 10.2 library already loaded before PyTorch 2.14's hook
imports the module, the module refuses it with the version error rather
than binding it.
- `amd-smi version` and `amd-smi list`: work from both locations.

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

---------

Signed-off-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github actions Pull requests that update GitHub Actions code organization: ROCm project: amdsmi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants