Repository navigation
fix(hrr): keep graph landing buffers stable until teardown - #14156
Merged
Merged
Conversation
A graph memcpy node keeps the host pointer it was given, but host_landing_buffer resized a std::vector per recorded key, so a later event asking for a larger buffer for the same key reallocated it and left earlier nodes pointing at freed memory. A later graph launch then wrote into freed host memory. A grown buffer now gets a new block; the outgrown block is retired, not freed. Replay does not replay hipGraphDestroy/hipGraphExecDestroy (no-ops) and destroys its graphs at teardown, so blocks live as long as the context. Both blocks are resident, so a grown block is charged to the host budget whole. Tracks AIRUNTIME-3067.
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
andraghetti
approved these changes
Oct 26, 2026
andraghetti
left a comment
andraghetti
left a comment
Contributor
There was a problem hiding this comment.
MI350 gate on 21819a5: no new failures.
andraghetti
reviewed
Oct 26, 2026
andraghetti
reviewed
Oct 26, 2026
…IRUNTIME-3067) sz is already under the host budget cap and the zero-fill touches every page, so the nothrow check could not fire; plain make_unique matches develop's resize. The growth test keeps the one assertion that fails without the fix (a write through the pre-growth pointer), and the comments say only what the code does not.
andraghetti
reviewed
Oct 27, 2026
hip_playback.h only needed it for std::nothrow, removed with the nothrow branch. AIRUNTIME-3067
Contributor
TheRock Submodule Bump ActivityNewest first
|
Merged
1 task done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JIRA ID : AIRUNTIME-3067
Motivation
Replay gave each graph memcpy node a pointer into a
std::vector<uint8_t>keyed by the recorded host address. When a later event asked for a larger buffer for the same key,resizereallocated the vector. The earlier node still held the old pointer, so a later graph launch wrote into freed host memory.Technical Details
PlaybackContext::host_landing_buffer(projects/hrr/playback/hip_playback.h) now never moves or frees a block it has handed out. A larger request allocates a new block and retires the outgrown one instead of freeing it. Replay treatshipGraphDestroyandhipGraphExecDestroyas no-ops and destroys its graphs only at teardown, so blocks live as long as the context, which is as long as any graph can be launched. Both blocks are resident, so a grown block is charged to the host budget whole (it was growth-only before). A refused charge or failed allocation leaves the buffer as it was.Covers every caller: the from-symbol graph handlers and their exec forms all go through this function.
Issue Tracking
JIRA ID : AIRUNTIME-3067
Test Plan
Unit_HRR_HostBudget_LandingBufferPointerSurvivesGrowth(new, inhrr-handler-tests, AddressSanitizer): writes through a pointer taken before one growth of the same key.Unit_HRR_HostBudget_LandingBufferChargesWholeBlock(renamed from...ChargesGrowthOnly): updated for whole-block charging and for a refusal leaving the buffer unchanged.hip_playback.hthe new test aborts with an ASAN heap-use-after-free WRITE.Test Result
On gbt350 (private ROCr + capture CLR + HRR,
HIP_VISIBLE_DEVICES=4,5, tree922aa52e4e7c):hrr-unit-tests78 cases, 1487 assertions pass;hrr-handler-tests31 cases, 242 assertions pass; integration 147 pass, 1 known failure (Unit_HRR_SecureExecIgnoresCaptureOutput, AIRUNTIME-3074). Negative control (outgrown block freed instead of retired) fails the growth test with an ASAN heap-use-after-free.Submission Checklist