You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(hrr): strip build info from stored code objects, blank GPU identifiers - #13701
An HRR capture that leaves the machine carries the build tree and the GPU identity with it. Code objects keep the compiler build in .comment. With debug information, they also keep source file names and absolute build paths in .debug*. On one vLLM capture, ten of 33 code objects held absolute paths. The per-process manifest records the GPU's PCI address, UUID and LUID. Replay needs none of these. Capture now leaves them out.
Technical Details
New hip_capture_scrub.{h,cpp} in CLR's HRR capture layer. scrub_code_object() zeroes the contents of every .comment and .debug* section of an ELF64 image. For an uncompressed clang offload bundle, it does the same for each ELF entry.
Sections are zeroed in place, not removed. Sizes and offsets stay the same, so a bundle's entry table stays valid and the image loads as before.
Each zeroed section is also retyped SHT_NOBITS in its section header. Left as zeros, readelf, debuggers and profilers would parse them as malformed DWARF; as SHT_NOBITS they read as absent. Only sections without SHF_ALLOC are scrubbed, so the loader maps nothing from them and what runs is unchanged.
The parser reads fixed offsets with memcpy and checks every bound. A section name must end inside the string table. It does not include <elf.h>, which Windows lacks and whose typedefs collide with CLR's ELF headers.
writer::write_code_object() copies the image, scrubs the copy, then hashes and stores it. The archive stays content-addressed. Launches reference the hash this function returns, so they resolve as before.
Both fat-binary paths store a scrubbed copy: the __hipRegisterFatBinary shim and the static-init sweep in hip_capture_init. The application's own memory is never written.
Two known gaps. Compressed (CCOB) fat binaries are stored unchanged: recompressing them needs zstd, which CLR does not link. Linker inputs given to hipLinkAddData are stored as passed: they are SPIR-V or LLVM bitcode, not ELF, and keep their compiler identification and, with -g, their directories; only an IR rewriter could clean them.
What the GPU loads is not touched. A device assert() keeps its absolute source path in .rodata, and kernel names stay.
The manifest keeps the pci, uuid and luid keys and writes them as "". The unused hex helpers are removed. luid_device_node_mask stays.
hipChooseDevice and its R0000 variant record the caller's hipDeviceProp_t in events.bin. Capture now zeroes its uuid, luid, pciDomainID, pciBusID and pciDeviceID there, through a new blank_members field of the generator's Deref. CLR's hipChooseDevice compares only numeric limits, so replay picks the same device.
Kernel and symbol names are untouched, because replay resolves kernels by name. On gfx950, a -g build had no STT_FILE symbol. Its source name appeared only in .debug* sections.
inspect_archive.py verify (enable-recording skill) now reports, per process, hardware_ids that are not blank. It also reports debug_info_files: code objects or uncompressed fat binaries that still hold non-zero .comment or .debug* bytes; and path_files: those whose loaded, non-executable sections hold an absolute path string (/… or a drive letter, spaces and UTF-8 names included), such as an assert() source path. Compressed fat binaries are counted as unchecked_fat_binaries. __hipRegisterFatBinary is id 549 in archive format v8 and 550 in v9 and v10; for any other version, verify reports fat_binaries_scanned: false rather than scan, and tools/tests/test_skill_api_ids.py fails once HRR_VERSION moves past that table.
README, the enable-recording SKILL.md and PRIVACY.md describe what capture now drops and what it still keeps.
Issue Tracking
JIRA ID: AIRUNTIME-2881
Test Plan
Unit_HRR_CodeObjectScrub (new integration case). Its workload compiles a probe kernel with hiprtc and -g. It registers the kernel as a fat binary, then loads and launches it as a module. The driver replays the archive and checks the kernel's result. It then walks every stored code object and fat binary with its own ELF parser. Each copy must hold at least two .comment or .debug* sections, all zero. No stored file may contain clang version or the source file name.
Unit_HRR_MetadataManifest (changed). The pci, uuid and luid keys must be present and blank. The workload's hipChooseDevice record must hold the device name with UUID, LUID and PCI ids zero.
test_verify_reports_hardware_identifiers_and_build_info (new pytest). It builds synthetic ELF64 images and bundles, and checks the JSON and text output of verify. test_fat_binaries_are_found_by_the_id_of_the_archive_version covers v7 to v10 archives; test_a_section_name_ends_inside_its_string_table covers a name completed only by bytes past its table; test_an_absolute_path_in_loaded_data_is_reported covers a path in .rodata, and none counted from code or unloaded sections; test_paths_with_spaces_non_ascii_or_a_drive_letter_are_reported covers a space, a UTF-8 name, C:\ and D:/.
test_skill_api_ids.py (new, in hrr-generator-rules). It reads HRR_VERSION and HRR_API_HIPREGISTERFATBINARY from hrr_api_args.h and requires verify's table to map the one to the other.
Unit_HRR_Scrub_ZeroesAndRetypesBuildInfo, Unit_HRR_Scrub_SkipsUnterminatedName, Unit_HRR_Scrub_RefusesOutOfBoundsTables (new unit cases). hrr-unit-tests compiles the scrubber on its own and feeds it a valid ELF and bundle, a section name with no terminator inside the string table, a section, string table or section table that runs past the end or wraps, a bundle entry id length past the end, and every truncation of both. Built from a checkout of projects/hrr without CLR's src/hrr, the three cases are registered and skip, so the binary still matches unit.yaml.
Gate: HRR build with capture compiled into CLR, unit and integration ctest, then every Catch2 case one at a time, hidden cases included, on MI350X (gfx950). It ran on head da99aff028, which merges develop 5b4535b7ba.
Negative controls: for hipChooseDevice, the same tree with hip_capture_generated.cpp from b4d82a8022, rebuilt, running Unit_HRR_MetadataManifest. For the scrubber and verify, the earlier run below. On da99aff028: without the v10 entry, with the old path pattern, or with HRR_VERSION set to 11, the matching new test fails; built without CLR's src/hrr, 2aea59d041 leaves the three scrub cases MISSING in hrr_test_config.py check and da99aff lists them as skipped.
Baseline: develop at 5b4535b7ba, same runner and same scripts.
By hand: the scrub code on a real gfx950 -g code object and a gfx950+gfx942 bundle, under UBSan. Every truncation of both inputs was also fed in. It zeroed 10 sections per ELF and removed the compiler string and the source name. verify's path check on gfx950: a kernel with assert() compiled from an absolute path is flagged, the same kernel compiled from a relative path is not, and of the 4,270 files in the hipBLASLt and rocBLAS libraries of ROCm 7.0.2 one is flagged, for a real build path in its .rodata.
1 hang in Unit_HRR_PinnedHostFork, which passes run alone
pass
Unit Catch2 cases
78 of 78 pass
75 of 75 pass
Visible integration cases
126 pass, 4 skip
124 pass, 4 skip
Hidden integration cases
123 of 154 pass
122 of 153 pass
pytest, enable-recording skill (local)
137 pass, 28 skip
—
Apart from the new cases, every case has the same status as on develop. The 31 hidden cases that do not pass have the same status on develop. Unit_HRR_PinnedHostFork passed on 2aea59d and on develop, and none of the files it builds changed since. With capture's hipChooseDevice shim as on develop, Unit_HRR_MetadataManifest fails on the UUID and on pciBusID (6 == 0). With the path check disabled, or applied to every loaded section, test_an_absolute_path_in_loaded_data_is_reported fails.
Earlier run, on b90e2f0fa6: with every non-test file reverted to develop, CodeObjectScrub and MetadataManifest failed. The module copy kept 9 such sections and the fat binaries kept 88 across 9 ELFs, none of them zeroed, and the manifest carried a real PCI address, UUID and LUID. Against c1e2bffe3e's scrubber, Unit_HRR_Scrub_SkipsUnterminatedName fails; with the old fat binary id or the old name read in verify, the matching new pytest fails.
Not tested: the Windows build and capture path, which no runner here covers. Not tested either: a capture of a real vLLM or PyTorch workload. Compressed fat binaries and hipLinkAddData inputs keep their build information.
…ifiers
A capture meant to be shared carried the build tree and the machine with it:
code objects keep the compiler build in .comment and, when built with debug
information, source file names and absolute build paths in .debug*; the
manifest named the GPU by PCI address, UUID and LUID. Replay needs none of it.
Capture now zeroes the contents of every .comment and .debug* section before
it stores a code object, both the device images under code_objects/ and each
entry of an uncompressed fat binary under blobs/. Zeroing in place keeps sizes
and offsets, so a bundle's entry table stays valid and the image loads as
before; the stored bytes are hashed after scrubbing, so the archive stays
content-addressed. Compressed (CCOB) fat binaries are stored as they were. The
manifest keeps the pci, uuid and luid keys and writes them blank.
inspect_archive.py verify reports, per process, which hardware identifiers
are still set and how many code objects still hold build information, so the
person approving a transfer can see it.
Jira: AIRUNTIME-2881
…t binaries
inspect_archive.py verify skipped .comment and .debug* sections carrying
SHF_ALLOC. Capture leaves exactly those sections unchanged, so a build path in
one of them was reported as clean. The verifier now checks them whatever their
flags; the allocation guard stays in the capture-side scrubber.
blobs/ also holds host buffer contents, and a buffer's first bytes can look
like a fat binary: a 4-byte "CCOB" buffer counted as an unchecked compressed
fat binary, and a buffer holding an offload bundle counted as a code object.
The verifier now takes as fat binaries only the blobs that
__hipRegisterFatBinary records in events.bin name by hash.
Jira: AIRUNTIME-2881
…test's name read
Zeroed .comment and .debug* sections are now SHT_NOBITS, so DWARF readers
on a replay see no debug information instead of malformed zeros. They are
not SHF_ALLOC and keep their offset, so the loader and the image size are
unchanged.
The test's ELF walker reads a section name only inside the string table.
SKILL.md and README say what is removed: the debug information and the
compiler build; the symbol table is not touched.
The section-name search extends beyond the declared string table. In the truncated-table fixture from Unit_HRR_CodeObjectScrubWalker, the declared name is only .comm, but bytes outside the table complete .comment, so this verifier incorrectly reports build information. Validate the table's extent and require each name and its NUL terminator to lie inside it.
…archive version
The scrubber matched a section name that has no terminator inside the
string table, and the verifier read names past the table's end. Both now
skip such a name. hrr-unit-tests compiles the scrubber on its own and feeds
it malformed images.
verify looked for __hipRegisterFatBinary under id 550 only, which is v9's.
It now uses 549 for a v8 archive and reports fat binaries as not scanned
for any other version.
The reason will be displayed to describe this comment to others. Learn more.
Review — needs work
Full review of the whole change (develop…c1e2bffe). Implements AIRUNTIME-2881 as the description states it; ticket not retrievable from here.
Blocking
The description never says capture also rewrites sh_type to SHT_NOBITS — PR description (Technical Details); hip_capture_scrub.cpp:90, hip_capture_scrub.h, README.md:239-241
The description says sections are "zeroed in place, not removed" and the image "loads as before". The code also mutates the section header of every scrubbed section, so readelf, debuggers and profilers see NOBITS rather than zeros. The header comment, README and test (typed == 0) all cover it. Only the description omits it, and it is the part a reader would most want to weigh. Confidence 85 · mechanical · Fix: add the retype and its reason to Technical Details.
CI is not green and the rocprofiler-sdk failure is unattributed — Test rocprofiler-sdk (shard 1/1) (gfx94X-dcgpu), run 39203242231
I could not tie it to this change; the Windows sanity failure is in "Run setup test environment workflow" and therock-pr-bot timed out waiting on checks, both infra. A failing check still blocks until a rerun passes or it is shown to fail the same way on develop. Confidence 70 · architectural · Fix: rerun; if it persists, show it also fails on develop.
Non-blocking
README sentence cut off — projects/hrr/README.md:242
"Symbol names stay" ends mid-sentence, and the new manifest.json bullet follows straight after. SKILL.md has the full wording ("The symbol table is not touched, so kernel names stay"). Confidence 90 · mechanical · Fix: finish the sentence.
hipLinkAddData still stores its input unscrubbed — hip_capture.cpp:2329 (write_blob(data, size))
Linker inputs can be code objects or bitcode carrying the same compiler string and paths, and they land in blobs/ as-is. SKILL.md/README promise the removal only for code_objects/ and fat binaries, so the docs are accurate. But the PR names compressed fat binaries as its one known gap, and verify cannot see this one. Confidence 70 · architectural · Fix: scrub ELF inputs here too, or list it as a second known gap.
Decisions for the author
Zero and retype in place, rather than remove sections — tradeoff
In place keeps offsets, sizes and a bundle's entry table valid with no re-layout, at the cost of leaving section names and headers. Removal is cleaner but has to rewrite the ELF and the bundle table. The in-place choice looks deliberate and is well tested.
Compressed (CCOB) fat binaries stored unchanged — tradeoff
Avoids linking zstd into CLR, but a compressed bundle in blobs/ keeps everything this PR removes elsewhere. verify reports these as unchecked_fat_binaries, so the gap is visible.
Positive signals
The scrubber reads fixed offsets with memcpy, checks every bound, and copies before scrubbing, so the application's memory is never written.
The integration test parses ELF independently of capture's code, counts sections that must be non-empty, so it cannot pass on a no-op, and runs a negative control against develop.
verify separates "left" from "could not check" (unchecked_fat_binaries) instead of calling them clean.
Deployment notes
The code object and fat binary hashes change for existing workloads, since they now cover the scrubbed bytes. Archives from before this change will not dedupe against new ones.
The manifest keeps pci, uuid, luid as "". Nothing in the tree reads them, but external tooling that expects non-empty values will see blanks.
What this covered
All 12 changed files read completely, plus the callers of write_code_object/write_blob in hip_capture.cpp and hip_capture_writer.cpp. CI result names read from statusCheckRollup; two failure logs were not retrievable. Not run: the tests (no gfx950 runner here), so nothing is scored 100. Did not run: history and prior-changes passes. Single-context review, not a fan-out, so recall is lower than a split review. Not reconciled against any discussion.
Previously missed in review r5568199161, "Restrict section-name searches to the declared string table" (inspect_archive.py:204): fixed in b90e2f0fa6. verify now requires the string table to lie inside the image and each name to end inside the table, as capture does. test_a_section_name_ends_inside_its_string_table uses the truncated .comm table and fails with the old read.
… keeps
A device assert() keeps its source path in .rodata, which capture cannot
zero. verify now counts such code objects (path_files) and calls the
.comment/.debug* count debug_info_files. The docs and PRIVACY.md say what
stays: loaded strings, compressed fat binaries, hipLinkAddData bitcode,
and luid_device_node_mask.
The reason will be displayed to describe this comment to others. Learn more.
Re-checked at 336c4dd: the description now covers the NOBITS retype and lists hipLinkAddData as a known gap, the README sentence is finished, and the scrubber's bounds checks (string table, unterminated names, bundle id length) and the v8/v9 event-id handling in verify match their tests.
Nits:
projects/clr/hipamd/src/hrr/hip_capture_scrub.cpp:80 - the SHF_ALLOC and SHT_NOBITS skips have no unit case (make_elf always emits PROGBITS, flags 0); an allocated .comment that must stay untouched would pin the guard that protects loaded data.
projects/hrr/skills/enable-recording/scripts/inspect_archive.py:859 - prints blank when the manifest is missing or unreadable, since hardware_ids is [] then too; say unknown in that case.
…uild-info
Conflict in hip_capture_writer.cpp: write_code_object() now writes through
the code_objects/ directory descriptor (#13696) and still stores the
scrubbed copy of the image, whose hash names the file.
The test case set changes: the configs gain this branch's cases, which the
binaries register but the lists did not name: Unit_HRR_CodeObjectScrub,
Unit_HRR_CodeObjectScrubWalker and Unit_HRR_CodeObjectScrub_Direct in
integration.yaml, the three Unit_HRR_Scrub_* cases in unit.yaml.
…tter; keep scrub cases listed without CLR
inspect_archive.py had no __hipRegisterFatBinary id for format v10, so every
archive develop writes skipped the fat binary scan. Map v10 to 550, and fail
hrr-generator-rules (tools/tests/test_skill_api_ids.py) when HRR_VERSION in
hrr_api_args.h moves past the table.
The absolute path pattern now takes any byte from 0x20 up and a drive letter
prefix, so paths with spaces, UTF-8 names and C:\ paths are reported.
hrr_scrub_test.cc registers its three cases as skipped when the CLR sources
are not checked out, so the case list still matches tests/config/unit.yaml.
The case set is unchanged.
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
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.
danielbolsson
left a comment
Motivation
An HRR capture that leaves the machine carries the build tree and the GPU identity with it. Code objects keep the compiler build in
.comment. With debug information, they also keep source file names and absolute build paths in.debug*. On one vLLM capture, ten of 33 code objects held absolute paths. The per-process manifest records the GPU's PCI address, UUID and LUID. Replay needs none of these. Capture now leaves them out.Technical Details
hip_capture_scrub.{h,cpp}in CLR's HRR capture layer.scrub_code_object()zeroes the contents of every.commentand.debug*section of an ELF64 image. For an uncompressed clang offload bundle, it does the same for each ELF entry.SHT_NOBITSin its section header. Left as zeros,readelf, debuggers and profilers would parse them as malformed DWARF; asSHT_NOBITSthey read as absent. Only sections withoutSHF_ALLOCare scrubbed, so the loader maps nothing from them and what runs is unchanged.memcpyand checks every bound. A section name must end inside the string table. It does not include<elf.h>, which Windows lacks and whose typedefs collide with CLR's ELF headers.writer::write_code_object()copies the image, scrubs the copy, then hashes and stores it. The archive stays content-addressed. Launches reference the hash this function returns, so they resolve as before.__hipRegisterFatBinaryshim and the static-init sweep inhip_capture_init. The application's own memory is never written.CCOB) fat binaries are stored unchanged: recompressing them needs zstd, which CLR does not link. Linker inputs given tohipLinkAddDataare stored as passed: they are SPIR-V or LLVM bitcode, not ELF, and keep their compiler identification and, with-g, their directories; only an IR rewriter could clean them.assert()keeps its absolute source path in.rodata, and kernel names stay.pci,uuidandluidkeys and writes them as"". The unused hex helpers are removed.luid_device_node_maskstays.hipChooseDeviceand its R0000 variant record the caller'shipDeviceProp_tinevents.bin. Capture now zeroes itsuuid,luid,pciDomainID,pciBusIDandpciDeviceIDthere, through a newblank_membersfield of the generator'sDeref. CLR'shipChooseDevicecompares only numeric limits, so replay picks the same device.-gbuild had noSTT_FILEsymbol. Its source name appeared only in.debug*sections.inspect_archive.py verify(enable-recording skill) now reports, per process,hardware_idsthat are not blank. It also reportsdebug_info_files: code objects or uncompressed fat binaries that still hold non-zero.commentor.debug*bytes; andpath_files: those whose loaded, non-executable sections hold an absolute path string (/…or a drive letter, spaces and UTF-8 names included), such as anassert()source path. Compressed fat binaries are counted asunchecked_fat_binaries.__hipRegisterFatBinaryis id 549 in archive format v8 and 550 in v9 and v10; for any other version,verifyreportsfat_binaries_scanned: falserather than scan, andtools/tests/test_skill_api_ids.pyfails onceHRR_VERSIONmoves past that table.SKILL.mdandPRIVACY.mddescribe what capture now drops and what it still keeps.Issue Tracking
JIRA ID: AIRUNTIME-2881
Test Plan
Unit_HRR_CodeObjectScrub(new integration case). Its workload compiles a probe kernel with hiprtc and-g. It registers the kernel as a fat binary, then loads and launches it as a module. The driver replays the archive and checks the kernel's result. It then walks every stored code object and fat binary with its own ELF parser. Each copy must hold at least two.commentor.debug*sections, all zero. No stored file may containclang versionor the source file name.Unit_HRR_MetadataManifest(changed). Thepci,uuidandluidkeys must be present and blank. The workload'shipChooseDevicerecord must hold the device name with UUID, LUID and PCI ids zero.test_verify_reports_hardware_identifiers_and_build_info(new pytest). It builds synthetic ELF64 images and bundles, and checks the JSON and text output ofverify.test_fat_binaries_are_found_by_the_id_of_the_archive_versioncovers v7 to v10 archives;test_a_section_name_ends_inside_its_string_tablecovers a name completed only by bytes past its table;test_an_absolute_path_in_loaded_data_is_reportedcovers a path in.rodata, and none counted from code or unloaded sections;test_paths_with_spaces_non_ascii_or_a_drive_letter_are_reportedcovers a space, a UTF-8 name,C:\andD:/.test_skill_api_ids.py(new, inhrr-generator-rules). It readsHRR_VERSIONandHRR_API_HIPREGISTERFATBINARYfromhrr_api_args.hand requiresverify's table to map the one to the other.Unit_HRR_Scrub_ZeroesAndRetypesBuildInfo,Unit_HRR_Scrub_SkipsUnterminatedName,Unit_HRR_Scrub_RefusesOutOfBoundsTables(new unit cases).hrr-unit-testscompiles the scrubber on its own and feeds it a valid ELF and bundle, a section name with no terminator inside the string table, a section, string table or section table that runs past the end or wraps, a bundle entry id length past the end, and every truncation of both. Built from a checkout ofprojects/hrrwithout CLR'ssrc/hrr, the three cases are registered and skip, so the binary still matchesunit.yaml.hipChooseDevice, the same tree withhip_capture_generated.cppfrom b4d82a8022, rebuilt, runningUnit_HRR_MetadataManifest. For the scrubber andverify, the earlier run below. On da99aff028: without the v10 entry, with the old path pattern, or withHRR_VERSIONset to 11, the matching new test fails; built without CLR'ssrc/hrr, 2aea59d041 leaves the three scrub cases MISSING inhrr_test_config.py checkand da99aff lists them as skipped.-gcode object and a gfx950+gfx942 bundle, under UBSan. Every truncation of both inputs was also fed in. It zeroed 10 sections per ELF and removed the compiler string and the source name.verify's path check on gfx950: a kernel withassert()compiled from an absolute path is flagged, the same kernel compiled from a relative path is not, and of the 4,270 files in the hipBLASLt and rocBLAS libraries of ROCm 7.0.2 one is flagged, for a real build path in its.rodata.Test Result
Unit_HRR_PinnedHostFork, which passes run aloneApart from the new cases, every case has the same status as on develop. The 31 hidden cases that do not pass have the same status on develop.
Unit_HRR_PinnedHostForkpassed on 2aea59d and on develop, and none of the files it builds changed since. With capture'shipChooseDeviceshim as on develop,Unit_HRR_MetadataManifestfails on the UUID and onpciBusID(6 == 0). With the path check disabled, or applied to every loaded section,test_an_absolute_path_in_loaded_data_is_reportedfails.Earlier run, on b90e2f0fa6: with every non-test file reverted to develop,
CodeObjectScrubandMetadataManifestfailed. The module copy kept 9 such sections and the fat binaries kept 88 across 9 ELFs, none of them zeroed, and the manifest carried a real PCI address, UUID and LUID. Against c1e2bffe3e's scrubber,Unit_HRR_Scrub_SkipsUnterminatedNamefails; with the old fat binary id or the old name read inverify, the matching new pytest fails.Not tested: the Windows build and capture path, which no runner here covers. Not tested either: a capture of a real vLLM or PyTorch workload. Compressed fat binaries and
hipLinkAddDatainputs keep their build information.Submission Checklist