Skip to content

fix(rocprofiler-sdk): patch bundled ELFIO for hash table and note bounds - #13363

Draft
sstamenk wants to merge 1 commit into
developfrom
users/sstamenk/rocprofiler-sdk-elfio-bounds
Draft

sstamenk wants to merge 1 commit into
developfrom
users/sstamenk/rocprofiler-sdk-elfio-bounds

Conversation

@sstamenk

Copy link
Copy Markdown
Member

Motivation

rocprofiler-sdk bundles ELFIO through its external/elfio submodule and uses it in both standalone and TheRock builds (ROCPROFILER_BUILD_ELFIO defaults to ON). Upstream ELFIO has out-of-bounds reads and unbounded loops in symbol hash table lookup and note parsing. The fixes are in review upstream (serge1/ELFIO#174, serge1/ELFIO#173), but ELFIO has had no release since 3.12 (2023), so the SDK needs to carry them until they land.

Same fixes in CLR's ELFIO copy: #10982, #10979.

Technical Details

  • Move the external/elfio submodule from 575bfdb to upstream main (660e905), which is the base of both upstream PRs, so the patches are the upstream changes verbatim. The non-git fallback clone now checks out the same commit instead of Release_3.12, so git and non-git builds use the same ELFIO.
  • Add external/patches/elfio/0001-Validate-note-accessor-bounds.patch and 0002-Validate-symbol-hash-table-bounds.patch. Each names its upstream PR and should be removed once the submodule moves to a commit that includes it.
  • ELFIO is header-only. At configure time, copy its headers to the build tree, apply the patches there, and expose the result as elfio::elfio, replacing add_subdirectory(elfio), which only defined that header-only target. The submodule stays unmodified.
    • Patched headers are only rewritten when their content changes, so reconfiguring doesn't trigger rebuilds.
    • Configure reruns automatically when a patch or submodule header changes.
    • A patch that fails to apply stops configuration with the patch output.
  • Builds with ROCPROFILER_BUILD_ELFIO=OFF keep using the external ELFIO package unchanged.

JIRA ID

SWSPLAT-24296, SWSPLAT-24318

Test Plan

  • Configure and build rocprofiler-sdk with GCC 13 against an installed ROCm, and check that the ELFIO users (rocprofiler-sdk-common-library, rocprofiler-sdk-rocattach-shared-library) compile only against the patched headers.
  • Check that the patched headers match the upstream PR heads.
  • Run rocattach-symbol-lookup-test, whose fixtures cover both GNU (--hash-style=gnu) and SysV (--hash-style=sysv) hash tables.
  • Run rocprofv3 --kernel-trace on the transpose sample on gfx1100.
  • In a standalone project using the same CMake block, check that:
    • an unchanged reconfigure leaves the patched headers untouched;
    • editing a patch re-runs configure;
    • a patch that doesn't apply fails configure with a clear message.

Test Result

Note: on an older ROCm runtime (7.15), rocprofv3 aborts with heap corruption, with or without this change. The cause is the hsa_amd_signal_wait_any out-of-bounds write fixed in #8564, which the SDK's queue-interposition completion monitor hits whenever its wake signal (last slot) fires. That's unrelated to ELFIO; ROCPROFILER_QUEUE_INTERPOSITION=0 avoids it on such runtimes.

Submission Checklist

Prepared with AI assistance.

🤖 Generated with Claude Code

Carry the ELFIO symbol hash table and note accessor fixes from upstream
ELFIO PRs #174 and #173 until they land upstream.

- Move the external/elfio submodule from 575bfdb to upstream main
  (660e905), the base of both upstream PRs, so the patches are the
  upstream changes verbatim. The non-git fallback clone checks out the
  same commit instead of Release_3.12.
- Apply patches/elfio/*.patch at configure time to a copy of the
  header-only ELFIO in the build tree and expose it as elfio::elfio, so
  the submodule stays unmodified. Patched headers are only rewritten
  when their content changes, and configure reruns when a patch or
  submodule header changes.

Builds with ROCPROFILER_BUILD_ELFIO=OFF use the external ELFIO package
unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@therock-pr-bot

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?

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