Skip to content

fix(rocRAND): fix Sobol rightmost_zero_bit at period wrap - #13327

Merged
amd-mcieslak merged 1 commit into
developfrom
users/mcieslak/rocrand_fix_sobol_wrap
Oct 9, 2026
Merged

amd-mcieslak merged 1 commit into
developfrom
users/mcieslak/rocrand_fix_sobol_wrap

Conversation

@amd-mcieslak

Copy link
Copy Markdown
Contributor

JIRA ID : ROCM-31895

Motivation

AddressSanitizer reports a stack-buffer-overflow in Sobol32 host generation (int_test_large/Sobol32_*, RUN_SLOW_TESTS=1). The same root cause produces silently wrong values on device for Sobol32, Scrambled Sobol32, Sobol64 and Scrambled Sobol64 whenever a single generate() call crosses the end of the sequence period (2^32 / 2^64).

Technical Details

rightmost_zero_bit(x) has no valid result when x is all ones, which happens when the engine advances from index 2^N-1 to 0:

  • host: the loop returned 32/64, causing an out-of-bounds read of vectors[32]/vectors[64] in discard_state_power2 (the ASan finding)

Wrapping from 2^N-1 to 0 flips only the top bit of the Gray code, so the correct index is 31 (Sobol32) / 63 (Sobol64). rightmost_zero_bit now returns that on host and device. Results for all other inputs are unchanged, so output before the wrap is bit-identical.

Test Plan

  • Build with BUILD_ADDRESS_SANITIZER=ON (gfx90a:xnack+), RUN_SLOW_TESTS=1
  • test_rocrand_host: int_test_large/Sobol32_* and int_test_large/Sobol64_*
  • Full test_rocrand_host

Test Result

  • int_test_large/Sobol32_*: 4/4 pass, no ASan findings (previously aborted with stack-buffer-overflow)
  • int_test_large/Sobol64_*: pass
  • ctest Sobol suites: 8/8 pass
  • full test_rocrand_host: pass

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Oct 8, 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: projects/rocrand/library/include/rocrand/rocrand_sobol32.h, projects/rocrand/library/include/rocrand/rocrand_sobol64.h; 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

therock-pr-bot Bot commented Oct 8, 2026

Copy link
Copy Markdown

Pre-commit check failed

⛔ pre-commit failed

Please run locally:

  • python -m pip install pre-commit
  • pre-commit install
  • pre-commit run --all-files --show-diff-on-failure

This repo uses .pre-commit-config.yaml.

@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 28.57143% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../rocrand/library/include/rocrand/rocrand_sobol32.h 40.00% 2 Missing and 1 partial ⚠️
.../rocrand/library/include/rocrand/rocrand_sobol64.h 0.00% 1 Missing and 1 partial ⚠️

❌ Your project check has failed because the head coverage (57.42%) 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   #13327      +/-   ##
===========================================
- Coverage    72.14%   72.14%   -0.00%     
===========================================
  Files         2868     2868              
  Lines       473296   473300       +4     
  Branches     69809    69811       +2     
===========================================
  Hits        341437   341437              
- Misses      108152   108154       +2     
- Partials     23707    23709       +2     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 49.33% <ø> (ø) Carriedforward from 21906e4
TensileLite-Unit 83.32% <ø> (ø) Carriedforward from 21906e4
hipBLAS 90.62% <ø> (ø) Carriedforward from 21906e4
hipBLASLt 39.34% <ø> (ø) Carriedforward from 21906e4
hipCUB 82.68% <ø> (ø) Carriedforward from 21906e4
hipDNN 86.66% <ø> (ø) Carriedforward from 21906e4
hipFFT 42.52% <ø> (ø) Carriedforward from 21906e4
hipRAND 76.12% <ø> (ø) Carriedforward from 21906e4
hipSOLVER 68.96% <ø> (ø) Carriedforward from 21906e4
hipSPARSE 86.99% <ø> (ø) Carriedforward from 21906e4
rocBLAS 48.17% <ø> (ø) Carriedforward from 21906e4
rocFFT 51.45% <ø> (ø) Carriedforward from 21906e4
rocRAND 57.42% <28.57%> (-0.03%) ⬇️
rocSOLVER 76.83% <ø> (ø) Carriedforward from 21906e4
rocSPARSE 74.07% <ø> (ø) Carriedforward from 21906e4
rocThrust 93.15% <ø> (ø) Carriedforward from 21906e4

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
.../rocrand/library/include/rocrand/rocrand_sobol64.h 72.00% <0.00%> (-1.97%) ⬇️
.../rocrand/library/include/rocrand/rocrand_sobol32.h 72.00% <40.00%> (-1.97%) ⬇️

... and 13 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.

@amd-jmahovsky amd-jmahovsky 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.

LGTM

JIRA ID : ROCM-31895

## Motivation

AddressSanitizer reports a stack-buffer-overflow in Sobol32 host
generation (int_test_large/Sobol32_*, RUN_SLOW_TESTS=1). The same root
cause produces silently wrong values on device for Sobol32, Scrambled
Sobol32, Sobol64 and Scrambled Sobol64 whenever a single generate() call
crosses the end of the sequence period (2^32 / 2^64).

## Technical Details

rightmost_zero_bit(x) has no valid result when x is all ones, which
happens when the engine advances from index 2^N-1 to 0:
- host: the loop returned 32/64, causing an out-of-bounds read of
  vectors[32]/vectors[64] in discard_state_power2 (the ASan finding)

Wrapping from 2^N-1 to 0 flips only the top bit of the Gray code, so the
correct index is 31 (Sobol32) / 63 (Sobol64). rightmost_zero_bit now
returns that on host and device. Results for all other inputs are
unchanged, so output before the wrap is bit-identical.

## Test Plan

- Build with BUILD_ADDRESS_SANITIZER=ON (gfx90a:xnack+), RUN_SLOW_TESTS=1
- test_rocrand_host: int_test_large/Sobol32_* and int_test_large/Sobol64_*
- Full test_rocrand_host

## Test Result

- int_test_large/Sobol32_*: 4/4 pass, no ASan findings (previously
  aborted with stack-buffer-overflow); int_test_large/Sobol64_*: pass
- ctest Sobol suites: 8/8 pass; full test_rocrand_host: pass

## Submission Checklist

- [x] Look over the contributing guidelines at
    https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
@amd-mcieslak
amd-mcieslak force-pushed the users/mcieslak/rocrand_fix_sobol_wrap branch from 1b3528d to a3d692d Compare October 8, 2026 22:28
@therock-pr-bot

therock-pr-bot Bot commented Oct 8, 2026

Copy link
Copy Markdown

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

@amd-mcieslak
amd-mcieslak merged commit 6d48fa1 into develop Oct 9, 2026
104 of 106 checks passed
@amd-mcieslak
amd-mcieslak deleted the users/mcieslak/rocrand_fix_sobol_wrap branch October 9, 2026 17:12
assistant-librarian Bot pushed a commit to ROCm/rocRAND that referenced this pull request Oct 9, 2026
fix(rocRAND): fix Sobol rightmost_zero_bit at period wrap
 (#13327)

JIRA ID : ROCM-31895

## Motivation

AddressSanitizer reports a stack-buffer-overflow in Sobol32 host
generation (int_test_large/Sobol32_*, RUN_SLOW_TESTS=1). The same root
cause produces silently wrong values on device for Sobol32, Scrambled
Sobol32, Sobol64 and Scrambled Sobol64 whenever a single generate() call
crosses the end of the sequence period (2^32 / 2^64).

## Technical Details

rightmost_zero_bit(x) has no valid result when x is all ones, which
happens when the engine advances from index 2^N-1 to 0:
- host: the loop returned 32/64, causing an out-of-bounds read of
vectors[32]/vectors[64] in discard_state_power2 (the ASan finding)

Wrapping from 2^N-1 to 0 flips only the top bit of the Gray code, so the
correct index is 31 (Sobol32) / 63 (Sobol64). rightmost_zero_bit now
returns that on host and device. Results for all other inputs are
unchanged, so output before the wrap is bit-identical.

## Test Plan

- Build with BUILD_ADDRESS_SANITIZER=ON (gfx90a:xnack+),
RUN_SLOW_TESTS=1
- test_rocrand_host: int_test_large/Sobol32_* and
int_test_large/Sobol64_*
- Full test_rocrand_host

## Test Result

- int_test_large/Sobol32_*: 4/4 pass, no ASan findings (previously
aborted with stack-buffer-overflow)
- int_test_large/Sobol64_*: pass
- ctest Sobol suites: 8/8 pass
- full test_rocrand_host: pass

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
@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.

2 participants