Skip to content

fix(MIOpen) : ASAN opt-out for hipconv and CK grouped-conv plugins - #15333

Open
nunnikri wants to merge 1 commit into
developfrom
users/nunnikri/disable-asan-hipconv-ckimpl
Open

nunnikri wants to merge 1 commit into
developfrom
users/nunnikri/disable-asan-hipconv-ckimpl

Conversation

@nunnikri

@nunnikri nunnikri commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

Motivation

Nightly ASAN builds have failed two nights running when ld.lld links libMIOpen.so, with relocation R_X86_64_PC32 out of range errors. libMIOpen.so statically links hipconv and MIOpen's own ai_heuristics.cpp directly, and ASAN's stack-use-after-return instrumentation on their dense, per-arch/per-kernel templated code has grown that link past the linker's +/-2GB PC-relative relocation limit. This is cumulative image bloat from ordinary kernel/feature growth (e.g. #13718, #14627), not a defect in any single PR, and will keep recurring as more kernels land.

ISSUE ID : ROCm/TheRock#9812

Technical Details

Adds MIOPEN_HIPCONV_SANITIZER and MIOPEN_CKIMPL_SANITIZER CACHE options, consumed in hipconv's and ck_impl's own CMakeLists.txt. When set to OFF, each strips -fsanitize=address from just that component:

  • hipconv: compile-option override only, on both the main hipconv target and each per-arch object-library target in _arch_targets (where the oversized template instantiations actually live). hipconv contributes object files/a static archive into libMIOpen.so's own link, not a link of its own, so there's no link step to override.
  • ck_impl: both compile- and link-option overrides, applied per MIOpenCKGroupedConv_<arch> plugin target — each is its own SHARED library with its own independent link step, mirroring the existing MIOPEN_DISABLE_DEVICE_UBSAN pattern already in this file. Unlike that UBSan precedent, this isn't -Xarch_device-scoped: the failures we're seeing are in host-side kernel-launcher code (under this build's unscoped, full-ASAN mode), so both host and device instrumentation are dropped here.

Neither option affects the rest of MIOpen, which keeps full ASAN/TSAN coverage. Both default to unset (no override, today's behavior unchanged) and are driven by matching preset knobs in ROCm/TheRock's CMakePresets.json (companion PR).

Mixing instrumented and uninstrumented code in one process is a standard, supported ASAN configuration — allocation/deallocation is intercepted process-wide regardless of which TU allocated or freed, and the sanitizer flag doesn't change a function's ABI. The tradeoff is narrower detection coverage: a memory bug originating purely inside hipconv/ck_impl's own code won't be caught at the point of fault anymore (though resulting heap corruption can still sometimes surface later via instrumented code touching the same memory).

Test Plan

MIOPEN_HIPCONV_SANITIZER / MIOPEN_CKIMPL_SANITIZER (set to OFF) strip
sanitizer instrumentation from these two targets only. Their dense
per-arch/per-Config template instantiation grows ASAN's
stack-use-after-return descriptors enough to push a R_X86_64_PC32
relocation past its +/-2GB range, failing libMIOpen.so's (hipconv) or
a CK plugin's own (ck_impl) link. The rest of MIOpen keeps full
ASAN/TSAN coverage.
@nunnikri
nunnikri requested a review from a team as a code owner November 7, 2026 08:46
@nunnikri
nunnikri requested a review from amd-alavin November 7, 2026 08:47
@therock-pr-bot

therock-pr-bot Bot commented Nov 7, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

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

🎉 All checks passed! This PR is ready for review.

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

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Nov 7, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant