Skip to content

fix(rocroller): release acquired COMGR executable data - #14698

Merged
rkayaith merged 1 commit into
developfrom
users/rkayaith/ROCM-33540-comgr-executable-release
Nov 4, 2026
Merged

rkayaith merged 1 commit into
developfrom
users/rkayaith/ROCM-33540-comgr-executable-release

Conversation

@rkayaith

@rkayaith rkayaith commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

JIRA ID : ROCM-33540

Motivation

rocRoller's in-process assembler acquires the linked executable from the code-object manager (COMGR) but never releases that reference. Each assembly leaks executable data, causing host AddressSanitizer runs to fail at exit (ROCM-33540).

Found while expanding ASAN testing in #14240.

Technical Details

Release the reference acquired by amd_comgr_action_data_get_data after copying the executable bytes. Destroying the dataset releases its own reference; it does not release this separately acquired reference.

Test Plan

With a host AddressSanitizer (HOST_ASAN) TheRock build, run the existing CPU-only test without leak suppressions:

env -u LD_LIBRARY_PATH -u LD_PRELOAD -u ROCROLLER_ASSEMBLER -u ROCROLLER_SAVE_ASSEMBLY \
  ASAN_OPTIONS=quarantine_size_mb=600:detect_leaks=1 LSAN_OPTIONS= \
  HIP_VISIBLE_DEVICES=-1 ROCR_VISIBLE_DEVICES= HSA_XNACK=0 \
  rocroller-tests --gtest_filter=AssemblerTest.Basic

Test Result

Leaked bytes went from 4,204 to 0.

The mci/rocm-libraries/code-analysis(rocroller) failure is pre-existing: the job fails before analysis at docker build -f null, the same way it does on develop and other PRs.

ASAN error output (before fix)
=================================================================
==1667238==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1240 byte(s) in 1 object(s) allocated from:
    #0 0x70e0e936daed in operator new(unsigned long, std::nothrow_t const&) /__w/rocm-libraries/rocm-libraries/TheRock/compiler/amd-llvm/compiler-rt/lib/asan/asan_new_delete.cpp:114:3
    #1 0x70e0cb7f51a5 in amd_comgr_create_data (/home/rkayaith/repos/iree-workspace/workspaces/hipblaslt-hipsparselt-host-asan/tmp/comgr-existing-test/current-pin-dist/bin/../lib/libamd_comgr.so.3+0x5f51a5) (BuildId: 14c1bc839727066f9233f0481c919d82cabe711d)
    #2 0x7ffe694a9f17  (<unknown module>)

Indirect leak of 2959 byte(s) in 1 object(s) allocated from:
    #0 0x70e0e935d174 in malloc /__w/rocm-libraries/rocm-libraries/TheRock/compiler/amd-llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:127:3
    #1 0x70e0cbb4d96d  (/home/rkayaith/repos/iree-workspace/workspaces/hipblaslt-hipsparselt-host-asan/tmp/comgr-existing-test/current-pin-dist/bin/../lib/libamd_comgr.so.3+0x94d96d) (BuildId: 14c1bc839727066f9233f0481c919d82cabe711d)
    #2 0x726170737069682c  (<unknown module>)

Indirect leak of 5 byte(s) in 1 object(s) allocated from:
    #0 0x70e0e935d174 in malloc /__w/rocm-libraries/rocm-libraries/TheRock/compiler/amd-llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:127:3
    #1 0x70e0cb7d1b06  (/home/rkayaith/repos/iree-workspace/workspaces/hipblaslt-hipsparselt-host-asan/tmp/comgr-existing-test/current-pin-dist/bin/../lib/libamd_comgr.so.3+0x5d1b06) (BuildId: 14c1bc839727066f9233f0481c919d82cabe711d)

SUMMARY: AddressSanitizer: 4204 byte(s) leaked in 3 allocation(s).

Submission Checklist

Risk level

Low (2/5): releases one acquired reference after the executable bytes have been copied.

@math-ci-jobs

math-ci-jobs Bot commented Oct 29, 2026

Copy link
Copy Markdown

Generated Documentation

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are recorded for the change adding cleanup of the COMGR executable-data reference after its bytes are copied.

No open issues 🎉

What changed in this PR

Fixes a host-memory leak in rocRoller’s in-process assembler, identified while expanding AddressSanitizer coverage.

Changes:

  • Releases the executable-data reference acquired from COMGR after its bytes have been copied.
  • The PR reports that the existing CPU-only assembler test’s detected leaks dropped from 4,204 bytes to zero.
File Description
shared/​rocroller/​lib/​source/​Assemblers/​InProcessAssembler.cpp Adds the missing amd_comgr_release_data(execData) cleanup call.

Review effort: Balanced

@rkayaith
rkayaith removed this pull request from stack #14699 October 29, 2026 23:00
@rkayaith
rkayaith added this pull request to stack #14715 October 29, 2026 23:00
@math-ci-jobs

math-ci-jobs Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Code Coverage Report for gfx950

Summary

Type Total Missed Master Missed Missed Change Coverage Master Coverage Coverage Change
Lines 71790 14634 14551 83 79.62% 79.73% -.11%
Functions 6455 1356 1353 3 78.99% 79.04% -.05%
Regions 44841 13003 12982 21 71.00% 71.04% -.04%
Branches 24386 7107 7089 18 70.86% 70.93% -.07%

This PR adds/edits 62 newly uncovered lines.

Artifacts

Commit Hashes

@rkayaith
rkayaith marked this pull request as ready for review October 30, 2026 19:24
@rkayaith
rkayaith requested a review from a team as a code owner October 30, 2026 19:24
@therock-pr-bot

therock-pr-bot Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: shared/rocroller/lib/source/Assemblers/InProcessAssembler.cpp; no test file found
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

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

📖 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.

@rkayaith
rkayaith force-pushed the users/rkayaith/hipblaslt-tuning-scratch-leak branch from c523531 to 0abae0b Compare November 2, 2026 18:12
@rkayaith
rkayaith force-pushed the users/rkayaith/ROCM-33540-comgr-executable-release branch from b04145e to 01beaca Compare November 2, 2026 18:12
amd_comgr_action_data_get_data increments the executable data reference count. Dataset destruction releases its own reference, leaving the separately acquired execData reference outstanding. Release execData after copying its bytes, alongside the existing assemblyData cleanup.

Fixes ROCM-33540.
@rkayaith
rkayaith removed this pull request from stack #14923 November 3, 2026 16:13
@rkayaith
rkayaith changed the base branch from users/rkayaith/hipblaslt-tuning-scratch-leak to develop November 3, 2026 16:14
@rkayaith
rkayaith requested a review from a team as a code owner November 3, 2026 16:14
@rkayaith
rkayaith force-pushed the users/rkayaith/ROCM-33540-comgr-executable-release branch from 01beaca to 49dcc3a Compare November 3, 2026 16:14
@rkayaith
rkayaith enabled auto-merge (squash) November 3, 2026 16:14
@rkayaith
rkayaith merged commit 3f51737 into develop Nov 4, 2026
111 of 112 checks passed
@rkayaith
rkayaith deleted the users/rkayaith/ROCM-33540-comgr-executable-release branch November 4, 2026 05:53
assistant-librarian Bot pushed a commit to ROCm/rocRoller that referenced this pull request Nov 4, 2026
fix(rocroller): release acquired COMGR executable data
 (#14698)

JIRA ID : ROCM-33540

## Motivation

rocRoller's in-process assembler acquires the linked executable from the
code-object manager (COMGR) but never releases that reference. Each
assembly leaks executable data, causing host AddressSanitizer runs to
fail at exit
([ROCM-33540](https://amd-hub.atlassian.net/browse/ROCM-33540)).

Found while expanding ASAN testing in
[#14240](ROCm/rocm-libraries#14240).

## Technical Details

Release the reference acquired by `amd_comgr_action_data_get_data` after
copying the executable bytes. Destroying the dataset releases its own
reference; it does not release this separately acquired reference.

## Test Plan

With a host AddressSanitizer (`HOST_ASAN`) TheRock build, run the
existing CPU-only test without leak suppressions:

```sh
env -u LD_LIBRARY_PATH -u LD_PRELOAD -u ROCROLLER_ASSEMBLER -u ROCROLLER_SAVE_ASSEMBLY \
  ASAN_OPTIONS=quarantine_size_mb=600:detect_leaks=1 LSAN_OPTIONS= \
  HIP_VISIBLE_DEVICES=-1 ROCR_VISIBLE_DEVICES= HSA_XNACK=0 \
  rocroller-tests --gtest_filter=AssemblerTest.Basic
```

## Test Result

Leaked bytes went from 4,204 to 0.

The `mci/rocm-libraries/code-analysis(rocroller)` failure is
pre-existing: the job fails before analysis at `docker build -f null`,
the same way it does on
[`develop`](https://math-ci.amd.com/job/rocm-libraries/job/code-analysis/job/rocroller/job/develop/)
and other PRs.

<details>
<summary>ASAN error output (before fix)</summary>

```text
=================================================================
==1667238==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1240 byte(s) in 1 object(s) allocated from:
    #0 0x70e0e936daed in operator new(unsigned long, std::nothrow_t const&) /__w/rocm-libraries/rocm-libraries/TheRock/compiler/amd-llvm/compiler-rt/lib/asan/asan_new_delete.cpp:114:3
    #1 0x70e0cb7f51a5 in amd_comgr_create_data (/home/rkayaith/repos/iree-workspace/workspaces/hipblaslt-hipsparselt-host-asan/tmp/comgr-existing-test/current-pin-dist/bin/../lib/libamd_comgr.so.3+0x5f51a5) (BuildId: 14c1bc839727066f9233f0481c919d82cabe711d)
    #2 0x7ffe694a9f17  (<unknown module>)

Indirect leak of 2959 byte(s) in 1 object(s) allocated from:
    #0 0x70e0e935d174 in malloc /__w/rocm-libraries/rocm-libraries/TheRock/compiler/amd-llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:127:3
    #1 0x70e0cbb4d96d  (/home/rkayaith/repos/iree-workspace/workspaces/hipblaslt-hipsparselt-host-asan/tmp/comgr-existing-test/current-pin-dist/bin/../lib/libamd_comgr.so.3+0x94d96d) (BuildId: 14c1bc839727066f9233f0481c919d82cabe711d)
    #2 0x726170737069682c  (<unknown module>)

Indirect leak of 5 byte(s) in 1 object(s) allocated from:
    #0 0x70e0e935d174 in malloc /__w/rocm-libraries/rocm-libraries/TheRock/compiler/amd-llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:127:3
    #1 0x70e0cb7d1b06  (/home/rkayaith/repos/iree-workspace/workspaces/hipblaslt-hipsparselt-host-asan/tmp/comgr-existing-test/current-pin-dist/bin/../lib/libamd_comgr.so.3+0x5d1b06) (BuildId: 14c1bc839727066f9233f0481c919d82cabe711d)

SUMMARY: AddressSanitizer: 4204 byte(s) leaked in 3 allocation(s).
```

</details>

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.

## Risk level

Low (2/5): releases one acquired reference after the executable bytes
have been copied.

[ROCM-33540]:
https://amd-hub.atlassian.net/browse/ROCM-33540?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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.

5 participants