[shwetagkhatri]

What this changes

hsa-runtime64 decides whether to compile its ASan-specific device-memory
handling by asking the compiler, not the build system:

#if defined(__clang__)
#if __has_feature(address_sanitizer)
#define SANITIZER_AMDGPU 1
#endif
#endif

SANITIZER_AMDGPU gates AllocateAsan on GPU pools, which is what drives
hsaKmtReplaceAsanHeaderPage. That call swaps the first page of a VRAM
allocation for ordinary host anonymous memory. If the instrumentation flag never
reaches the compiler, SANITIZER_AMDGPU is quietly never defined, the build stays
green, and the failure surfaces much later as a bogus free diagnostic that looks
like an application bug.

Two changes:

1. runtime/hsa-runtime/CMakeLists.txt — add -fsanitize=address and the
   matching link flags to the hsa-runtime64 target directly when ASan is
   requested, mirroring the block that already exists in
   libhsakmt/CMakeLists.txt. Today the thunk opts itself in and the runtime does
   not, so the two halves of one allocator can disagree.

2. runtime/hsa-runtime/core/inc/runtime.h — define HSA_ASAN_REQUESTED from
   the build when ASan was asked for, and #error if it disagrees with
   SANITIZER_AMDGPU.

Two details in the CMake half, both raised in review:

• The sanitizer runtime is only chosen when the caller has not chosen one. A
  shared library cannot carry the static ASan runtime, so a Clang BUILD_SHARED_LIBS
  build adds -shared-libsan — but only if CMAKE_CXX_FLAGS,
  CMAKE_SHARED_LINKER_FLAGS and
  CMAKE_EXE_LINKER_FLAGS contain no -shared-libsan/-static-libsan already.
  A build that picked its own runtime keeps it, rather than being forced into the
  "linked against incompatible ASan runtimes" pair.
• The static configuration exports the link requirement. LINK_FLAGS are not
  applied to a static archive — the step is an ar invocation, not a link — and are
  not exported, so the archive's objects were instrumented while consumers got
  nothing and would fail on unresolved __asan_*. -fsanitize=address is now
  carried on the INTERFACE link requirements of the consumer-facing hsa-runtime64
  target that wraps the archive, alongside the existing ones.

__has_feature remains the single source of truth for SANITIZER_AMDGPU. The new
macro only cross-checks it. Defining SANITIZER_AMDGPU from CMake directly would
trade a silent miss for a silent false positive — ROCr remapping head pages with no
ASan runtime present — which is strictly worse.

Why the check is at compile time

A check_cxx_source_compiles probe does not see this class of failure by default,
because try_compile does not inherit CMAKE_CXX_COMPILER_LAUNCHER. Measured on
CMake 3.29.6 with a logging script as the launcher: zero launcher invocations
during configure, one during the build — the probe reported success while the real
compile line was the thing at issue.

This is a default, not an absolute: setting
CMAKE_TRY_COMPILE_PLATFORM_VARIABLES to include CMAKE_CXX_COMPILER_LAUNCHER
before the probe does propagate it, and the behaviour is CMake-version sensitive. A
#error on the real compile line avoids having to get that opt-in right.

Relationship to #14327

#14327 (open, by @dileepr1, who also authored ROCm/TheRock#9511) solves the same
problem class for projects/clr by a different mechanism: a post-build
check_asan_instrumentation.cmake that runs nm -D on the linked library and fails
if __asan_init is absent.

The two are worth comparing deliberately rather than landing side by side by
accident:

	#14327 (clr)	this PR (rocr)
When	post-build	compile time
Scope	the whole shipped .so	per TU
Misses	a library where at least one TU was instrumented and the rest were not	TUs that do not include runtime.h
Cost	one nm per link	none

Neither dominates the other — they fail in different places. nm -D sees every TU but
only asks whether any of them was instrumented (__asan_init is emitted as soon as
one object is), and it returns early unless ASan is enabled and
BUILD_SHARED_LIBS AND UNIX. A #error
asks per TU, but only in TUs that include the header. Reviewers should say whether
rocm-systems wants one mechanism
across both components — if so, adopting #14327's post-build check for
hsa-runtime64 too is the more consistent outcome, and the CMake half of this PR
(the missing ASan flags) still stands on its own.

#14327 names the same root cause as ROCm/TheRock#9511 — "ccache drops
-Xarch_host -fsanitize=address unless the compile also passes -arch, so clang
never saw the sanitizer flag" — and says explicitly that the hipFree bad-free
"also needs the ROCr runtime compiled with that same plain flag, so
SANITIZER_AMDGPU is defined". #14327 does that for projects/clr; this PR does it
for projects/rocr-runtime, which is the component #14327 points at.

Relationship to ROCm/TheRock#9511

TheRock PR 9511 adds a pre_hook_ROCR-Runtime.cmake that re-adds plain host ASan
flags to this subproject, because ccache reportedly drops -Xarch_* when the
compile has no -arch.

This PR is independent of 9511 and can merge before it, after it, or without it.
Different repositories, no shared files, no textual conflict.

9511 state	Result with this PR
Not merged	ROCr adds the flag itself; __has_feature is true; check passes. Fixed from the ROCr side.
Merged	Hook and ROCr both add -fsanitize=address. Measured idempotent — doubled flag compiles and links, exit 0.
Merged later	No change — already consistent.

9511 fixes the flag at the TheRock layer; this fixes the inference at the ROCr layer,
and additionally covers ROCr builds that never go through TheRock — distro packaging,
Spack, rocm-systems CI, or a plain cmake invocation.

Testing

All figures in this section were taken on this branch's single commit, on one Linux host with
AMD Clang 18 and CMake 3.29.6. They are configure-time and compile-time observations,
not a device run.

-DADDRESS_SANITIZER=ON:

• Configure succeeds; hsa-runtime64: AddressSanitizer (ASAN) enabled emitted.
• -fsanitize=address, -fno-omit-frame-pointer and HSA_ASAN_REQUESTED=1 all
  present on the generated compile line; -fsanitize=address and -shared-libsan
  on the link line, once each.
• core/runtime/runtime.cpp compiles clean (pre-existing warnings only).
• Taking that exact compile command and removing only -fsanitize=address — what a
  launcher would leave behind — fails with the new #error, exit 1.

Sanitizer-runtime and static-interface behaviour, measured across three configures:

Configure	Measured
shared + ASan, caller silent	-shared-libsan on the link line, once
shared + ASan + CMAKE_CXX_FLAGS=-static-libsan	no -shared-libsan added; caller's choice kept
BUILD_SHARED_LIBS=OFF + ASan	exported hsa-runtime64Targets.cmake carries -fsanitize=address (was absent)

Default build, no ASan requested: no status message, no -fsanitize=address, no
HSA_ASAN_REQUESTED; same TU builds with zero errors. Unaffected.

Preprocessor logic verified across all four states: no-ASan/no-define compiles,
requested+instrumented compiles, requested+stripped fails, instrumented-without-define
compiles (no false alarm).

Coverage of the check

The check fires only in translation units that include core/inc/runtime.h, which is
a subset of this target's sources — so it is not a whole-target guarantee. Nor is
#14327's post-build nm -D check, which passes as soon as a single object in the
library was instrumented. What this check does guarantee is the part that
matters: both TUs in this target that consume SANITIZER_AMDGPU —
core/runtime/runtime.cpp and core/runtime/hsa_ext_amd.cpp — include that header,
so every site whose behaviour depends on the macro is covered.

Not covered / known limits

• Not a full build. The device-code targets on the development host fail for an
  unrelated, pre-existing toolchain reason — the .hsaco steps pick up a system
  clang that rejects the gfx950 and gfx1250 target IDs and cannot find the ROCm
  device library. Those are not host C++ compiles and this change does not touch
  them. The host TU was built directly instead. CI exercises the full path.
• No GPU run. No hipMalloc/hipFree ASan scenario was executed on a device.
• ccache not reproduced. No ccache on the development host; the launcher
  behaviour was demonstrated with a stand-in script, which shows the try_compile
  blindness but not ccache's specific -Xarch_* handling.
• GCC: this patch changes behaviour. Measured against the base commit with
  g++ -DADDRESS_SANITIZER=ON, hsa-runtime64 previously received
  -fsanitize=address on neither its compile nor its link line; with this patch it
  receives both, matching libhsakmt. What is pre-existing is only that
  SANITIZER_AMDGPU is __clang__-gated, so ROCr's ASan-specific handling still
  does not activate under GCC. The consistency check is therefore withheld under GCC
  — it could only ever fire — and CMake emits a warning instead. Reviewers may
  reasonably prefer that GCC get no flags at all; say so if you do.
• Does not cover flags passed by hand. A developer passing
  -DCMAKE_CXX_FLAGS=-fsanitize=address through a launcher that eats it still gets
  no warning, because nothing told the build ASan was requested. The check covers
  builds requesting ASan via ADDRESS_SANITIZER or THEROCK_SANITIZER.
• No unit test, and the policy bot warns about it. What this change adds is a
  build-system assertion: its entire behaviour is whether a configure emits certain
  flags and whether a compile fails when they are missing. There is no runtime entry
  point for a test_*.cpp to call. The equivalent verification is the configure and
  compile matrix in Testing above. Happy to add one if a reviewer sees a shape for it.
• The predicate deliberately mirrors libhsakmt's rather than the UBSAN block's,
  which also consults environment variables. Worth a reviewer opinion on whether the
  two should converge.

Provenance of the allocator description

The reason the head-page swap matters at all is that, when the application is
instrumented, ASan's interceptor serves hsa_amd_memory_pool_allocate from its own
4096-aligned device allocator and places the 16-byte AsanChunk bookkeeping header
in the last 16 bytes of that first page, written by the host CPU. If that page is
still device memory, free re-reads the header, does not find CHUNK_ALLOCATED, and
reports attempting free on address which was not malloc()-ed.

That description was read from a shallow checkout of an AMD llvm-project fork
at commit 7c0e3b4170de, dated 2026-03-30, not from upstream LLVM at any released
tag. The constants (kChunkHeaderSize == 16, CHUNK_ALLOCATED = 2,
kPageSize_ = 4096) hold in that tree. Anyone relying on this reasoning should
re-confirm against the compiler-rt actually shipping with their ROCm build.

🤖 Generated with Claude Code

[therock-pr-bot]

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	⚠️ Warning	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*). Current: code file(s) changed: projects/rocr-runtime/runtime/hsa-runtime/core/inc/runtime.h; no test file found
🚫 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?

[therock-pr-bot]

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

• ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

[Copilot]

🟡 Changes recommended

The ASan build changes omit required sanitizer linkage for static-library consumers and force a shared ASan runtime that conflicts with applications using Clang’s default static runtime.

2 open issues

• Avoid forcing shared ASan without matching consumer linkage · New
• Propagate ASan link requirements for static builds · New

What changed in this PR

This PR adds explicit AddressSanitizer instrumentation to hsa-runtime64 and a compile-time check to catch requested instrumentation being lost before compilation.

Changes:

• Add ASan compile and link flags when requested through ADDRESS_SANITIZER or THEROCK_SANITIZER.
• Define HSA_ASAN_REQUESTED for Clang builds and reject compilation when ASan-specific memory handling is inactive.
• Warn when ASan is requested with a non-Clang compiler.

File	Description
projects/​rocr-runtime/​runtime/​hsa-runtime/​core/​inc/​runtime.h	Check requested ASan instrumentation against compiler-derived support.
projects/​rocr-runtime/​runtime/​hsa-runtime/​CMakeLists.txt	Add sanitizer flags, the consistency-check definition, and a compiler warning.

_{Review effort: Balanced}

[ApurvMishra-amd]

Seems like libhsakmt does the same, so it should be okay.

[ApurvMishra-amd]

This seems like a valid concern, @shwetagkhatri. Look at my comment below for target_link_options.

[ApurvMishra-amd]

To address the issue pointed out by Copilot, we can add here:

  if ( HSA_ASAN_REQUESTED )
    target_link_options ( ${CORE_RUNTIME_NAME} INTERFACE -fsanitize=address )
  endif()

[ApurvMishra-amd]

Look at the comments, it looks good to me except for the second comment from Copilot.

[cfreeamd]

🤖 AI-assisted pre-review (for @cfreeamd) — CMakeLists.txt and runtime.h changes look structurally sound; the runtime.h consistency check is a good paranoid-check pattern. Two issues with the CMake link-flag logic flagged inline. [Required] PR title should follow type(component): format — suggest: build(rocr): add AddressSanitizer flags and instrumentation consistency check

[cfreeamd]

🤖 AI-assisted pre-review (for @cfreeamd)

[Important] Linking ROCr's shared library with -shared-libsan bakes the shared ASan runtime into libhsa-runtime64.so. Any application using Clang's default static ASan runtime (-fsanitize=address without -shared-libsan) will abort on startup with "Your application is linked against incompatible ASan runtimes". For example, rocrtst built with CMAKE_CXX_FLAGS=-fsanitize=address uses static ASan by default and will fail.

The more composable approach for a shared library: omit the runtime-variant flag (-shared-libsan/-static-libsan) from the library's own link step and let consumers control their runtime selection. If you need to enforce shared-runtime linkage, propagate it via target_link_options(... INTERFACE -shared-libsan) and document the requirement explicitly.

[cfreeamd]

🤖 AI-assisted pre-review (for @cfreeamd)

[Important] LINK_FLAGS has no effect on static archives — it is silently ignored when BUILD_SHARED_LIBS=OFF. Consumers linking the static ROCr library will get instrumented object code but no ASan link requirement, resulting in unresolved __asan_* symbols at link time unless they independently add -fsanitize=address.

Fix: in the BUILD_SHARED_LIBS=OFF + HSA_ASAN_REQUESTED branch, add:

target_link_options(${CORE_RUNTIME_TARGET} INTERFACE -fsanitize=address)

(CMake 3.13+ required — already asserted near line 251. Clang-only guard applies here too.)