Skip to content

fix(rocthrust): harden HIPSTDPAR allocation interposition - #11498

Draft
sstamenk wants to merge 2 commits into
developfrom
users/sstamenk/SWSPLAT-24312
Draft

fix(rocthrust): harden HIPSTDPAR allocation interposition#11498
sstamenk wants to merge 2 commits into
developfrom
users/sstamenk/SWSPLAT-24312

Conversation

@sstamenk

Copy link
Copy Markdown
Member

Validate allocation arithmetic and alignment, preserve libc and C++ allocation contracts, and clean up failed allocations in both interposer versions.

Add behavioral coverage for calloc, reallocarray, aligned allocation, new/delete, mmap, and forced-v0 execution.

JIRA: SWSPLAT-24312

Motivation

HIPSTDPAR allocation interposition did not check calloc multiplication overflow and could pass a null allocation to memset. This could cause undersized allocations, memory corruption, or crashes when allocation sizes are attacker-controlled.

The surrounding allocation shims also had incomplete libc, POSIX, and C++ allocation semantics. This PR hardens both the current v1 interposer and the legacy v0 fallback while adding behavioral regression coverage.

Technical Details

  • Reject overflowing calloc and reallocarray size calculations with ENOMEM.
  • Check allocation results before zero-initialization.
  • Preserve the original allocation when reallocarray rejects an overflowing request.
  • Propagate v0 hipMallocManaged failures safely through the PMR allocation boundary.
  • Refactor v0 aligned allocation to use std::align.
  • Validate power-of-two alignment and internal allocation-size arithmetic.
  • Correct posix_memalign behavior:
    • return 0 on success;
    • return EINVAL for invalid alignment;
    • return ENOMEM on allocation failure;
    • preserve errno and the output pointer on failure.
  • Implement C++ allocation semantics:
    • normalize zero-sized operator new requests;
    • invoke and retry through std::new_handler;
    • throw std::bad_alloc;
    • return nullptr from nothrow overloads.
  • Make free(nullptr) a no-op.
  • Route v0 sized and aligned-sized deletion through stored allocation metadata.
  • Release v1 allocations when device lookup or hipMemAdvise fails.
  • Preserve MAP_FAILED semantics and clean up mappings on v1 setup failure.
  • Ensure underlying free and munmap still execute when HIP un-advice fails.
  • Add explicit v0 test targets by undefining __HIPSTDPAR_INTERPOSE_ALLOC_V1__.
  • Use noinline runtime wrappers in tests so optimized builds cannot fold allocation calls before HipStdPar interposition.

Test Plan

  • Configure, build, and install rocThrust as a clean header-only package.
  • Build and run the following HIPSTDPAR tests for both v1 and forced-v0:
    • test_interpose
    • test_calloc
    • test_realloc
  • Exercise Debug, optimized Release, and UBSan configurations.
  • Build and run the complete HIPSTDPAR CTest suite on a synthetic merge with current develop.
  • Verify optimized LLVM IR retains calls to the intended HipStdPar allocation and deallocation helpers.
  • Repeat Release tests with glibc heap checking and memory perturbation enabled.

Test Result

  • All 7 official HIPSTDPAR CTest cases passed.
  • All v0/v1 Debug, Release, and UBSan allocation test configurations passed.
  • 150 repeated Release executions passed.
  • 60 additional executions with MALLOC_CHECK_=3 and MALLOC_PERTURB_=165 passed.
  • Clean package build and installation passed.
  • Synthetic merge with current develop built and tested without conflicts.
  • git diff --check passed.

Submission Checklist

Validate allocation arithmetic and alignment, preserve libc and C++ allocation contracts, and clean up failed allocations in both interposer versions.

Add behavioral coverage for calloc, reallocarray, aligned allocation, new/delete, mmap, and forced-v0 execution.

JIRA: SWSPLAT-24312
Keep null free, zero-size new, and sized delete calls visible under optimization, and clarify zero-size calloc expectations.
@therock-pr-bot

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
JIRA ID : TESTAUTO-6039
JIRA ID - #330
JIRA ID #330
JIRA ID (on separate line)
ROCM-25757
ISSUE ID : TESTUTO-3334
ISSUE ID #3334
ISSUE ID - TESTAUTO-3433
ISSUE ID (on separate line)
AIRUNTIME-2352
ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
Closes #10
Fixes octo-org/octo-repo#100
Resolves: #123
#123
https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword reference found
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

📖 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

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #11498      +/-   ##
===========================================
+ Coverage    69.65%   69.72%   +0.07%     
===========================================
  Files         2753     2783      +30     
  Lines       453403   455349    +1946     
  Branches     66800    67109     +309     
===========================================
+ Hits        315806   317479    +1673     
- Misses      117321   117558     +237     
- Partials     20276    20312      +36     
Flag Coverage Δ *Carryforward flag
TensileLite 34.23% <ø> (ø) Carriedforward from b91a3e7
TensileLite-CPP 38.12% <ø> (ø) Carriedforward from b91a3e7
TensileLite-Unit 64.82% <ø> (ø) Carriedforward from b91a3e7
hipBLAS 90.62% <ø> (ø) Carriedforward from b91a3e7
hipBLASLt 34.89% <ø> (ø) Carriedforward from b91a3e7
hipCUB 83.47% <ø> (ø) Carriedforward from b91a3e7
hipDNN 86.48% <ø> (ø) Carriedforward from b91a3e7
hipFFT 47.34% <ø> (ø) Carriedforward from b91a3e7
hipRAND 76.12% <ø> (ø) Carriedforward from b91a3e7
hipSOLVER 69.18% <ø> (ø) Carriedforward from b91a3e7
hipSPARSE 86.27% <ø> (ø) Carriedforward from b91a3e7
rocBLAS 48.23% <ø> (ø) Carriedforward from b91a3e7
rocFFT 47.61% <ø> (ø) Carriedforward from b91a3e7
rocRAND 57.02% <ø> (ø) Carriedforward from b91a3e7
rocSOLVER 76.92% <ø> (ø) Carriedforward from b91a3e7
rocSPARSE 72.56% <ø> (ø) Carriedforward from b91a3e7
rocThrust 91.34% <ø> (-0.26%) ⬇️

*This pull request uses carry forward flags. Click here to find out more.
see 328 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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