[rocsparse] Add null/zero-size guards to memstat allocation functions#6597
Merged
kliegeois merged 2 commits intoROCm:developfrom Apr 21, 2026
Merged
[rocsparse] Add null/zero-size guards to memstat allocation functions#6597kliegeois merged 2 commits intoROCm:developfrom
kliegeois merged 2 commits intoROCm:developfrom
Conversation
rocsparse_free/rocsparse_free_async now return hipSuccess for nullptr. rocsparse_malloc/rocsparse_malloc_async now return hipSuccess with a nullptr output for zero-size allocations.
jsandham
approved these changes
Apr 20, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (47.50%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #6597 +/- ##
===========================================
+ Coverage 66.79% 66.82% +0.03%
===========================================
Files 2255 2255
Lines 325081 324953 -128
Branches 41532 41506 -26
===========================================
Hits 217132 217132
+ Misses 93289 93161 -128
Partials 14660 14660
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
ntrost57
approved these changes
Apr 21, 2026
assistant-librarian Bot
pushed a commit
to ROCm/rocSPARSE
that referenced
this pull request
Apr 21, 2026
[rocsparse] Add null/zero-size guards to memstat allocation functions (#6597) ## What changed and why The memstat-enabled variants of `rocsparse_free`, `rocsparse_free_async`, `rocsparse_malloc`, and `rocsparse_malloc_async` in `rocsparse_memstat.cpp` lack defensive guards for null pointers and zero-size allocations. - Calling `rocsparse_free(nullptr)` falls through to the HIP allocator and memstat bookkeeping unnecessarily. - Calling `rocsparse_malloc(&p, 0)` issues a zero-size allocation to HIP, whose behavior is implementation-defined. The non-memstat build path already handles these cases (e.g., the `rocsparse_hipFreeAsync` macro has a null check), but the memstat path did not, creating an inconsistency. This PR adds early-return guards: - `rocsparse_free` / `rocsparse_free_async`: return `hipSuccess` for `nullptr` without entering the allocator or memstat tracking. - `rocsparse_malloc` / `rocsparse_malloc_async`: set output to `nullptr` and return `hipSuccess` for zero-size requests. ## API changes / backward compatibility None. These are internal allocation wrappers, not part of the public API. The guards make behavior consistent with the non-memstat path. ## Performance impact None. The guards add a single branch at function entry, avoiding unnecessary work for no-op calls. ## Testing The existing test suite exercises these paths implicitly through normal library usage. The guards handle edge cases that previously produced undefined or unnecessary behavior. A memstat-enabled build can be used to verify correct bookkeeping. ## Areas needing review attention - Confirm that returning `hipSuccess` without calling `memstat::remove` is correct for the null-pointer free case (it is, since a null pointer was never tracked by `memstat::add`). - Confirm that zero-size allocations returning `nullptr` is the expected contract for all callers.
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
4 participants
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.
What changed and why
The memstat-enabled variants of
rocsparse_free,rocsparse_free_async,rocsparse_malloc, androcsparse_malloc_asyncinrocsparse_memstat.cpplack defensive guards for null pointers and zero-size allocations.rocsparse_free(nullptr)falls through to the HIP allocator and memstat bookkeeping unnecessarily.rocsparse_malloc(&p, 0)issues a zero-size allocation to HIP, whose behavior is implementation-defined.The non-memstat build path already handles these cases (e.g., the
rocsparse_hipFreeAsyncmacro has a null check), but the memstat path did not, creating an inconsistency.This PR adds early-return guards:
rocsparse_free/rocsparse_free_async: returnhipSuccessfornullptrwithout entering the allocator or memstat tracking.rocsparse_malloc/rocsparse_malloc_async: set output tonullptrand returnhipSuccessfor zero-size requests.API changes / backward compatibility
None. These are internal allocation wrappers, not part of the public API. The guards make behavior consistent with the non-memstat path.
Performance impact
None. The guards add a single branch at function entry, avoiding unnecessary work for no-op calls.
Testing
The existing test suite exercises these paths implicitly through normal library usage. The guards handle edge cases that previously produced undefined or unnecessary behavior. A memstat-enabled build can be used to verify correct bookkeeping.
Areas needing review attention
hipSuccesswithout callingmemstat::removeis correct for the null-pointer free case (it is, since a null pointer was never tracked bymemstat::add).nullptris the expected contract for all callers.