Skip to content

fix(rocr): compile the runtime with plain host ASan flags - #9511

Open
dileepr1 wants to merge 1 commit into
mainfrom
users/dravindr/fix-host-asan-rocr
Open

dileepr1 wants to merge 1 commit into
mainfrom
users/dravindr/fix-host-asan-rocr

Conversation

@dileepr1

@dileepr1 dileepr1 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

libamdhip64 itself stays link-only ASan until CLR passes a plain flag that ccache keeps. That change is ROCm/rocm-systems#14327. The local hipFree check passed only with both.

Test plan

  • HOST_ASAN build of ROCR compiles hsa-runtime with plain -fsanitize=address and the hsa_amd_memory_pool_allocate trampoline references __asan_init
  • A non-HOST_ASAN build does not compile ROCR with -fsanitize=address from this hook
  • With the companion CLR change, hipMalloc/hipFree on gfx942 (HSA_XNACK=1) no longer bad-frees

ccache drops -Xarch_host, so hsa-runtime never defines SANITIZER_AMDGPU and hipFree bad-frees the device pool allocation.
@therock-pr-bot

therock-pr-bot Bot commented Oct 27, 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?

@therock-pr-bot

Copy link
Copy Markdown

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

@shwetagkhatri shwetagkhatri left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM.
I have a follow up ROCr PR - ROCm/rocm-systems#14925 - after this one lands.

Approving this one - since TheRock HOST_ASAN build silently produces an un-instrumented ROCr and the breakage shows up much later as a bogus - attempting free on address which was not malloc()-ed. This PR restores the flag, which turns SANITIZER_AMDGPU on and activates the head-page swap that was supposed to be running. So this is a corrective fix for TheRock.

@ApurvMishra-amd

ApurvMishra-amd commented Nov 3, 2026 •

Copy link
Copy Markdown

Seems like it should be resolved as #9502 reported this exact ccache bug. It was closed by #9506, which moved the Docker image to ccache 4.12.1, and #9589, which repinned the CI images. Both merged on Oct 29, two days after this PR was opened, and the PR doesn't mention them. The branch is 72 commits behind main.
Please, try to repro and see this issue still exists using TheRock's main branch. If not resolved, go ahead and merge it, else it can be closed in favour of Shweta's rocm-systems#14925.

@dileepr1

dileepr1 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Seems like it should be resolved as #9502 reported this exact ccache bug. It was closed by #9506, which moved the Docker image to ccache 4.12.1, and #9589, which repinned the CI images. Both merged on Oct 29, two days after this PR was opened, and the PR doesn't mention them. The branch is 72 commits behind main. Please, try to repro and see this issue still exists using TheRock's main branch. If not resolved, go ahead and merge it, else it can be closed in favour of Shweta's rocm-systems#14925.

Thank you, looks like the above tickets should take care of the issue.
I'll confirm and close.

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

Labels

None yet

Projects

Status: TODO

Development

Successfully merging this pull request may close these issues.

3 participants