dimforge / dimforge/nalgebra

Stacked Borrows UB in MatrixViewMut sub-view creation

Open
#1,588 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
4.8k
Forks
565
PR merge metrics
No merged PRs in 30d

Description

## What's happening

When you create a sub-view from a `MatrixViewMut` (e.g. via `rows_range_mut`), Miri flags UB under Stacked Borrows. The culprit is `&mut self.data` in the `matrix_view_impl!` macro — it performs a Unique retag that kills sibling pointers derived from the same parent `ArrayStorage`.

Found this while digging into #1520. The `axcpy_uninit` fix in #1587 addressed the immediate problem, but this one sits deeper in the view infrastructure.

## Where it happens

The `matrix_view_impl!` macro (`src/base/matrix_view.rs`, around line 819) generates `rows_range_mut` and friends. They go through `&mut self.data` on `ViewStorageMut`, which under Stacked Borrows retags with Unique over the entire pointer provenance — wiping out any sibling borrows from the same allocation.

In practice this fires whenever `columns_range_pair_mut` is followed by sub-view ops on the resulting views:

```rust
let (mut col_a, col_b) = matrix.columns_range_pair_mut(0, 1..2);
let sub = col_a.rows_range_mut(..);
// ^^ &mut self.data retags Unique, invalidating col_b's provenance
```

Every decomposition that uses `columns_range_pair_mut` + sub-views hits this path.

## Does it matter?

- **Stacked Borrows:** yes, UB
- **Tree Borrows:** no violation — doesn't invalidate siblings on mutable retag
- **Real-world:** no known miscompilations

## Repro

Minimal reproducer — any `SMatrix` decomposition triggers this:

```rust
fn main() {
let mat = nalgebra::Matrix2::::identity();
let _ = mat.cholesky();
}
```

```bash
# Stacked Borrows (Miri default) — fails:
cargo +nightly miri run

# Tree Borrows — passes:
MIRIFLAGS="-Zmiri-tree-borrows" cargo +nightly miri run
```

Note: requires #1587 to be merged first, otherwise Miri will hit the `axcpy_uninit` slice aliasing UB before reaching the view system issue.

## Possible fixes

1. **Skip `&mut self.data` in sub-view creation** — go through the raw pointer directly without an intermediate mutable reference to the storage
2. **Restructure `ViewStorageMut`** — hold a raw pointer instead of a borrow, so sub-view creation doesn't retag
3. **Target Tree Borrows** — document that nalgebra assumes Tree Borrows semantics and move on

(3) is the pragmatic call given Tree Borrows is where things are headed, but (1) or (2) would make it sound under both models.

## Related

- #1520 — original Miri report
- #1587 — `axcpy_uninit` fix

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in src/base/matrix_view.rs around the matrix_view_impl! macro and reproduce the issue with cargo +nightly miri run after #1587 is merged. Trace rows_range_mut and columns_range_pair_mut through ViewStorageMut, then verify that the chosen fix preserves sibling views and passes under Stacked Borrows as well as Tree Borrows.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.