diff options
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_const_eval/src/check_consts/check.rs | 5 | ||||
| -rw-r--r-- | compiler/rustc_const_eval/src/const_eval/eval_queries.rs | 26 | ||||
| -rw-r--r-- | compiler/rustc_const_eval/src/const_eval/machine.rs | 23 | ||||
| -rw-r--r-- | compiler/rustc_const_eval/src/errors.rs | 7 | ||||
| -rw-r--r-- | compiler/rustc_const_eval/src/interpret/intern.rs | 18 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/lib.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_lint_defs/src/builtin.rs | 46 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/mir/interpret/pointer.rs | 47 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/ty/codec.rs | 9 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/ty/impls_ty.rs | 4 |
10 files changed, 98 insertions, 91 deletions
diff --git a/compiler/rustc_const_eval/src/check_consts/check.rs b/compiler/rustc_const_eval/src/check_consts/check.rs index 8be327a8b56..828d8354c63 100644 --- a/compiler/rustc_const_eval/src/check_consts/check.rs +++ b/compiler/rustc_const_eval/src/check_consts/check.rs @@ -538,8 +538,9 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { // final value. // Note: This is only sound if every local that has a `StorageDead` has a // `StorageDead` in every control flow path leading to a `return` terminator. - // The good news is that interning will detect if any unexpected mutable - // pointer slips through. + // If anything slips through, there's no safety net -- safe code can create + // references to variants of `!Freeze` enums as long as that variant is `Freeze`, + // so interning can't protect us here. if self.local_is_transient(place.local) { self.check_op(ops::TransientCellBorrow); } else { diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index 7ccebd83f24..594b98f34ef 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -10,7 +10,6 @@ use rustc_middle::traits::Reveal; use rustc_middle::ty::layout::LayoutOf; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{self, Ty, TyCtxt}; -use rustc_session::lint; use rustc_span::def_id::LocalDefId; use rustc_span::{Span, DUMMY_SP}; use rustc_target::abi::{self, Abi}; @@ -18,13 +17,12 @@ use tracing::{debug, instrument, trace}; use super::{CanAccessMutGlobal, CompileTimeInterpCx, CompileTimeMachine}; use crate::const_eval::CheckAlignment; -use crate::errors::{self, ConstEvalError, DanglingPtrInFinal}; use crate::interpret::{ create_static_alloc, eval_nullary_intrinsic, intern_const_alloc_recursive, throw_exhaust, CtfeValidationMode, GlobalId, Immediate, InternKind, InternResult, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking, StackPopCleanup, }; -use crate::CTRL_C_RECEIVED; +use crate::{errors, CTRL_C_RECEIVED}; // Returns a pointer to where the result lives #[instrument(level = "trace", skip(ecx, body))] @@ -105,18 +103,15 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>( return Err(ecx .tcx .dcx() - .emit_err(DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind }) + .emit_err(errors::DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind }) .into()); } Err(InternResult::FoundBadMutablePointer) => { - // only report mutable pointers if there were no dangling pointers - let err_diag = errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind }; - ecx.tcx.emit_node_span_lint( - lint::builtin::CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE, - ecx.machine.best_lint_scope(*ecx.tcx), - err_diag.span, - err_diag, - ) + return Err(ecx + .tcx + .dcx() + .emit_err(errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind }) + .into()); } } @@ -448,7 +443,12 @@ fn report_eval_error<'tcx>( error, DUMMY_SP, || super::get_span_and_frames(ecx.tcx, ecx.stack()), - |span, frames| ConstEvalError { span, error_kind: kind, instance, frame_notes: frames }, + |span, frames| errors::ConstEvalError { + span, + error_kind: kind, + instance, + frame_notes: frames, + }, ) } diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index 7405ca09342..6a691abae02 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -718,16 +718,29 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> { _kind: mir::RetagKind, val: &ImmTy<'tcx, CtfeProvenance>, ) -> InterpResult<'tcx, ImmTy<'tcx, CtfeProvenance>> { - // If it's a frozen shared reference that's not already immutable, make it immutable. + // If it's a frozen shared reference that's not already immutable, potentially make it immutable. // (Do nothing on `None` provenance, that cannot store immutability anyway.) if let ty::Ref(_, ty, mutbl) = val.layout.ty.kind() && *mutbl == Mutability::Not - && val.to_scalar_and_meta().0.to_pointer(ecx)?.provenance.is_some_and(|p| !p.immutable()) - // That next check is expensive, that's why we have all the guards above. - && ty.is_freeze(*ecx.tcx, ecx.param_env) + && val + .to_scalar_and_meta() + .0 + .to_pointer(ecx)? + .provenance + .is_some_and(|p| !p.immutable()) { + // That next check is expensive, that's why we have all the guards above. + let is_immutable = ty.is_freeze(*ecx.tcx, ecx.param_env); let place = ecx.ref_to_mplace(val)?; - let new_place = place.map_provenance(CtfeProvenance::as_immutable); + let new_place = if is_immutable { + place.map_provenance(CtfeProvenance::as_immutable) + } else { + // Even if it is not immutable, remember that it is a shared reference. + // This allows it to become part of the final value of the constant. + // (See <https://github.com/rust-lang/rust/pull/128543> for why we allow this + // even when there is interior mutability.) + place.map_provenance(CtfeProvenance::as_shared_ref) + }; Ok(ImmTy::from_immediate(new_place.to_ref(ecx), val.layout)) } else { Ok(val.clone()) diff --git a/compiler/rustc_const_eval/src/errors.rs b/compiler/rustc_const_eval/src/errors.rs index 0b366b43f95..57e52254757 100644 --- a/compiler/rustc_const_eval/src/errors.rs +++ b/compiler/rustc_const_eval/src/errors.rs @@ -35,13 +35,10 @@ pub(crate) struct NestedStaticInThreadLocal { pub span: Span, } -#[derive(LintDiagnostic)] +#[derive(Diagnostic)] #[diag(const_eval_mutable_ptr_in_final)] pub(crate) struct MutablePtrInFinal { - // rust-lang/rust#122153: This was marked as `#[primary_span]` under - // `derive(Diagnostic)`. Since we expect we may hard-error in future, we are - // keeping the field (and skipping it under `derive(LintDiagnostic)`). - #[skip_arg] + #[primary_span] pub span: Span, pub kind: InternKind, } diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs index 8b0a2afa4d6..e81431b45fb 100644 --- a/compiler/rustc_const_eval/src/interpret/intern.rs +++ b/compiler/rustc_const_eval/src/interpret/intern.rs @@ -223,16 +223,20 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval continue; } - // Crucially, we check this *before* checking whether the `alloc_id` - // has already been interned. The point of this check is to ensure that when - // there are multiple pointers to the same allocation, they are *all* immutable. - // Therefore it would be bad if we only checked the first pointer to any given - // allocation. + // Ensure that this is is derived from a shared reference. Crucially, we check this *before* + // checking whether the `alloc_id` has already been interned. The point of this check is to + // ensure that when there are multiple pointers to the same allocation, they are *all* + // derived from a shared reference. Therefore it would be bad if we only checked the first + // pointer to any given allocation. // (It is likely not possible to actually have multiple pointers to the same allocation, // so alternatively we could also check that and ICE if there are multiple such pointers.) + // See <https://github.com/rust-lang/rust/pull/128543> for why we are checking for + // "shared reference" and not "immutable", i.e., for why we are allowed interior-mutable + // shared references: they can actually be created in safe code while pointing to apparently + // "immutable" values, via promotion of `&None::<Cell<T>>`. if intern_kind != InternKind::Promoted && inner_mutability == Mutability::Not - && !prov.immutable() + && !prov.shared_ref() { if ecx.tcx.try_get_global_alloc(alloc_id).is_some() && !just_interned.contains(&alloc_id) @@ -245,7 +249,7 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval // this to the todo list, since after all it is already interned. continue; } - // Found a mutable pointer inside a const where inner allocations should be + // Found a mutable reference inside a const where inner allocations should be // immutable. We exclude promoteds from this, since things like `&mut []` and // `&None::<Cell<i32>>` lead to promotion that can produce mutable pointers. We rely // on the promotion analysis not screwing up to ensure that it is sound to intern diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index a9627e9b789..4139ac4e3e9 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -576,6 +576,10 @@ fn register_builtins(store: &mut LintStore) { <https://github.com/rust-lang/rust/issues/107457> for more information", ); store.register_removed("writes_through_immutable_pointer", "converted into hard error"); + store.register_removed( + "const_eval_mutable_ptr_in_final_value", + "partially allowed now, otherwise turned into a hard error", + ); } fn register_internals(store: &mut LintStore) { diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 25d33126754..f42f35c594b 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -29,7 +29,6 @@ declare_lint_pass! { CENUM_IMPL_DROP_CAST, COHERENCE_LEAK_CHECK, CONFLICTING_REPR_HINTS, - CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE, CONST_EVALUATABLE_UNCHECKED, CONST_ITEM_MUTATION, DEAD_CODE, @@ -2805,51 +2804,6 @@ declare_lint! { } declare_lint! { - /// The `const_eval_mutable_ptr_in_final_value` lint detects if a mutable pointer - /// has leaked into the final value of a const expression. - /// - /// ### Example - /// - /// ```rust - /// pub enum JsValue { - /// Undefined, - /// Object(std::cell::Cell<bool>), - /// } - /// - /// impl ::std::ops::Drop for JsValue { - /// fn drop(&mut self) {} - /// } - /// - /// const UNDEFINED: &JsValue = &JsValue::Undefined; - /// - /// fn main() { - /// } - /// ``` - /// - /// {{produces}} - /// - /// ### Explanation - /// - /// In the 1.77 release, the const evaluation machinery adopted some - /// stricter rules to reject expressions with values that could - /// end up holding mutable references to state stored in static memory - /// (which is inherently immutable). - /// - /// This is a [future-incompatible] lint to ease the transition to an error. - /// See [issue #122153] for more details. - /// - /// [issue #122153]: https://github.com/rust-lang/rust/issues/122153 - /// [future-incompatible]: ../index.md#future-incompatible-lints - pub CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE, - Warn, - "detects a mutable pointer that has leaked into final value of a const expression", - @future_incompatible = FutureIncompatibleInfo { - reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps, - reference: "issue #122153 <https://github.com/rust-lang/rust/issues/122153>", - }; -} - -declare_lint! { /// The `const_evaluatable_unchecked` lint detects a generic constant used /// in a type. /// diff --git a/compiler/rustc_middle/src/mir/interpret/pointer.rs b/compiler/rustc_middle/src/mir/interpret/pointer.rs index 6cfd07d699c..1700b0f02ec 100644 --- a/compiler/rustc_middle/src/mir/interpret/pointer.rs +++ b/compiler/rustc_middle/src/mir/interpret/pointer.rs @@ -80,14 +80,23 @@ pub trait Provenance: Copy + fmt::Debug + 'static { } /// The type of provenance in the compile-time interpreter. -/// This is a packed representation of an `AllocId` and an `immutable: bool`. +/// This is a packed representation of: +/// - an `AllocId` (non-zero) +/// - an `immutable: bool` +/// - a `shared_ref: bool` +/// +/// with the extra invariant that if `immutable` is `true`, then so +/// is `shared_ref`. #[derive(Copy, Clone, Eq, Hash, Ord, PartialEq, PartialOrd)] pub struct CtfeProvenance(NonZero<u64>); impl From<AllocId> for CtfeProvenance { fn from(value: AllocId) -> Self { let prov = CtfeProvenance(value.0); - assert!(!prov.immutable(), "`AllocId` with the highest bit set cannot be used in CTFE"); + assert!( + prov.alloc_id() == value, + "`AllocId` with the highest bits set cannot be used in CTFE" + ); prov } } @@ -103,12 +112,14 @@ impl fmt::Debug for CtfeProvenance { } const IMMUTABLE_MASK: u64 = 1 << 63; // the highest bit +const SHARED_REF_MASK: u64 = 1 << 62; +const ALLOC_ID_MASK: u64 = u64::MAX & !IMMUTABLE_MASK & !SHARED_REF_MASK; impl CtfeProvenance { /// Returns the `AllocId` of this provenance. #[inline(always)] pub fn alloc_id(self) -> AllocId { - AllocId(NonZero::new(self.0.get() & !IMMUTABLE_MASK).unwrap()) + AllocId(NonZero::new(self.0.get() & ALLOC_ID_MASK).unwrap()) } /// Returns whether this provenance is immutable. @@ -117,10 +128,38 @@ impl CtfeProvenance { self.0.get() & IMMUTABLE_MASK != 0 } + /// Returns whether this provenance is derived from a shared reference. + #[inline] + pub fn shared_ref(self) -> bool { + self.0.get() & SHARED_REF_MASK != 0 + } + + pub fn into_parts(self) -> (AllocId, bool, bool) { + (self.alloc_id(), self.immutable(), self.shared_ref()) + } + + pub fn from_parts((alloc_id, immutable, shared_ref): (AllocId, bool, bool)) -> Self { + let prov = CtfeProvenance::from(alloc_id); + if immutable { + // This sets both flags, so we don't even have to check `shared_ref`. + prov.as_immutable() + } else if shared_ref { + prov.as_shared_ref() + } else { + prov + } + } + /// Returns an immutable version of this provenance. #[inline] pub fn as_immutable(self) -> Self { - CtfeProvenance(self.0 | IMMUTABLE_MASK) + CtfeProvenance(self.0 | IMMUTABLE_MASK | SHARED_REF_MASK) + } + + /// Returns a "shared reference" (but not necessarily immutable!) version of this provenance. + #[inline] + pub fn as_shared_ref(self) -> Self { + CtfeProvenance(self.0 | SHARED_REF_MASK) } } diff --git a/compiler/rustc_middle/src/ty/codec.rs b/compiler/rustc_middle/src/ty/codec.rs index 46203ee150f..7e533bc4291 100644 --- a/compiler/rustc_middle/src/ty/codec.rs +++ b/compiler/rustc_middle/src/ty/codec.rs @@ -165,8 +165,7 @@ impl<'tcx, E: TyEncoder<I = TyCtxt<'tcx>>> Encodable<E> for AllocId { impl<'tcx, E: TyEncoder<I = TyCtxt<'tcx>>> Encodable<E> for CtfeProvenance { fn encode(&self, e: &mut E) { - self.alloc_id().encode(e); - self.immutable().encode(e); + self.into_parts().encode(e); } } @@ -295,10 +294,8 @@ impl<'tcx, D: TyDecoder<I = TyCtxt<'tcx>>> Decodable<D> for AllocId { impl<'tcx, D: TyDecoder<I = TyCtxt<'tcx>>> Decodable<D> for CtfeProvenance { fn decode(decoder: &mut D) -> Self { - let alloc_id: AllocId = Decodable::decode(decoder); - let prov = CtfeProvenance::from(alloc_id); - let immutable: bool = Decodable::decode(decoder); - if immutable { prov.as_immutable() } else { prov } + let parts = Decodable::decode(decoder); + CtfeProvenance::from_parts(parts) } } diff --git a/compiler/rustc_middle/src/ty/impls_ty.rs b/compiler/rustc_middle/src/ty/impls_ty.rs index b5b7b8bcfef..5f6305bb48a 100644 --- a/compiler/rustc_middle/src/ty/impls_ty.rs +++ b/compiler/rustc_middle/src/ty/impls_ty.rs @@ -75,11 +75,9 @@ impl<'a> HashStable<StableHashingContext<'a>> for mir::interpret::AllocId { } } -// CtfeProvenance is an AllocId and a bool. impl<'a> HashStable<StableHashingContext<'a>> for mir::interpret::CtfeProvenance { fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { - self.alloc_id().hash_stable(hcx, hasher); - self.immutable().hash_stable(hcx, hasher); + self.into_parts().hash_stable(hcx, hasher); } } |