Nightly regressed igvm crate and now emits SIGILL at opt-level higher than 1 #136361
Assignees
Labels
A-mir-opt
Area: MIR optimizations
A-mir-opt-GVN
Area: MIR opt Global Value Numbering (GVN)
C-bug
Category: This is a bug.
I-miscompile
Issue: Correct Rust code lowers to incorrect machine code
I-unsound
Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
P-critical
Critical priority
regression-from-stable-to-nightly
Performance or correctness regression from stable to nightly.
S-has-bisection
Status: a bisection has been found for this issue
S-has-mcve
Status: A Minimal Complete and Verifiable Example has been found for this issue
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
durin42
added
C-bug
rustbot
added
I-prioritize
|
Update: a colleague has further reduced this to just zerocopy as a dep, and will share the sample here shortly |
|
This is a much simpler reproduction on rust playground. Note nightly and release mode must be selected for the SIGILL to occur. |
|
cc @scottmcm on that bisection |
jieyouxu
added
E-needs-mcve
|
Smaller: use std::slice;
fn main() {
let vp_ctx: &Box<[u8; 2]> = &Box::new([0, 0]);
let slf: *const [u8; 2] = &raw const **vp_ctx;
let bytes = unsafe { slice::from_raw_parts(slf.cast::<u8>(), 2) };
for _ in bytes {}
} |
jieyouxu
added
S-has-mcve
|
Cute -- @cyrgani's example compiles to almost nothing: 0000000000014b40 <_ZN11rust_1363614main17hc26238fcb10c7fa6E>:
14b40: 48 8b 05 81 3f 04 00 mov 0x43f81(%rip),%rax # 58ac8 <_DYNAMIC+0x220>
14b47: 0f b6 00 movzbl (%rax),%eax
14b4a: 0f 0b ud2And of course that FWIW, cranelift panics on this in release mode: cranelift panic and mir dump |
saethlin
added
the
I-unsound
|
Is this a duplicate of #135997? |
This comment has been minimized.
This comment has been minimized.
|
Reproducer without unsafe: fn main() {
let vp_ctx: &Box<u8> = &Box::new(0);
let slf: *const u8 = &raw const **vp_ctx;
let bytes = std::ptr::slice_from_raw_parts(slf, 1);
let _x = foo(bytes);
}
fn foo(bytes: *const [u8]) -> *mut u8 {
bytes as *const u8 as *mut u8 as *const u8 as *mut u8
} |
|
The issue reproduces even with a ZST. fn main() {
let vp_ctx: &Box<()>= &Box::new(());
let slf: *const () = &raw const **vp_ctx;
let bytes = std::ptr::slice_from_raw_parts(slf, 1);
let _x = foo(bytes);
}
fn foo(bytes: *const [()]) -> *mut () {
bytes as *const () as *mut () as *const () as *mut ()
} |
Noratrieb
added
P-critical
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Feb 2, 2025
Don't reset cast kind without also updating the operand in `simplify_cast` in GVN
Consider this heavily elided segment of the pre-GVN example code that was committed as a test:
```
let _4: *const ();
let _5: *const [()];
let mut _6: *const ();
let _7: *mut ();
let mut _8: *const [()];
let mut _9: std::boxed::Box<()>;
let mut _10: *const ();
/* ... */
_10 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *const () (Transmute);
_4 = copy _10;
_6 = copy _4;
_5 = *const [()] from (copy _6, copy _11);
_8 = copy _5;
_7 = copy _8 as *mut () (PtrToPtr);
```
A malformed optimization was changing `_7` to:
```
_7 = copy _5 as *mut () (Transmute);
```
(where `_8` was just replaced with `_5` bc of simple copy propagation, that part is not important... the CastKind changing to Transmute is the important part here).
In rust-lang#133324, two new functionalities were implemented:
* Peeking through unsized -> sized PtrToPtr casts whose operand is `AggregateKind::RawPtr`, to turn it into PtrToPtr casts of the base of the aggregate. In this case, this allows us to see that the value of `_7` is just a ptr-to-ptr cast of `_6`.
* Folding a PtrToPtr cast of an operand which is a Transmute cast into just a single Transmute, which (theoretically) allows us to treat `_7` as a transmute into `*mut ()` of the base of the cast of `_10`, which is the place projection of `((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>)`.
However, when applying those two subsequent optimizations, we must *not* update the CastKind of the final cast *unless* we also update the operand of the cast, since the operand may no longer make sense with the updated CastKind.
In this case, this is problematic because the type of `_8` is `*const [()]`, but that operand in assignment statement of `_7` does *not* get turned into something like `((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>)` -- **in other words, `try_to_operand` fails** -- because GVN only turns value nodes into locals or consts, not projections of locals. So we fail to update the operand, but we still update the CastKind to Transmute, which means we now are transmuting types of different sizes (a wide pointer and a thin pointer).
r? `@scottmcm` or `@cjgillot`
Fixes rust-lang#136361
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Feb 6, 2025
…thlin
Don't reset cast kind without also updating the operand in `simplify_cast` in GVN
Consider this heavily elided segment of the pre-GVN example code that was committed as a test:
```rust
let _4: *const ();
let _5: *const [()];
let mut _6: *const ();
let _7: *mut ();
let mut _8: *const [()];
let mut _9: std::boxed::Box<()>;
let mut _10: *const ();
/* ... */
// Deref a box
_10 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *const () (Transmute);
_4 = copy _10;
_6 = copy _4;
// Inlined body of `slice::from_raw_parts`, to turn a unit pointer into a slice-of-unit pointer
_5 = *const [()] from (copy _6, copy _11);
_8 = copy _5;
// Cast the raw slice-of-unit pointer back to a unit pointer
_7 = copy _8 as *mut () (PtrToPtr);
```
A malformed optimization was changing `_7` (which casted the slice-of-unit ptr to a unit ptr) to:
```
_7 = copy _5 as *mut () (Transmute);
```
...where `_8` was just replaced with `_5` bc of simple copy propagation, that part is not important... the CastKind changing to Transmute is the important part here.
In rust-lang#133324, two new functionalities were implemented:
* Peeking through unsized -> sized PtrToPtr casts whose operand is `AggregateKind::RawPtr`, to turn it into PtrToPtr casts of the base of the aggregate. In this case, this allows us to see that the value of `_7` is just a ptr-to-ptr cast of `_6`.
* Folding a PtrToPtr cast of an operand which is a Transmute cast into just a single Transmute, which (theoretically) allows us to treat `_7` as a transmute into `*mut ()` of the base of the cast of `_10`, which is the place projection of `((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>)`.
However, when applying those two subsequent optimizations, we must *not* update the CastKind of the final cast *unless* we also update the operand of the cast, since the operand may no longer make sense with the updated CastKind.
In this case, this is problematic because the type of `_8` is `*const [()]`, but that operand in assignment statement of `_7` does *not* get turned into something like `((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>)` -- **in other words, `try_to_operand` fails** -- because GVN only turns value nodes into locals or consts, not projections of locals. So we fail to update the operand, but we still update the CastKind to Transmute, which means we now are transmuting types of different sizes (a wide pointer and a thin pointer).
r? `@scottmcm` or `@cjgillot`
Fixes rust-lang#136361
Fixes rust-lang#135997
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Feb 7, 2025
…thlin
Don't reset cast kind without also updating the operand in `simplify_cast` in GVN
Consider this heavily elided segment of the pre-GVN example code that was committed as a test:
```rust
let _4: *const ();
let _5: *const [()];
let mut _6: *const ();
let _7: *mut ();
let mut _8: *const [()];
let mut _9: std::boxed::Box<()>;
let mut _10: *const ();
/* ... */
// Deref a box
_10 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *const () (Transmute);
_4 = copy _10;
_6 = copy _4;
// Inlined body of `slice::from_raw_parts`, to turn a unit pointer into a slice-of-unit pointer
_5 = *const [()] from (copy _6, copy _11);
_8 = copy _5;
// Cast the raw slice-of-unit pointer back to a unit pointer
_7 = copy _8 as *mut () (PtrToPtr);
```
A malformed optimization was changing `_7` (which casted the slice-of-unit ptr to a unit ptr) to:
```
_7 = copy _5 as *mut () (Transmute);
```
...where `_8` was just replaced with `_5` bc of simple copy propagation, that part is not important... the CastKind changing to Transmute is the important part here.
In rust-lang#133324, two new functionalities were implemented:
* Peeking through unsized -> sized PtrToPtr casts whose operand is `AggregateKind::RawPtr`, to turn it into PtrToPtr casts of the base of the aggregate. In this case, this allows us to see that the value of `_7` is just a ptr-to-ptr cast of `_6`.
* Folding a PtrToPtr cast of an operand which is a Transmute cast into just a single Transmute, which (theoretically) allows us to treat `_7` as a transmute into `*mut ()` of the base of the cast of `_10`, which is the place projection of `((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>)`.
However, when applying those two subsequent optimizations, we must *not* update the CastKind of the final cast *unless* we also update the operand of the cast, since the operand may no longer make sense with the updated CastKind.
In this case, this is problematic because the type of `_8` is `*const [()]`, but that operand in assignment statement of `_7` does *not* get turned into something like `((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>)` -- **in other words, `try_to_operand` fails** -- because GVN only turns value nodes into locals or consts, not projections of locals. So we fail to update the operand, but we still update the CastKind to Transmute, which means we now are transmuting types of different sizes (a wide pointer and a thin pointer).
r? `@scottmcm` or `@cjgillot`
Fixes rust-lang#136361
Fixes rust-lang#135997
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Code
I tried this code:
durin42/igvm@1880200
cargo +nightly test --releaseI expected to see this happen: Tests all pass.
Instead, this happened: Tests die with a SIGILL.
Version it worked on
This works on stable, and also up until b6b8361 (merge of #133324)
Version with regression
b6b8361 breaks, aka the merge of #133324. All nightlies since then show the same behavior.
Backtrace
I don't get one, I assume because it's a crash.
bisect-rustc output:
searched nightlies: from nightly-2025-01-07 to nightly-2025-01-13
regressed nightly: nightly-2025-01-10
searched commit range: a580b5c...8247594
regressed commit: b6b8361
bisected with cargo-bisect-rustc v0.6.8
Host triple: x86_64-unknown-linux-gnu
Reproduce with:
cargo bisect-rustc --start 2025-01-07 --end 2025-01-13 -- test --releaseIt's plausible to me that one of the crates in here is guilty of some unsafe crimes, but I'm not sure how to prove who's at fault at this point.
@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged
The text was updated successfully, but these errors were encountered: