fix(hrr): make decode-and-triage produce a correct finding on real archives - #9758
Open
andraghetti wants to merge 5 commits into
Open
fix(hrr): make decode-and-triage produce a correct finding on real archives#9758andraghetti wants to merge 5 commits into
andraghetti wants to merge 5 commits into
Conversation
Four defects in the decode-and-triage skill, each reproduced against replay output recorded on a gfx950 host, and each covered by a test built from that output rather than from a hand-written sample. A GPU memory fault was reported as a plain API error. Surfacing a fault through --sync-after-event makes hrr-playback print "Fatal: GPU error after ..." for the same fault the runtime reports, and the abort branch was tested before the memory-fault branch, so a read-only page fault classified as replay_fatal_api. That is the mode used to localize a fault, so the flagship fault class was the one being lost. Memory faults are now classified first. A clean replay was attributed a kernel. The archive lists the kernels it ran, a GEMM was matched out of that listing, and the finding printed it next to a PASS as though it were a culprit. Observed on the replay of a real vLLM inference capture that passed every device-to-host check. A clean replay now implicates nothing. The opposite case was silent: a memory fault can tear the process down before the failing dispatch is attributed, leaving a log with no kernel at all. The --info kernel table is now parsed, and when the archive holds exactly one kernel the finding names it and records that the name was inferred. More than one stays unknown. Names truncated by the table's column width are ignored, since a cut-off symbol cannot be looked up; in the vLLM capture every name in the table is truncated. Device selection picked the busiest card. The awk matched "Used Memory" and "Total Memory" as separate patterns although the used line reads "VRAM Total Used Memory" and so matches both. On two devices reporting the same total, where the first is nearly full, it selected the first and reported its whole VRAM as free. On a shared host that sends the replay onto somebody else's device. Totals are now collected per device and subtracted once. Also mask the replay with HIP_VISIBLE_DEVICES. hrr-playback is a HIP program, and setting ROCR_VISIBLE_DEVICES re-indexes devices underneath a HIP mask, which can place the replay on a device other than the one picked. The Windows and container paths already used the HIP mask, so this makes the three agree. Smaller parsing gaps closed at the same time: the Complete: matcher accepted only uppercase while --info prints "yes (clean shutdown)", so a clean archive never recorded its state; the labelled Events:, Kernels: and Recovered: lines were unparsed, so an --info run carried no totals; and a version mismatch recorded a note but not the two versions it found. JIRA ID: AIRUNTIME-2597
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes multiple correctness issues in the decode-and-triage skill’s replay analysis and replay execution wrapper so that reported fault classes, kernel attribution, archive metadata, and GPU selection reflect what the run actually supports.
Changes:
- Fix fault classification priority so GPU memory faults aren’t masked by the generic “Fatal … aborting” line.
- Rework kernel attribution so clean replays report no kernel, and single-kernel archives can infer attribution only when unambiguous (ignoring truncated
--infonames). - Fix GPU picking and native replay masking: correct
rocm-smiparsing for free VRAM and switch native replay masking toHIP_VISIBLE_DEVICES.
Reviewed changes
Copilot reviewed 8 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/SKILL.md | Documents kernel attribution rules (no kernel on PASS; inferred-only when unambiguous). |
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/scripts/triage_archive.sh | Fixes pick_gpu VRAM parsing and masks native replay with HIP_VISIBLE_DEVICES. |
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/scripts/test_triage_archive.py | Adds unit tests for GPU selection parsing and masking behavior (no GPU required). |
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/scripts/test_analyze_replay_finding.py | Extends tests for fault classification precedence and end-to-end recorded fixtures. |
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/scripts/analyze_replay_finding.py | Implements classification fix, parses --info totals and kernel table, records version fields, and finalizes attribution once all inputs are parsed. |
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/scripts/fixtures/version_mismatch.log | Adds recorded fixture for version-mismatch parsing. |
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/scripts/fixtures/rocm_smi_vram.txt | Adds recorded rocm-smi output fixture for GPU-selection tests. |
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/scripts/fixtures/replay_pass.log | Adds recorded passing replay fixture. |
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/scripts/fixtures/replay_memory_fault.log | Adds recorded memory-fault replay fixture. |
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/scripts/fixtures/info_pass.txt | Adds recorded --info fixture for clean archive totals and truncated kernel name handling. |
| projects/clr/hipamd/src/hrr/skills/decode-and-triage/scripts/fixtures/info_crash.txt | Adds recorded --info fixture for single-kernel crash archive attribution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Running the skill end to end on a gfx950 host showed native replay was impossible even with hrr-playback present and supplied through HRR_PLAYBACK: triage_archive.sh reported "ensure_playback.sh --build failed" and stopped. export_playback_env ended with `[[ -n "$rocr_lib" ]] && export ROCR_LIB=...`. Under `set -e` that trailing conditional is the function's exit status, so whenever the ROCR lib dir does not resolve, which is any host that is not a CLR source tree, the function returned 1 and took the script down with it before it printed the playback path. The binary was found and then discarded. Make the export a plain if, and cover it with a test that runs the script outside any CLR tree with HRR_PLAYBACK pointing at a stub. The test needs bash 4.4 or newer, since expanding an empty array under `set -u` is an error on the bash 3.2 that macOS still ships, and these scripts target Linux. JIRA ID: AIRUNTIME-2597
Running the skill against a faulting archive on a gfx950 host showed it produced no finding at all in the one case it exists for. run_native_replay re-enabled `set -e` inside itself before `return "$rc"`, cancelling the `set +e` the caller wraps it in precisely so a failing replay is survivable. Any non-zero replay exit therefore killed the script before the analyzer ran, so a replay that faulted left a log and nothing else. The inner `set -e` is dropped; the caller restores it as it always intended to. Replay now runs with --sync-after-launch by default, with --no-sync to opt out. Default playback serializes the GPU once at the end, so a fault is reported but never attributed and the finding carries no failing event and no kernel. On the faulting archive the synchronized run additionally makes the runtime print the dispatch packet and name the kernel outright. Measured on a real vLLM inference capture of 13233 kernels, the option costs 41.7 s against 39.6 s, so the attribution is close to free. Three parser gaps found in review against the HRR runbooks: - The Memory Fault Error bracket varies by ROCm build. Requiring the `host:` field dropped the kernel name on every build that starts the bracket at `GPU index:`, which is the one field the report exists to give. The regex now anchors on the two fields actually consumed. Both forms occur on our own hardware, so neither can be assumed. - The `--info` kernel rows tolerate a missing id column, and the kernel total is recovered from the API call-count block when no `Kernels:` line is printed. Without the latter the total stays unknown and the single-kernel inference can never fire on a crash-resilience archive. - Per-event progress lines are parsed, so the last launch to start before a fault stands in when the runtime's fault line named no kernel, recorded in the finding as such. A fault on a <<<>>>-launched ATen kernel keeps its class rather than becoming a recording artefact. Capture records the pointers these kernels pass inside by-value structs and replay translates them, so such a fault can be genuine and reclassifying it would suppress a real finding. The translation is a value-based heuristic and archives predating it carry no recorded offsets, so the finding carries a note and the skill asks for the original failure signature instead of resolving the ambiguity either way. JIRA ID: AIRUNTIME-2597
Recorded a PyTorch capture whose embedding lookup indexes past its table, and
replayed it on a gfx950 host. The replay reproduced the fault and the runtime
named the culprit outright:
Callback: Queue ... aborting with error : HSA_STATUS_ERROR_EXCEPTION: An
HSAIL operation resulted in a hardware exception. code: 0x1016
[host: ..., GPU index: 0, kernel: void at::native::(anonymous
namespace)::indexSelectSmallIndex<...>]
The finding called it a hang with an unknown kernel. Two reasons, both fixed.
That bracket carries a kernel but no faulting address, so the memory-fault
regex could not see it and the one name the runtime handed us was dropped. It
is now read from the abort line when nothing else named a kernel.
`hang` was reachable from any HSA_STATUS_ERROR_{MEMORY_FAULT,ABORTED,EXCEPTION}.
None of those is a hang: MEMORY_FAULT says it is a fault, and EXCEPTION is a
hardware exception, which an out-of-bounds access raises. MEMORY_FAULT now
classifies as illegal_memory_access and the other two as replay_aborted, the
outcome follows the same split instead of staying UNKNOWN next to a decided
fault class, and the raw HSA status is recorded in the notes. A genuine hang
shows up as no progress against the clock, which needs a replay timeout the
skill does not have; reporting one from an HSA status alone was sending the
reader after stalled work that never existed.
The kernel family now recognises the demangled `at::native::` form as well as
the mangled `_ZN` one, since the memory-fault line prints one and the queue
abort line the other. That also lets the ATen caveat fire, which this capture
is the first real case of.
The replay log is checked in as a fixture, unedited.
JIRA ID: AIRUNTIME-2597
Four defects found by exercising the paths the earlier commits did not: a hang, metadata-only triage, and a packaged playback layout. A hung replay never returned, so the analyzer never ran and the archive yielded no finding whatsoever, which is the same silence as a crash used to produce. The replay is now bounded, default 1800s, `HRR_REPLAY_TIMEOUT=0` disables. The bound goes around the playback process rather than around any container that may wrap it: a timeout on the container leaves the replay running inside it. When the bound fires the runner records it in the log, the analyzer classifies `hang` from that marker, and the outcome is `HANG`. Verified with a 45s bound against a hanging archive: a finding is produced where none was before. Metadata-only triage silently discarded every piece of archive evidence. `--no-replay` never resolved a playback binary, so `hrr-playback --info` was never run and events, kernel totals, kernel names and the completeness flag were all absent from the finding. That is the one mode meant to work without a GPU. It now locates a binary without ever building one and, when there is none, degrades exactly as before. That fix alone was not enough, which exposed the third defect: the library path only handled a playback inside a CLR build tree. A packaged playback ships as bin/ and lib/ siblings, and its own lib dir was never added, so the binary loaded the system libamdhip64 and failed on the symbols it was built against. This took out the metadata-only path and would take out replay on any installed playback. `last_progress_kernel` was assigned unconditionally after its match loop, so the `--info` pass, which has no progress lines, erased the kernel the replay log had already recorded. The default pipeline always parses both, so the field was always empty in practice. Two corrections to what the earlier commits claimed. The per-event lines the finding falls back on come from `--verbose`, not from the sync flags: `hrr_playback.cpp` gates them on `ctx.verbose` alone, so the note now says so. And the GPU is no longer picked in metadata-only mode, which announced a device it never used. JIRA ID: AIRUNTIME-2597
andraghetti
changed the title
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.
Summary
The
decode-and-triageskill was run end to end against real archives on a gfx950 host, and reviewed against the HRR runbooks. It did not survive either exercise: on a crashing archive it produced no finding at all, and on the archives where it did produce one, the verdict was often wrong.This is a skill-only change. No capture or playback source is touched.
What was wrong
hrr-playbacksuppliedexport_playback_envended in a trailing&&, which underset -ebecame the function's exit status and killed the script before it printed the pathrun_native_replayre-armedset -einside itself, cancelling theset +ethe caller wraps it in, so any non-zero replay exit ended the script before the analyzer ran--no-replaynever resolved a playback binary, so--infonever ranbin/andlib/siblingshangwas reachable from anyHSA_STATUS_ERROR_*, and the abort line carries a kernel but no faulting address, so the memory-fault regex could not see itreplay_fatal_api--infokernel table was not parsed, so a single-kernel archive named nothingRE_MEM_FAULT_ERRrequired ahost:field that not every build emitsUsed MemoryandTotal Memoryas separate patterns, but the used line readsVRAM Total Used Memoryand matches bothROCR_VISIBLE_DEVICES, which re-indexes underneath a HIP mask; the Windows and container paths already usedHIP_VISIBLE_DEVICESComplete:,Events:,Kernels:and both format versions unreportedLast progress kernelalways empty--infopass erased what the replay log had recordedTwo of these deserve a note.
Nothing beats a wrong verdict for cost. Defects 6 and 8 are the expensive ones: one reports a hang that never happened and drops the kernel the runtime had just named, the other prints a GEMM next to a
PASS, where it reads as a culprit. Both send a kernel developer after a defect the run never observed.A hang is now bounded, not detected. The replay is stopped after
HRR_REPLAY_TIMEOUTseconds, default 1800, and the runner records that in the log so the finding sayshangrather than nothing. The bound goes around the playback process, not around a container that may wrap it: a timeout on the container leaves the replay running inside it.What changed in behaviour
--sync-after-launchby default;--no-syncopts out. Measured cost on a real vLLM capture of 13233 kernels: 41.7 s against 39.6 s.--infocolumn width are never used.HANG.HSA_STATUS_ERROR_MEMORY_FAULTclassifies asillegal_memory_access, the other HSA statuses asreplay_aborted, and the raw status goes in the notes.<<<>>>-launched ATen kernel keeps its class and gains a caveat. Capture records the pointers these kernels pass inside by-value structs and replay translates them, so such a fault can be genuine and reclassifying it would suppress a real finding; but the translation is a heuristic and older archives carry no offsets, so the skill asks for the original failure signature instead of resolving the ambiguity.Testing
Fixtures are unedited tool output recorded on a gfx950 host, so a change in what
hrr-playbackprints fails a test rather than silently degrading a finding. Device selection, playback discovery and the library layout are covered without a GPU by lifting the shell functions out of the script and running them against recorded input.Unit suite: 24 tests before, 42 after. One test in
test_check_replay_compat.pyfails identically on unmodifieddevelopon both Linux and macOS and is untouched here. One test skips below bash 4.4.End to end with
triage_archive.shon a gfx950 host:PASS/replay_pass, no kernel implicated,Complete: yes, 185469 eventsMAF/illegal_memory_accesswith fault address, kernelcrash_ooband the dispatch grid. Previously: no finding at allHSA_STATUS_ERROR_EXCEPTIONon the same kernel.ABORT/replay_aborted, fullindexSelectSmallIndexsignature, ATen caveat. Previously:UNKNOWN/hang, kernel unknownHANG/hangwith a finding. Previously: ran until killed, no findingversion_mismatchwith archive version 3 and reader version 4, no kernel inventedcrash_oob,Complete: no. Previously: every fieldn/aKnown gaps
pid-*directory, andload_capture_metadatareads only<archive>/manifest.json, so an archive root loses the preflight and the capture fields.triage_archive.ps1gains no changes here beyond already using the HIP mask.Issue Tracking
JIRA ID: AIRUNTIME-2597
Submission Checklist