Skip to content

Fix out-of-bounds read in bdsqr_lower2upper (ROCM-21372)#7915

Draft
Copilot wants to merge 2 commits into
developfrom
copilot/develop
Draft

Fix out-of-bounds read in bdsqr_lower2upper (ROCM-21372)#7915
Copilot wants to merge 2 commits into
developfrom
copilot/develop

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 30, 2026

In bdsqr_lower2upper(), array E has length n-1, but the final loop iteration (i == n-2) reads E[i+1]E[n-1], which is past the end of the array. This affects all data types via the templated implementation (float, double, complex float, complex double).

Change

  • rocauxiliary_bdsqr.hpp line 519: Guard the read with a bounds check; use 0 on the last iteration since g only seeds the next (non-existent) loop iteration.
// Before
g = E[i + 1];

// After
g = ((i + 1) < (n - 1)) ? E[i + 1] : 0;
Original prompt

Fix an out-of-bounds read in bdsqr_lower2upper() in projects/rocsolver/library/src/auxiliary/rocauxiliary_bdsqr.hpp in repository ROCm/rocm-libraries.

Context:

  • Jira ticket: ROCM-21372
  • The issue is reported at line 519 in routine bdsqr_lower2upper().
  • In this routine, array E has logical length n - 1.
  • Inside the loop, g = E[i + 1]; can read out of bounds on the last iteration when i == n - 2, because that evaluates to E[n - 1].
  • This affects all supported data types through the templated implementation, including float, double, complex float, and complex double.

Requested fix:

  • Add a bounds guard to prevent the out-of-bounds read on the last iteration.
  • A valid approach is:
    g = ((i + 1) < (n - 1)) ? E[i + 1] : 0;
  • Ensure the fix preserves existing algorithm behavior while avoiding invalid memory access.

Files:

  • projects/rocsolver/library/src/auxiliary/rocauxiliary_bdsqr.hpp

Requirements:

  • Implement the fix on branch develop.
  • Create a pull request with the change.
  • Include a clear PR description referencing ROCM-21372 and explaining that the final loop iteration must not read past the end of E.
  • If there are relevant tests for this path, update or add them if appropriate; otherwise keep the change minimal and safe.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Suppose you are an expert HIP ROCM programmer for AMD GPU, in Jira ticket ROCM-21372, there is mention of out of bounds access at line 519 of routine bdsqr_lower2upper() in rocauxiliary_bdsqr.hpp. The array "E" has length n-1, so access to "E[i+1]" may be a problem in the for loop at line 512. This error affects all data types (float, double, double complex, float complex).

Please create a PR to fix this out of bounds read access problem at line 519 by adding a bounds guard. Perhaps something like
" g = ( (i+1) < (n-1) ) ? E[i+1] : 0;".

Assistant: I’m waiting for your confirmation on the pull request creation request; once you accept it, I can continue with the fix.

This pull request was created from Copilot chat.

In bdsqr_lower2upper(), array E has logical length n-1. The loop runs
for i = 0 to n-2, and on the last iteration (i == n-2) the expression
E[i+1] evaluates to E[n-1], which is past the end of the array.

Add a bounds guard so the out-of-bounds read is avoided:
  g = ((i + 1) < (n - 1)) ? E[i + 1] : 0;
Copilot AI changed the title [WIP] Fix out-of-bounds read in bdsqr_lower2upper Fix out-of-bounds read in bdsqr_lower2upper (ROCM-21372) May 30, 2026
Copilot AI requested a review from EdDAzevedo May 30, 2026 19:32
@EdDAzevedo EdDAzevedo requested review from Copilot and tfalders May 30, 2026 19:33
@assistant-librarian assistant-librarian Bot added the external contribution Code contribution from users community.. label May 30, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes an out-of-bounds read in the GPU bdsqr_lower2upper kernel where the final loop iteration read E[n-1] past the end of the length-n-1 array. The value loaded into g on the last iteration is unused (no next lartg call occurs), so substituting 0 preserves behavior while eliminating the invalid access.

Changes:

  • Guard g = E[i + 1] with a bounds check, returning 0 on the last iteration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (77.83%) 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    #7915   +/-   ##
========================================
  Coverage    61.43%   61.43%           
========================================
  Files         2091     2091           
  Lines       358852   358852           
  Branches     54266    54266           
========================================
  Hits        220451   220451           
  Misses      119672   119672           
  Partials     18729    18729           
Flag Coverage Δ *Carryforward flag
TensileLite 27.41% <ø> (ø) Carriedforward from 9c82e2e
hipBLAS 90.65% <ø> (ø) Carriedforward from 9c82e2e
hipBLASLt 41.23% <ø> (ø) Carriedforward from 9c82e2e
hipCUB 82.21% <ø> (ø) Carriedforward from 9c82e2e
hipDNN 86.59% <ø> (ø) Carriedforward from 9c82e2e
hipFFT 51.59% <ø> (ø) Carriedforward from 9c82e2e
hipRAND 76.12% <ø> (ø) Carriedforward from 9c82e2e
hipSOLVER 69.24% <ø> (ø) Carriedforward from 9c82e2e
hipSPARSE 86.54% <ø> (ø) Carriedforward from 9c82e2e
rocBLAS 48.09% <ø> (ø) Carriedforward from 9c82e2e
rocFFT 45.24% <ø> (ø) Carriedforward from 9c82e2e
rocRAND 57.03% <ø> (ø) Carriedforward from 9c82e2e
rocSOLVER 77.83% <ø> (ø)
rocSPARSE 72.67% <ø> (ø) Carriedforward from 9c82e2e

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

Files with missing lines Coverage Δ
...olver/library/src/auxiliary/rocauxiliary_bdsqr.hpp 76.12% <ø> (ø)
🚀 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

Labels

external contribution Code contribution from users community.. project: rocsolver

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants