Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StorageLive: refresh storage (instead of UB) when local is already live #126154

Merged
merged 2 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ const_eval_division_by_zero =
dividing by zero
const_eval_division_overflow =
overflow in signed division (dividing MIN by -1)
const_eval_double_storage_live =
StorageLive on a local that was already live

const_eval_dyn_call_not_a_method =
`dyn` call trying to call something that is not a method
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1103,11 +1103,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
Operand::Immediate(Immediate::Uninit)
});

// StorageLive expects the local to be dead, and marks it live.
// If the local is already live, deallocate its old memory.
let old = mem::replace(&mut self.frame_mut().locals[local].value, local_val);
if !matches!(old, LocalValue::Dead) {
throw_ub_custom!(fluent::const_eval_double_storage_live);
}
self.deallocate_local(old)?;
Ok(())
}

Expand All @@ -1121,7 +1119,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
assert!(local != mir::RETURN_PLACE, "Cannot make return place dead");
trace!("{:?} is now dead", local);

// It is entirely okay for this local to be already dead (at least that's how we currently generate MIR)
// If the local is already dead, this is a NOP.
let old = mem::replace(&mut self.frame_mut().locals[local].value, LocalValue::Dead);
self.deallocate_local(old)?;
Ok(())
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,16 +361,19 @@ pub enum StatementKind<'tcx> {
/// At any point during the execution of a function, each local is either allocated or
/// unallocated. Except as noted below, all locals except function parameters are initially
/// unallocated. `StorageLive` statements cause memory to be allocated for the local while
/// `StorageDead` statements cause the memory to be freed. Using a local in any way (not only
/// reading/writing from it) while it is unallocated is UB.
/// `StorageDead` statements cause the memory to be freed. In other words,
/// `StorageLive`/`StorageDead` act like the heap operations `allocate`/`deallocate`, but for
/// stack-allocated local variables. Using a local in any way (not only reading/writing from it)
/// while it is unallocated is UB.
///
/// Some locals have no `StorageLive` or `StorageDead` statements within the entire MIR body.
/// These locals are implicitly allocated for the full duration of the function. There is a
/// convenience method at `rustc_mir_dataflow::storage::always_storage_live_locals` for
/// computing these locals.
///
/// If the local is already allocated, calling `StorageLive` again is UB. However, for an
/// unallocated local an additional `StorageDead` all is simply a nop.
/// If the local is already allocated, calling `StorageLive` again will implicitly free the
/// local and then allocate fresh uninitilized memory. If a local is already deallocated,
/// calling `StorageDead` again is a NOP.
StorageLive(Local),

/// See `StorageLive` above.
Expand Down
14 changes: 14 additions & 0 deletions src/tools/miri/tests/fail/storage-live-dead-var.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#![feature(core_intrinsics, custom_mir)]
use std::intrinsics::mir::*;

#[custom_mir(dialect = "runtime")]
fn main() {
mir! {
let val: i32;
{
val = 42; //~ERROR: accessing a dead local variable
StorageLive(val); // too late... (but needs to be here to make `val` not implicitly live)
Return()
}
}
}
15 changes: 15 additions & 0 deletions src/tools/miri/tests/fail/storage-live-dead-var.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: Undefined Behavior: accessing a dead local variable
--> $DIR/storage-live-dead-var.rs:LL:CC
|
LL | val = 42;
| ^^^^^^^^ accessing a dead local variable
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `main` at $DIR/storage-live-dead-var.rs:LL:CC

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

17 changes: 17 additions & 0 deletions src/tools/miri/tests/fail/storage-live-resets-var.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![feature(core_intrinsics, custom_mir)]
use std::intrinsics::mir::*;

#[custom_mir(dialect = "runtime")]
fn main() {
mir! {
let val: i32;
let _val2: i32;
{
StorageLive(val);
val = 42;
StorageLive(val); // reset val to `uninit`
_val2 = val; //~ERROR: uninitialized
Return()
}
}
}
15 changes: 15 additions & 0 deletions src/tools/miri/tests/fail/storage-live-resets-var.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: Undefined Behavior: constructing invalid value: encountered uninitialized memory, but expected an integer
--> $DIR/storage-live-resets-var.rs:LL:CC
|
LL | _val2 = val;
| ^^^^^^^^^^^ constructing invalid value: encountered uninitialized memory, but expected an integer
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `main` at $DIR/storage-live-resets-var.rs:LL:CC

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

Loading