fix(rccl): fix ASAN GPU target selection, and propagate ASAN to rocSHMEM by lfmeadow · Pull Request #12314 · ROCm/rocm-systems

lfmeadow · 2026-09-26T00:27:54Z

JIRA ID : LCOMPILER-2794

ASAN builds were rewriting GPU targets to include :xnack+. This is needed for
gfx9 but not for gfx1250, and in fact not allowed there — gfx1250:xnack+ is a
hard invalid target ID error from clang, not a warning.

What changed

Replaces the rewrite with an explicit per-family policy in
cmake/AsanGpuTargets.cmake:

family	rule
`gfx9*`	normalise to `:xnack+`, replacing any `:xnack-` the caller asked for and preserving other feature terms such as `sramecc+`
`gfx1250`, `gfx1250-strict`	xnack is always on; strip any explicit xnack term, which would otherwise be rejected
everything else	drop, with a message

Dropping is deliberate. Clang accepts -fsanitize=address together with an
offload arch that cannot support it and merely warns, so leaving such a target
in the list produces a silently uninstrumented device image inside an ASAN
build.

rocSHMEM

Two changes. -DASAN=ON is now passed to the rocSHMEM ExternalProject: under
ENABLE_ROCSHMEM, librocshmem.a is linked into librccl.so, so an ASAN build
of rccl was shipping an uninstrumented rocSHMEM inside an instrumented library —
precisely the allocations ASAN is there to police.

rocSHMEM's own ASAN block is rewritten to the same allowlist shape. It
previously used a two-entry denylist (gfx1100, gfx1201), which covered
rocSHMEM's own defaults but let a user-supplied target such as gfx1030 or
gfx1151 through uninstrumented. The policy is repeated inline rather than
shared because rccl builds rocSHMEM as a separate cmake process.

Tests

tools/scripts/test_runner/tests/test_asan_gpu_targets.py — 11 cases driving the
function through cmake -P, no configure and no compiler. Every case is one row
of the table above, so reclassifying a family fails a test instead of producing a
silently uninstrumented image.

Scope: this does not by itself instrument device code

Worth being explicit, since the obvious reading of the title is that gfx9 device
ASAN works after this. It does not yet.

cmake/DeviceLinker.cmake reduces GPU_TARGETS to bare arch names before
compiling, so on develop the device compile receives --arch=gfx942 alongside
-fsanitize=address and clang responds:

warning: ignoring '-fsanitize=address' option for offload arch 'gfx942' as it is
not currently supported there. Use it with an offload arch containing 'xnack+'
instead [-Woption-ignored]

Measured on a clean gfx942 ASAN build of this branch: the linked device.elf
and the gfx942 entry in .hip_fatbin both carry ELF flags 0x54c (xnack,
i.e. ANY) and zero asan symbols, while host code in librccl.so has 45 __asan
dynamic symbols. For comparison, a device image built correctly as
gfx942:xnack+ carries flags 0x74c and 76 asan symbols.

Carrying the full target ID through the device linker is #12394. The two PRs are
independent and neither breaks the other in either merge order, but gfx9 device
ASAN only actually instruments once both have landed. What this PR fixes on its
own is gfx1250, where the appended :xnack+ was a hard compile error rather
than a silent strip, and the uninstrumented-target-in-an-ASAN-build class of
problem generally.

Notes for review

One of the PRs replacing the closed #9763 and #12221.

therock-pr-bot · 2026-09-26T00:28:28Z

✅ 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?

lfmeadow · 2026-09-26T20:09:22Z

@argus re-review

lfmeadow · 2026-09-26T20:36:28Z

@argus re-review

lfmeadow · 2026-09-27T04:04:41Z

@argus re-review

lfmeadow · 2026-09-27T04:19:41Z

@argus re-review

amd-argus

ARGUS

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

alex-breslow-amd

LGTM. I will find someone who works on rocshmem to help review.

amd-argus

ARGUS

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

amirsojoodi

Thanks for PR. Some of these changes were actually necessary!
Just one minor comment added below.

I haven't seen any failures during the build and the tests (w/o ASAN) locally. It would be great if we don't see any failures in the tests too. I added ci:asan to kick off the CI's ASAN pipeline.

I'd suggest @drprajap, @edgargabriel, or @abouteiller approve the PR before merge.

An ASAN build rewrote every requested GPU target to <arch>:xnack+. That is right for gfx9, where xnack is a selectable target-ID feature and a bare arch name compiles with -fsanitize=address silently ignored. It is wrong for everything else: - gfx1250 has xnack always on, so the feature is not selectable and "gfx1250:xnack+" is an invalid target ID. rocm_check_target_ids then dropped it without comment, so asking for gfx1250 in an ASAN build quietly built nothing for gfx1250. - gfx10xx/gfx11xx/gfx12xx have no xnack at all. Appending :xnack+ made them invalid too, so they were also silently dropped. - the test for an existing suffix looked for the substring ":xnack+", which does not match ":xnack-". A requested gfx942:xnack- became "gfx942:xnack-:xnack+". Replace the rewrite with an explicit per-family policy in cmake/AsanGpuTargets.cmake: normalise gfx9 to :xnack+, pass always-on-xnack parts through untouched, and drop the rest with a message rather than leaving a target that would produce an uninstrumented device image inside an ASAN build. Fail the configure if nothing survives. Each rule was verified against amdclang++ and is pinned by tests/test_asan_gpu_targets.py. Also pass -DASAN=ON to the rocSHMEM ExternalProject. Under ENABLE_ROCSHMEM, librocshmem.a is linked into librccl.so, so an ASAN build of rccl was shipping an uninstrumented rocSHMEM inside an instrumented library -- precisely the allocations ASAN is there to police. rocSHMEM's own ASAN block gets the same gfx9-only restriction, since it cannot include rccl's module: rccl configures it as a separate cmake process, and it also builds standalone.

The policy ran after rocm_check_target_ids, so the one input it exists to rewrite never reached it: gfx1250:xnack+ is a hard "invalid target ID" error from clang, not a warning, so validation dropped it and an ASAN configure died with "ASAN requires a GPU target that supports xnack" instead of normalising to gfx1250. Reproduced with a standalone rocSHMEM configure before the change. Moving the policy above validation also collapses it: SUPPORTED_GPUS now comes back already normalised, so GPU_TARGETS and GPU_TARGETS_FULL both inherit it and the two-list foreach goes away. It is a function now, matching the shape of rccl's AsanGpuTargets.cmake. It also no longer sits inside NOT BUILD_TESTS_ONLY, where an ASAN build previously got unnormalised targets. BITCODE_GPU_ARCHS_FULL gets the same FORCE treatment as GPU_BUILD_TARGETS just above it. cmake/DeviceBitcode.cmake caches it without FORCE, so a build directory first configured without ASAN kept a bare gfx942 while GPU_TARGETS had correctly become gfx942:xnack+, and the bitcode compile lost the feature. Verified by standalone rocSHMEM configures: gfx1250:xnack+ -> gfx1250 (was fatal), gfx942 -> gfx942:xnack+, gfx942:sramecc+:xnack- -> gfx942:sramecc+:xnack+, defaults drop gfx1100 and gfx1201, a non-ASAN configure is untouched, and a no-ASAN-then-ASAN reconfigure of one build directory now updates BITCODE_GPU_ARCHS_FULL where it previously stayed stale.

GPU_TARGETS_FULL was set only when the user passed -DGPU_TARGETS. That was right when the defaults path could not produce a feature suffix, but the ASAN policy now gives it one, so a default-target ASAN configure left GPU_TARGETS_FULL unset and DeviceBitcode.cmake fell back to the bare _BITCODE_DEFAULT_ARCHS -- rocSHMEM bitcode compiled without :xnack+ and therefore uninstrumented, the exact failure this PR exists to close. Key the branch on whether SUPPORTED_GPUS actually carries a suffix instead. The clearing arm keeps its stale-value protection and loses nothing, since DeviceBitcode.cmake's fallback is the same bare list it would have been given. Before: defaults + ASAN gave GPU_TARGETS=gfx90a:xnack+;gfx942:xnack+;gfx950:xnack+;gfx1250 but BITCODE_GPU_ARCHS_FULL=gfx90a;gfx942;gfx950;gfx1250. Both now match.

Moving the policy above the GPU_TARGETS cache write made its result the value that gets FORCE-cached, so the next configure of the same build directory read it back as input. A later -DASAN=OFF then kept the :xnack+ and never brought back the dropped families: develop restores gfx1100 and gfx1201, this branch did not. Remember GPU_TARGETS as it arrived and re-derive from that each time; GPU_TARGETS still matching what we wrote last time means nobody has edited it, anything else is a user edit and becomes the new request. BITCODE_GPU_ARCHS_FULL gets the else arm its GPU_TARGETS_FULL counterpart already had, so a suffixed value cannot outlive a configure that resolves to bare archs. The policy function now mirrors cmake/AsanGpuTargets.cmake name for name. _always_on held the list here and the list(FIND) index there, so one name meant opposite things in two copies that have to stay in sync. On the rccl side, ExternalProject stamps its build step on completion rather than on BUILD_COMMAND text, so flipping ASAN over an existing tree kept the previous uninstrumented librocshmem.a. Writing the options to a file and hanging the build step off it re-runs the sub-build exactly when they change, which BUILD_ALWAYS would also do but at the cost of every invocation. Verified on both Unix Makefiles and Ninja. The pre-built ROCSHMEM_INSTALL_DIR path returns before the ASAN propagation, so it now warns instead of silently linking a possibly uninstrumented archive into an ASAN rccl. Tests: gfx1250-strict is pinned (it was the one entry of the always-on list no case reached, so deleting it left all cases green), and adjust() returns stderr as well as stdout so the tests do not pin the policy to a message level.

cmake/DeviceBitcode.cmake invokes clang directly with its own BITCODE_COMPILE_FLAGS_BASE, so it inherited none of the ASAN flags the rest of the build picks up from CMAKE_CXX_FLAGS. Propagating -DASAN=ON therefore left librocshmem_device_*.bc uninstrumented, and rccl's device linker injects that bitcode for ENABLE_ROCSHMEM -- uninstrumented device code inside an ASAN build, which is the case the propagation exists to close. Measured on a standalone gfx950 ASAN build: librocshmem_device_gfx950.bc goes from 0 to 72 asan symbols, keeping target-features=+xnack. End to end, ./install.sh --ninja --rocshmem --address-sanitizer --amdgpu_targets gfx950 still reaches 4010/4010 with the instrumented bitcode: device.elf links at 0x74f (gfx950, xnack+, sramecc) and librccl.so is unchanged on the host side.

BITCODE_GPU_ARCHS is the list DeviceBitcode.cmake iterates, and it was the one of the three arch caches still left without FORCE. A directory configured without ASAN and then reconfigured with -DASAN=ON kept gfx1100 and gfx1201 -- the families the policy had just dropped as uninstrumentable -- and compiled them with -fsanitize=address on the bare _FULL_ARCH fallback. All three now move together in both directions. The policy moves to projects/rocshmem/cmake/AsanGpuTargets.cmake, included where it used to sit, following the cmake/rocm_local_targets.cmake precedent. Inline it could not be loaded under cmake -P, so the rocSHMEM copy had no coverage at all, which is how the earlier _always_on collision survived. The existing suite is now parameterised over both copies, 67 tests up from 55; deleting gfx1250-strict from the rocSHMEM copy turns two of them red. The rocshmem_ext options stamp moves beside the install it describes. ext/rocshmem is shared by the release and debug trees while the stamp was per-config, so with --no_clean this sequence linked an uninstrumented archive into an ASAN build: --rocshmem --address-sanitizer 1181 __asan syms in librocshmem.a --rocshmem --debug 0, debug overwrote the shared install --rocshmem --address-sanitizer 0, release stamp unchanged, step skipped With the stamp shared, the third step is 1181 again. It needs --no_clean to arm, since install.sh otherwise removes the build directory each run.

The bitcode compile passes -Xclang -disable-llvm-passes, so -fsanitize=address there only marked functions sanitize_address and linked the runtime in; no pass ever instrumented them. The __asan symbols counted before were the runtime's own definitions. Run asan in the opt step instead. Unix Makefiles, flipping ASAN in place: OFF: 0 __asan_report call sites ON: 127798, all 12 sources recompiled and opt re-run Also skip the rocSHMEM policy tests when projects/rocshmem is absent.

…get left

amd-argus

ARGUS

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

lfmeadow · 2026-09-30T18:18:28Z

@abouteiller gentle ping when you have a moment: this one is approved by alex-breslow-amd and amirsojoodi (rocSHMEM), CI is green, and there are no open threads. It needs one more RCCL-Reviewers approval to merge. Thanks!

abouteiller · 2026-10-02T15:26:27Z

rocshmem CI error is independent of this PR btw.

systems-assistant · 2026-10-05T04:21:03Z

TheRock Submodule Bump Activity

Newest first

2026-10-05 — Included in TheRock via ROCm/TheRock#8732.

…eline (#12394) JIRA ID : LCOMPILER-2794 Replaces #12313, which was closed by accident. Same changes, rebased onto current develop. Feature suffixes were being lost in the device linker pipeline. This caused features like `xnack+` to be lost when compiling for ASAN. ## What changed These changes keep both the bare processor name and the feature string. One site outside the device linker pipeline also dropped the feature string: the rocSHMEM build. This is also fixed. ## Tests New cases in the canonical driver suite at `tools/scripts/test_runner/tests/`. ## Notes for review One of the PRs replacing the closed #9763 and #12221. Nothing here depends on the others, though #12318 touches the same file and whichever lands second will need a rebase. ASAN device code needs both this and #12314: the latter decides *which* targets an ASAN build asks for, and this PR makes the request reach codegen. They can land in either order, but kernels are only instrumented once both are in. Verified with clean builds at `gfx950` and `gfx942:xnack+`, plus a 3-arch ASAN build, on ROCm clang 23 and clang 24. After the rebase, re-verified with clean `gfx950` ASAN builds, one with tests and one with `--rocshmem`: no `ignoring '-fsanitize=address'` warnings. --------- Co-authored-by: Cursor <cursoragent@cursor.com>

lfmeadow requested review from a team and a lite review from Copilot September 26, 2026 00:27

lfmeadow requested a review from a team as a code owner September 26, 2026 00:27

github-actions Bot added project: rccl project: rocshmem labels Sep 26, 2026

Copilot started reviewing on behalf of lfmeadow September 26, 2026 00:28

View session

This comment was marked as resolved.

Sign in to view

systems-assistant Bot added the organization: ROCm label Sep 26, 2026

This comment was marked as outdated.

Sign in to view

lfmeadow force-pushed the users/lmeadows/asan-target-policy branch from 7a4d29a to 00a1605 Compare September 26, 2026 18:06

lfmeadow mentioned this pull request Sep 26, 2026

fix(rccl): carry the full GPU target ID through the device-linker pipeline #12313

Closed

lfmeadow requested a lite review from Copilot September 26, 2026 20:09

Copilot started reviewing on behalf of lfmeadow September 26, 2026 20:10

View session

This comment was marked as resolved.

Sign in to view

lfmeadow requested a lite review from Copilot September 26, 2026 20:36

Copilot started reviewing on behalf of lfmeadow September 26, 2026 20:37

View session

This comment was marked as resolved.

Sign in to view

lfmeadow force-pushed the users/lmeadows/asan-target-policy branch from 680eb26 to 39202fe Compare September 26, 2026 20:44

This comment was marked as outdated.

Sign in to view

lfmeadow requested a lite review from Copilot September 27, 2026 04:04

Copilot started reviewing on behalf of lfmeadow September 27, 2026 04:05

View session

This comment was marked as resolved.

Sign in to view

lfmeadow requested a lite review from Copilot September 27, 2026 04:19

Copilot started reviewing on behalf of lfmeadow September 27, 2026 04:20

View session

lfmeadow mentioned this pull request Sep 28, 2026

fix(rccl): carry the full GPU target ID through the device-linker pipeline #12394

Merged

lfmeadow force-pushed the users/lmeadows/asan-target-policy branch from d4e51e0 to a6c5199 Compare September 28, 2026 22:03

amd-argus Bot reviewed Sep 28, 2026

View reviewed changes

alex-breslow-amd reviewed Sep 29, 2026

View reviewed changes

Comment thread projects/rccl/cmake/AsanGpuTargets.cmake Resolved

alex-breslow-amd reviewed Sep 29, 2026

View reviewed changes

Comment thread projects/rccl/cmake/ROCSHMEM.cmake Resolved

alex-breslow-amd approved these changes Sep 29, 2026

View reviewed changes

lfmeadow force-pushed the users/lmeadows/asan-target-policy branch from a6c5199 to cced372 Compare September 29, 2026 00:32

amd-argus Bot reviewed Sep 29, 2026

View reviewed changes

amirsojoodi added the ci:asan Opt-in to building ASAN label Sep 29, 2026

amirsojoodi approved these changes Sep 29, 2026

View reviewed changes

Comment thread projects/rocshmem/CMakeLists.txt Resolved

lfmeadow added 10 commits September 29, 2026 05:44

docs(rocshmem): describe the ASAN target policy in DEBUG.md

45c37bd

build(rccl): shorten the rocSHMEM options-stamp comment

3a16cf8

build(rocshmem): fail an ASAN configure that has no supported GPU tar…

6b04310

…get left

lfmeadow force-pushed the users/lmeadows/asan-target-policy branch from cced372 to 6b04310 Compare September 29, 2026 10:44

lfmeadow requested a review from abouteiller September 29, 2026 10:46

amd-argus Bot reviewed Sep 29, 2026

View reviewed changes

abouteiller approved these changes Oct 2, 2026

View reviewed changes

lfmeadow merged commit f9eba07 into develop Oct 2, 2026
138 of 141 checks passed

lfmeadow deleted the users/lmeadows/asan-target-policy branch October 2, 2026 17:42

Conversation

lfmeadow commented Sep 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

rocSHMEM

Tests

Scope: this does not by itself instrument device code

Notes for review

Uh oh!

therock-pr-bot Bot commented Sep 26, 2026

✅ All Policy Checks Passed

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

lfmeadow commented Sep 26, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

lfmeadow commented Sep 26, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as outdated.

Uh oh!

lfmeadow commented Sep 27, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

lfmeadow commented Sep 27, 2026

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alex-breslow-amd left a comment

Choose a reason for hiding this comment

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

amirsojoodi left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

lfmeadow commented Sep 30, 2026

Uh oh!

abouteiller commented Oct 2, 2026

Uh oh!

Uh oh!

systems-assistant Bot commented Oct 5, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lfmeadow commented Sep 26, 2026 •

edited

Loading

amirsojoodi left a comment •

edited

Loading