Commit caa5994
authored
fix(rt): bug fix for
This PR addresses an issue that was first identified in solana-token's
`test_validate_owner_multisig.rs` proof. `iter_next_3.rs` is a minimized
test that reproduces the issue.
### Problem
When executing Rust's slice iterator (`core::slice::Iter`), the
`Iter::new` function creates two pointers: a start pointer and an end
pointer. The `next` method compares these pointers for equality to
determine whether iteration is complete.
During `Iter::new`, the end pointer undergoes a `PtrToPtr` cast (e.g.,
from `*const [u8; 32]` to `*mut [u8; 32]`) that changes its initially
correct metadata to one that causes the issue.
- Initially correct metadata: `metadata(staticSize(32), 3,
dynamicSize(3))`
- Resulting metadata: `metadata(staticSize(32), 3, staticSize(32))`
- Applied `convertMetadata` rule:
https://github.com/runtimeverification/mir-semantics/blob/8b331c654d225b13b410970599e559e95947edc5/kmir/src/kmir/kdist/mir-semantics/rt/data.md?plain=1#L1508
This rule sets the origin to `staticSize(32)` (matched with
`ORIGIN_SIZE`), discarding the previous correct `dynamicSize(3)`.
The start pointer does not undergo this additional cast, and maintains
its metadata: `metadata(staticSize(32), 3, dynamicSize(3))`.
This causes the two pointers to end up with different metadata:
`metadata(staticSize(32), 3, dynamicSize(3))` vs
`metadata(staticSize(32), 3, staticSize(32))`. When `Iter::next`
compares the pointers for equality at the end of iteration, the
comparison evaluates to `false` instead of `true`, causing the proof to
take the wrong branch (not-at-the-end) and eventually get stuck on an
out-of-bounds access.
### Solution
We add a `#cast` rule for `castKindPtrToPtr`, with higher priority, that
matches when the source and target pointer types have identical pointee
types. This rule preserves the source pointer metadata. This works
because when the pointee types are identical, the pointer representation
does not need to change. The only reason such a cast exists in MIR is a
mutability change, which does not affect the pointer's runtime
representation or metadata. When the pointee types are not identical,
the rule does not match, and we fallback to the existing rule and handle
the cast as before.
For casts where both the pointee type and mutability differ (e.g.,
`*const T1` to `*mut T2`: `T1=/=T2`), I am not sure how MIR is generated
for these, but either case is handled correctly:
- If it is a single cast, the pointee types differ, so the new rule does
not match and the fallback rule handles it as before. Mutability is not
involved in type projections or equality checks, so it does not
interfere.
- If it is two separate casts (one for mutability, one for the pointee
type), the new rule matches the mutability cast and the fallback rule
handles the pointee type cast.
### Why mutability information was not added
It would be possible to thread mutability information through the
semantics (from the smir.json, which now includes mutability on
`PtrType` and `RefType`, PR
[here](runtimeverification/stable-mir-json#127))
and add an explicit check that the cast is indeed a mutability-only
change. I chose not to include it, with the following reasoning.
The new mutability information would only be used to confirm that the
source and target pointer types differ in mutability in the new `#cast`
rule. Any other place where the new mutability information would be
threaded through would be `_`. At the same time, if the source and
target pointer types have identical pointee types, the only possible
difference that would have led to a cast is a difference in mutability.
Propagating the mutability in the semantics for this check alone seems
to add overhead for no benefit, especially considering that mutability
is not checked/enforced anywhere else.
So, it seems sufficient to just rely on the fact that because the
pointee type is the same, we do not need to change the metadata of the
pointer, so we simply do not. Mutability does not affect that.castKindPtrToPtr (#974)1 parent 022dafb commit caa5994
4 files changed
Lines changed: 51 additions & 0 deletions
File tree
- kmir/src
- kmir/kdist/mir-semantics/rt
- tests/integration
- data/prove-rs
- show
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1440 | 1440 | | |
1441 | 1441 | | |
1442 | 1442 | | |
| 1443 | + | |
| 1444 | + | |
| 1445 | + | |
| 1446 | + | |
| 1447 | + | |
| 1448 | + | |
| 1449 | + | |
| 1450 | + | |
| 1451 | + | |
| 1452 | + | |
| 1453 | + | |
| 1454 | + | |
| 1455 | + | |
| 1456 | + | |
1443 | 1457 | | |
1444 | 1458 | | |
1445 | 1459 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
Lines changed: 17 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| 46 | + | |
46 | 47 | | |
47 | 48 | | |
48 | 49 | | |
| |||
0 commit comments