Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 9b3e7d8

Browse files
committed
const-eval interning: accpt interior mutable pointers in final value (but keep rejecting mutable references)
1 parent 355a307 commit 9b3e7d8

20 files changed

+175
-562
lines changed

‎compiler/rustc_const_eval/src/const_eval/eval_queries.rs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,19 @@ use rustc_middle::traits::Reveal;
1010
use rustc_middle::ty::layout::LayoutOf;
1111
use rustc_middle::ty::print::with_no_trimmed_paths;
1212
use rustc_middle::ty::{self, Ty, TyCtxt};
13-
use rustc_session::lint;
1413
use rustc_span::def_id::LocalDefId;
1514
use rustc_span::{Span, DUMMY_SP};
1615
use rustc_target::abi::{self, Abi};
1716
use tracing::{debug, instrument, trace};
1817

1918
use super::{CanAccessMutGlobal, CompileTimeInterpCx, CompileTimeMachine};
2019
use crate::const_eval::CheckAlignment;
21-
use crate::errors::{self, ConstEvalError, DanglingPtrInFinal};
2220
use crate::interpret::{
2321
create_static_alloc, eval_nullary_intrinsic, intern_const_alloc_recursive, throw_exhaust,
2422
CtfeValidationMode, GlobalId, Immediate, InternKind, InternResult, InterpCx, InterpError,
2523
InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking, StackPopCleanup,
2624
};
27-
use crate::CTRL_C_RECEIVED;
25+
use crate::{errors,CTRL_C_RECEIVED};
2826

2927
// Returns a pointer to where the result lives
3028
#[instrument(level = "trace", skip(ecx, body))]
@@ -105,18 +103,15 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>(
105103
return Err(ecx
106104
.tcx
107105
.dcx()
108-
.emit_err(DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind })
106+
.emit_err(errors::DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind })
109107
.into());
110108
}
111109
Err(InternResult::FoundBadMutablePointer) => {
112-
// only report mutable pointers if there were no dangling pointers
113-
let err_diag = errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind };
114-
ecx.tcx.emit_node_span_lint(
115-
lint::builtin::CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE,
116-
ecx.machine.best_lint_scope(*ecx.tcx),
117-
err_diag.span,
118-
err_diag,
119-
)
110+
return Err(ecx
111+
.tcx
112+
.dcx()
113+
.emit_err(errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind })
114+
.into());
120115
}
121116
}
122117

@@ -448,7 +443,12 @@ fn report_eval_error<'tcx>(
448443
error,
449444
DUMMY_SP,
450445
|| super::get_span_and_frames(ecx.tcx, ecx.stack()),
451-
|span, frames| ConstEvalError { span, error_kind: kind, instance, frame_notes: frames },
446+
|span, frames| errors::ConstEvalError {
447+
span,
448+
error_kind: kind,
449+
instance,
450+
frame_notes: frames,
451+
},
452452
)
453453
}
454454

‎compiler/rustc_const_eval/src/const_eval/machine.rs‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -712,16 +712,29 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
712712
_kind: mir::RetagKind,
713713
val: &ImmTy<'tcx, CtfeProvenance>,
714714
) -> InterpResult<'tcx, ImmTy<'tcx, CtfeProvenance>> {
715-
// If it's a frozen shared reference that's not already immutable, make it immutable.
715+
// If it's a frozen shared reference that's not already immutable, potentially make it immutable.
716716
// (Do nothing on `None` provenance, that cannot store immutability anyway.)
717717
if let ty::Ref(_, ty, mutbl) = val.layout.ty.kind()
718718
&& *mutbl == Mutability::Not
719-
&& val.to_scalar_and_meta().0.to_pointer(ecx)?.provenance.is_some_and(|p| !p.immutable())
720-
// That next check is expensive, that's why we have all the guards above.
721-
&& ty.is_freeze(*ecx.tcx, ecx.param_env)
719+
&& val
720+
.to_scalar_and_meta()
721+
.0
722+
.to_pointer(ecx)?
723+
.provenance
724+
.is_some_and(|p| !p.immutable())
722725
{
726+
// That next check is expensive, that's why we have all the guards above.
727+
let is_immutable = ty.is_freeze(*ecx.tcx, ecx.param_env);
723728
let place = ecx.ref_to_mplace(val)?;
724-
let new_place = place.map_provenance(CtfeProvenance::as_immutable);
729+
let new_place = if is_immutable {
730+
place.map_provenance(CtfeProvenance::as_immutable)
731+
} else {
732+
// Even if it is not immutable, remember that it is a shared reference.
733+
// This allows it to become part of the final value of the constant.
734+
// (See <https://github.com/rust-lang/rust/pull/128543> for why we allow this
735+
// even when there is interior mutability.)
736+
place.map_provenance(CtfeProvenance::as_shared_ref)
737+
};
725738
Ok(ImmTy::from_immediate(new_place.to_ref(ecx), val.layout))
726739
} else {
727740
Ok(val.clone())

‎compiler/rustc_const_eval/src/errors.rs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,10 @@ pub(crate) struct NestedStaticInThreadLocal {
3535
pub span: Span,
3636
}
3737

38-
#[derive(LintDiagnostic)]
38+
#[derive(Diagnostic)]
3939
#[diag(const_eval_mutable_ptr_in_final)]
4040
pub(crate) struct MutablePtrInFinal {
41-
// rust-lang/rust#122153: This was marked as `#[primary_span]` under
42-
// `derive(Diagnostic)`. Since we expect we may hard-error in future, we are
43-
// keeping the field (and skipping it under `derive(LintDiagnostic)`).
44-
#[skip_arg]
41+
#[primary_span]
4542
pub span: Span,
4643
pub kind: InternKind,
4744
}

‎compiler/rustc_const_eval/src/interpret/intern.rs‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -223,16 +223,20 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
223223
continue;
224224
}
225225

226-
// Crucially, we check this *before* checking whether the `alloc_id`
227-
// has already been interned. The point of this check is to ensure that when
228-
// there are multiple pointers to the same allocation, they are *all* immutable.
229-
// Therefore it would be bad if we only checked the first pointer to any given
230-
// allocation.
226+
// Ensure that this is is derived from a shared reference. Crucially, we check this *before*
227+
// checking whether the `alloc_id` has already been interned. The point of this check is to
228+
// ensure that when there are multiple pointers to the same allocation, they are *all*
229+
// derived from a shared reference. Therefore it would be bad if we only checked the first
230+
// pointer to any given allocation.
231231
// (It is likely not possible to actually have multiple pointers to the same allocation,
232232
// so alternatively we could also check that and ICE if there are multiple such pointers.)
233+
// See <https://github.com/rust-lang/rust/pull/128543> for why we are checking for
234+
// "shared reference" and not "immutable", i.e., for why we are allowed interior-mutable
235+
// shared references: they can actually be created in safe code while pointing to apparently
236+
// "immutable" values, via promotion of `&None::<Cell<T>>`.
233237
if intern_kind != InternKind::Promoted
234238
&& inner_mutability == Mutability::Not
235-
&& !prov.immutable()
239+
&& !prov.shared_ref()
236240
{
237241
if ecx.tcx.try_get_global_alloc(alloc_id).is_some()
238242
&& !just_interned.contains(&alloc_id)
@@ -245,7 +249,7 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
245249
// this to the todo list, since after all it is already interned.
246250
continue;
247251
}
248-
// Found a mutable pointer inside a const where inner allocations should be
252+
// Found a mutable reference inside a const where inner allocations should be
249253
// immutable. We exclude promoteds from this, since things like `&mut []` and
250254
// `&None::<Cell<i32>>` lead to promotion that can produce mutable pointers. We rely
251255
// on the promotion analysis not screwing up to ensure that it is sound to intern

‎compiler/rustc_lint/src/lib.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,11 @@ fn register_builtins(store: &mut LintStore) {
569569
"byte_slice_in_packed_struct_with_derive",
570570
"converted into hard error, see issue #107457 \
571571
<https://github.com/rust-lang/rust/issues/107457> for more information",
572-
)
572+
);
573+
store.register_removed(
574+
"const_eval_mutable_ptr_in_final_value",
575+
"partially allowed now, otherwise turned into a hard error",
576+
);
573577
}
574578

575579
fn register_internals(store: &mut LintStore) {

‎compiler/rustc_lint_defs/src/builtin.rs‎

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ declare_lint_pass! {
2929
CENUM_IMPL_DROP_CAST,
3030
COHERENCE_LEAK_CHECK,
3131
CONFLICTING_REPR_HINTS,
32-
CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE,
3332
CONST_EVALUATABLE_UNCHECKED,
3433
CONST_ITEM_MUTATION,
3534
DEAD_CODE,
@@ -2772,51 +2771,6 @@ declare_lint! {
27722771
@feature_gate = strict_provenance;
27732772
}
27742773

2775-
declare_lint! {
2776-
/// The `const_eval_mutable_ptr_in_final_value` lint detects if a mutable pointer
2777-
/// has leaked into the final value of a const expression.
2778-
///
2779-
/// ### Example
2780-
///
2781-
/// ```rust
2782-
/// pub enum JsValue {
2783-
/// Undefined,
2784-
/// Object(std::cell::Cell<bool>),
2785-
/// }
2786-
///
2787-
/// impl ::std::ops::Drop for JsValue {
2788-
/// fn drop(&mut self) {}
2789-
/// }
2790-
///
2791-
/// const UNDEFINED: &JsValue = &JsValue::Undefined;
2792-
///
2793-
/// fn main() {
2794-
/// }
2795-
/// ```
2796-
///
2797-
/// {{produces}}
2798-
///
2799-
/// ### Explanation
2800-
///
2801-
/// In the 1.77 release, the const evaluation machinery adopted some
2802-
/// stricter rules to reject expressions with values that could
2803-
/// end up holding mutable references to state stored in static memory
2804-
/// (which is inherently immutable).
2805-
///
2806-
/// This is a [future-incompatible] lint to ease the transition to an error.
2807-
/// See [issue #122153] for more details.
2808-
///
2809-
/// [issue #122153]: https://github.com/rust-lang/rust/issues/122153
2810-
/// [future-incompatible]: ../index.md#future-incompatible-lints
2811-
pub CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE,
2812-
Warn,
2813-
"detects a mutable pointer that has leaked into final value of a const expression",
2814-
@future_incompatible = FutureIncompatibleInfo {
2815-
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
2816-
reference: "issue #122153 <https://github.com/rust-lang/rust/issues/122153>",
2817-
};
2818-
}
2819-
28202774
declare_lint! {
28212775
/// The `const_evaluatable_unchecked` lint detects a generic constant used
28222776
/// in a type.

‎compiler/rustc_middle/src/mir/interpret/pointer.rs‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,23 @@ pub trait Provenance: Copy + fmt::Debug + 'static {
8080
}
8181

8282
/// The type of provenance in the compile-time interpreter.
83-
/// This is a packed representation of an `AllocId` and an `immutable: bool`.
83+
/// This is a packed representation of:
84+
/// - an `AllocId` (non-zero)
85+
/// - an `immutable: bool`
86+
/// - a `shared_ref: bool`
87+
///
88+
/// with the extra invariant that if `immutable` is `true`, then so
89+
/// is `shared_ref`.
8490
#[derive(Copy, Clone, Eq, Hash, Ord, PartialEq, PartialOrd)]
8591
pub struct CtfeProvenance(NonZero<u64>);
8692

8793
impl From<AllocId> for CtfeProvenance {
8894
fn from(value: AllocId) -> Self {
8995
let prov = CtfeProvenance(value.0);
90-
assert!(!prov.immutable(), "`AllocId` with the highest bit set cannot be used in CTFE");
96+
assert!(
97+
prov.alloc_id() == value,
98+
"`AllocId` with the highest bits set cannot be used in CTFE"
99+
);
91100
prov
92101
}
93102
}
@@ -103,12 +112,14 @@ impl fmt::Debug for CtfeProvenance {
103112
}
104113

105114
const IMMUTABLE_MASK: u64 = 1 << 63; // the highest bit
115+
const SHARED_REF_MASK: u64 = 1 << 62;
116+
const ALLOC_ID_MASK: u64 = u64::MAX & !IMMUTABLE_MASK & !SHARED_REF_MASK;
106117

107118
impl CtfeProvenance {
108119
/// Returns the `AllocId` of this provenance.
109120
#[inline(always)]
110121
pub fn alloc_id(self) -> AllocId {
111-
AllocId(NonZero::new(self.0.get() & !IMMUTABLE_MASK).unwrap())
122+
AllocId(NonZero::new(self.0.get() & ALLOC_ID_MASK).unwrap())
112123
}
113124

114125
/// Returns whether this provenance is immutable.
@@ -117,10 +128,38 @@ impl CtfeProvenance {
117128
self.0.get() & IMMUTABLE_MASK != 0
118129
}
119130

131+
/// Returns whether this provenance is derived from a shared reference.
132+
#[inline]
133+
pub fn shared_ref(self) -> bool {
134+
self.0.get() & SHARED_REF_MASK != 0
135+
}
136+
137+
pub fn into_parts(self) -> (AllocId, bool, bool) {
138+
(self.alloc_id(), self.immutable(), self.shared_ref())
139+
}
140+
141+
pub fn from_parts((alloc_id, immutable, shared_ref): (AllocId, bool, bool)) -> Self {
142+
let prov = CtfeProvenance::from(alloc_id);
143+
if immutable {
144+
// This sets both flas, so we don't even have to check `shared_ref`.
145+
prov.as_immutable()
146+
} else if shared_ref {
147+
prov.as_shared_ref()
148+
} else {
149+
prov
150+
}
151+
}
152+
120153
/// Returns an immutable version of this provenance.
121154
#[inline]
122155
pub fn as_immutable(self) -> Self {
123-
CtfeProvenance(self.0 | IMMUTABLE_MASK)
156+
CtfeProvenance(self.0 | IMMUTABLE_MASK | SHARED_REF_MASK)
157+
}
158+
159+
/// Returns a "shared reference" (but not necessarily immutable!) version of this provenance.
160+
#[inline]
161+
pub fn as_shared_ref(self) -> Self {
162+
CtfeProvenance(self.0 | SHARED_REF_MASK)
124163
}
125164
}
126165

‎compiler/rustc_middle/src/ty/codec.rs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,7 @@ impl<'tcx, E: TyEncoder<I = TyCtxt<'tcx>>> Encodable<E> for AllocId {
165165

166166
impl<'tcx, E: TyEncoder<I = TyCtxt<'tcx>>> Encodable<E> for CtfeProvenance {
167167
fn encode(&self, e: &mut E) {
168-
self.alloc_id().encode(e);
169-
self.immutable().encode(e);
168+
self.into_parts().encode(e);
170169
}
171170
}
172171

@@ -295,10 +294,8 @@ impl<'tcx, D: TyDecoder<I = TyCtxt<'tcx>>> Decodable<D> for AllocId {
295294

296295
impl<'tcx, D: TyDecoder<I = TyCtxt<'tcx>>> Decodable<D> for CtfeProvenance {
297296
fn decode(decoder: &mut D) -> Self {
298-
let alloc_id: AllocId = Decodable::decode(decoder);
299-
let prov = CtfeProvenance::from(alloc_id);
300-
let immutable: bool = Decodable::decode(decoder);
301-
if immutable { prov.as_immutable() } else { prov }
297+
let parts = Decodable::decode(decoder);
298+
CtfeProvenance::from_parts(parts)
302299
}
303300
}
304301

‎compiler/rustc_middle/src/ty/impls_ty.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,9 @@ impl<'a> HashStable<StableHashingContext<'a>> for mir::interpret::AllocId {
7575
}
7676
}
7777

78-
// CtfeProvenance is an AllocId and a bool.
7978
impl<'a> HashStable<StableHashingContext<'a>> for mir::interpret::CtfeProvenance {
8079
fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
81-
self.alloc_id().hash_stable(hcx, hasher);
82-
self.immutable().hash_stable(hcx, hasher);
80+
self.into_parts().hash_stable(hcx, hasher);
8381
}
8482
}
8583

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
#![feature(core_intrinsics)]
22
#![feature(const_heap)]
33
#![feature(const_mut_refs)]
4-
#![deny(const_eval_mutable_ptr_in_final_value)]
54
use std::intrinsics;
65

76
const BAR: *mut i32 = unsafe { intrinsics::const_allocate(4, 4) as *mut i32 };
87
//~^ error: mutable pointer in final value of constant
9-
//~| WARNING this was previously accepted by the compiler
108

119
fn main() {}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /