Skip to content

Commit

Permalink
Auto merge of #129199 - RalfJung:writes_through_immutable_pointer, r=…
Browse files Browse the repository at this point in the history
…<try>

make writes_through_immutable_pointer a hard error

This turns the lint added in #118324 into a hard error. This has been reported in cargo's future-compat reports since Rust 1.76 (released in February). Given that const_mut_refs is still unstable, it should be impossible to even hit this error on stable: we did accidentally stabilize some functions that can cause this error, but that got reverted in #117905. Still, let's do a crater run just to be sure.

Given that this should only affect unstable code, I don't think it needs an FCP, but let's Cc `@rust-lang/lang` anyway -- any objection to making this unambiguous UB into a hard error during const-eval? This can be viewed as part of #129195 which is already nominated for discussion.
  • Loading branch information
bors committed Aug 17, 2024
2 parents 426a60a + 8b642a1 commit 2c70eb4
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 102 deletions.
8 changes: 7 additions & 1 deletion compiler/rustc_const_eval/src/const_eval/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub enum ConstEvalErrKind {
RecursiveStatic,
AssertFailure(AssertKind<ConstInt>),
Panic { msg: Symbol, line: u32, col: u32, file: Symbol },
WriteThroughImmutablePointer,
}

impl MachineStopType for ConstEvalErrKind {
Expand All @@ -35,12 +36,16 @@ impl MachineStopType for ConstEvalErrKind {
Panic { .. } => const_eval_panic,
RecursiveStatic => const_eval_recursive_static,
AssertFailure(x) => x.diagnostic_message(),
WriteThroughImmutablePointer => const_eval_write_through_immutable_pointer,
}
}
fn add_args(self: Box<Self>, adder: &mut dyn FnMut(DiagArgName, DiagArgValue)) {
use ConstEvalErrKind::*;
match *self {
RecursiveStatic | ConstAccessesMutGlobal | ModifiedGlobal => {}
RecursiveStatic
| ConstAccessesMutGlobal
| ModifiedGlobal
| WriteThroughImmutablePointer => {}
AssertFailure(kind) => kind.add_args(adder),
Panic { msg, line, col, file } => {
adder("msg".into(), msg.into_diag_arg());
Expand Down Expand Up @@ -159,6 +164,7 @@ where

/// Emit a lint from a const-eval situation, with a backtrace.
// Even if this is unused, please don't remove it -- chances are we will need to emit a lint during const-eval again in the future!
#[allow(unused)]
pub(super) fn lint<'tcx, L>(
tcx: TyCtxtAt<'tcx>,
machine: &CompileTimeMachine<'tcx>,
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use rustc_middle::query::TyCtxtAt;
use rustc_middle::ty::layout::{FnAbiOf, TyAndLayout};
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::{bug, mir};
use rustc_session::lint::builtin::WRITES_THROUGH_IMMUTABLE_POINTER;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::Span;
use rustc_target::abi::{Align, Size};
Expand Down Expand Up @@ -729,8 +728,8 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
}

fn before_memory_write(
tcx: TyCtxtAt<'tcx>,
machine: &mut Self,
_tcx: TyCtxtAt<'tcx>,
_machine: &mut Self,
_alloc_extra: &mut Self::AllocExtra,
(_alloc_id, immutable): (AllocId, bool),
range: AllocRange,
Expand All @@ -741,9 +740,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
}
// Reject writes through immutable pointers.
if immutable {
super::lint(tcx, machine, WRITES_THROUGH_IMMUTABLE_POINTER, |frames| {
crate::errors::WriteThroughImmutablePointer { frames }
});
return Err(ConstEvalErrKind::WriteThroughImmutablePointer.into());
}
// Everything else is fine.
Ok(())
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,13 +407,6 @@ pub struct ConstEvalError {
pub frame_notes: Vec<FrameNote>,
}

#[derive(LintDiagnostic)]
#[diag(const_eval_write_through_immutable_pointer)]
pub struct WriteThroughImmutablePointer {
#[subdiagnostic]
pub frames: Vec<FrameNote>,
}

#[derive(Diagnostic)]
#[diag(const_eval_nullary_intrinsic_fail)]
pub struct NullaryIntrinsicError {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,8 @@ fn register_builtins(store: &mut LintStore) {
"byte_slice_in_packed_struct_with_derive",
"converted into hard error, see issue #107457 \
<https://github.com/rust-lang/rust/issues/107457> for more information",
)
);
store.register_removed("writes_through_immutable_pointer", "converted into hard error");
}

fn register_internals(store: &mut LintStore) {
Expand Down
35 changes: 0 additions & 35 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ declare_lint_pass! {
USELESS_DEPRECATED,
WARNINGS,
WASM_C_ABI,
WRITES_THROUGH_IMMUTABLE_POINTER,
// tidy-alphabetical-end
]
}
Expand Down Expand Up @@ -4696,40 +4695,6 @@ declare_lint! {
};
}

declare_lint! {
/// The `writes_through_immutable_pointer` lint detects writes through pointers derived from
/// shared references.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![feature(const_mut_refs)]
/// const WRITE_AFTER_CAST: () = unsafe {
/// let mut x = 0;
/// let ptr = &x as *const i32 as *mut i32;
/// *ptr = 0;
/// };
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Shared references are immutable (when there is no `UnsafeCell` involved),
/// and writing through them or through pointers derived from them is Undefined Behavior.
/// The compiler recently learned to detect such Undefined Behavior during compile-time
/// evaluation, and in the future this will raise a hard error.
///
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub WRITES_THROUGH_IMMUTABLE_POINTER,
Warn,
"shared references are immutable, and pointers derived from them must not be written to",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
reference: "issue #X <https://github.com/rust-lang/rust/issues/X>",
};
}

declare_lint! {
/// The `private_macro_use` lint detects private macros that are imported
/// with `#[macro_use]`.
Expand Down
9 changes: 4 additions & 5 deletions tests/ui/consts/const-eval/ub-write-through-immutable.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Ensure we catch UB due to writing through a shared reference.
#![feature(const_mut_refs, const_refs_to_cell)]
#![deny(writes_through_immutable_pointer)]
#![allow(invalid_reference_casting)]

use std::mem;
Expand All @@ -9,15 +8,15 @@ use std::cell::UnsafeCell;
const WRITE_AFTER_CAST: () = unsafe {
let mut x = 0;
let ptr = &x as *const i32 as *mut i32;
*ptr = 0; //~ERROR: writes_through_immutable_pointer
//~^ previously accepted
*ptr = 0; //~ERROR: evaluation of constant value failed
//~| immutable
};

const WRITE_AFTER_TRANSMUTE: () = unsafe {
let mut x = 0;
let ptr: *mut i32 = mem::transmute(&x);
*ptr = 0; //~ERROR: writes_through_immutable_pointer
//~^ previously accepted
*ptr = 0; //~ERROR: evaluation of constant value failed
//~| immutable
};

// it's okay when there is interior mutability;
Expand Down
54 changes: 7 additions & 47 deletions tests/ui/consts/const-eval/ub-write-through-immutable.stderr
Original file line number Diff line number Diff line change
@@ -1,55 +1,15 @@
error: writing through a pointer that was derived from a shared (immutable) reference
--> $DIR/ub-write-through-immutable.rs:12:5
error[E0080]: evaluation of constant value failed
--> $DIR/ub-write-through-immutable.rs:11:5
|
LL | *ptr = 0;
| ^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #X <https://github.com/rust-lang/rust/issues/X>
note: the lint level is defined here
--> $DIR/ub-write-through-immutable.rs:3:9
|
LL | #![deny(writes_through_immutable_pointer)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^ writing through a pointer that was derived from a shared (immutable) reference

error: writing through a pointer that was derived from a shared (immutable) reference
--> $DIR/ub-write-through-immutable.rs:19:5
error[E0080]: evaluation of constant value failed
--> $DIR/ub-write-through-immutable.rs:18:5
|
LL | *ptr = 0;
| ^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #X <https://github.com/rust-lang/rust/issues/X>
| ^^^^^^^^ writing through a pointer that was derived from a shared (immutable) reference

error: aborting due to 2 previous errors

Future incompatibility report: Future breakage diagnostic:
error: writing through a pointer that was derived from a shared (immutable) reference
--> $DIR/ub-write-through-immutable.rs:12:5
|
LL | *ptr = 0;
| ^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #X <https://github.com/rust-lang/rust/issues/X>
note: the lint level is defined here
--> $DIR/ub-write-through-immutable.rs:3:9
|
LL | #![deny(writes_through_immutable_pointer)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Future breakage diagnostic:
error: writing through a pointer that was derived from a shared (immutable) reference
--> $DIR/ub-write-through-immutable.rs:19:5
|
LL | *ptr = 0;
| ^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #X <https://github.com/rust-lang/rust/issues/X>
note: the lint level is defined here
--> $DIR/ub-write-through-immutable.rs:3:9
|
LL | #![deny(writes_through_immutable_pointer)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0080`.

0 comments on commit 2c70eb4

Please sign in to comment.