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 9317d46

Browse files
committed
turn errors that should be impossible due to our static checks into ICEs
1 parent bf24164 commit 9317d46

File tree

5 files changed

+55
-22
lines changed

5 files changed

+55
-22
lines changed

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

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use rustc_hir as hir;
2020
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
2121
use rustc_middle::mir::interpret::{ConstAllocation, CtfeProvenance, InterpResult};
2222
use rustc_middle::query::TyCtxtAt;
23+
use rustc_middle::span_bug;
2324
use rustc_middle::ty::layout::TyAndLayout;
2425
use rustc_span::def_id::LocalDefId;
2526
use rustc_span::sym;
@@ -223,49 +224,59 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
223224
continue;
224225
}
225226

226-
// Ensure that this is is derived from a shared reference. Crucially, we check this *before*
227+
// Ensure that this is derived from a shared reference. Crucially, we check this *before*
227228
// checking whether the `alloc_id` has already been interned. The point of this check is to
228229
// ensure that when there are multiple pointers to the same allocation, they are *all*
229230
// derived from a shared reference. Therefore it would be bad if we only checked the first
230231
// pointer to any given allocation.
231232
// (It is likely not possible to actually have multiple pointers to the same allocation,
232233
// 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>>`.
234+
// See <https://github.com/rust-lang/rust/pull/128543> for why we are checking for "shared
235+
// reference" and not "immutable", i.e., for why we are allowing interior-mutable shared
236+
// references: they can actually be created in safe code while pointing to apparently
237+
// "immutable" values, via promotion or tail expression lifetime extension of
238+
// `&None::<Cell<T>>`.
239+
// We also exclude promoteds from this as `&mut []` can be promoted, which is a mutable
240+
// reference pointing to an immutable (zero-sized) allocation. We rely on the promotion
241+
// analysis not screwing up to ensure that it is sound to intern promoteds as immutable.
237242
if intern_kind != InternKind::Promoted
238243
&& inner_mutability == Mutability::Not
239244
&& !prov.shared_ref()
240245
{
241-
if ecx.tcx.try_get_global_alloc(alloc_id).is_some()
242-
&& !just_interned.contains(&alloc_id)
243-
{
246+
let is_already_global = ecx.tcx.try_get_global_alloc(alloc_id).is_some();
247+
if is_already_global && !just_interned.contains(&alloc_id) {
244248
// This is a pointer to some memory from another constant. We encounter mutable
245249
// pointers to such memory since we do not always track immutability through
246250
// these "global" pointers. Allowing them is harmless; the point of these checks
247251
// during interning is to justify why we intern the *new* allocations immutably,
248-
// so we can completely ignore existing allocations. We also don't need to add
249-
// this to the todo list, since after all it is already interned.
252+
// so we can completely ignore existing allocations.
253+
// We can also skip the rest of this loop iteration, since after all it is already
254+
// interned.
250255
continue;
251256
}
252-
// Found a mutable reference inside a const where inner allocations should be
253-
// immutable. We exclude promoteds from this, since things like `&mut []` and
254-
// `&None::<Cell<i32>>` lead to promotion that can produce mutable pointers. We rely
255-
// on the promotion analysis not screwing up to ensure that it is sound to intern
256-
// promoteds as immutable.
257-
trace!("found bad mutable pointer");
258-
// Prefer dangling pointer errors over mutable pointer errors
259-
if result.is_ok() {
260-
result = Err(InternResult::FoundBadMutablePointer);
257+
// If this is a dangling pointer, that's actually fine -- the problematic case is
258+
// when there is memory there that someone might expect to be mutable, but we make it immutable.
259+
let dangling = !is_already_global && !ecx.memory.alloc_map.contains_key(&alloc_id);
260+
if !dangling {
261+
// Found a mutable reference inside a const where inner allocations should be
262+
// immutable.
263+
if !ecx.tcx.sess.opts.unstable_opts.unleash_the_miri_inside_of_you {
264+
span_bug!(
265+
ecx.tcx.span,
266+
"the static const safety checks accepted mutable references they should not have accepted"
267+
);
268+
}
269+
// Prefer dangling pointer errors over mutable pointer errors
270+
if result.is_ok() {
271+
result = Err(InternResult::FoundBadMutablePointer);
272+
}
261273
}
262274
}
263275
if ecx.tcx.try_get_global_alloc(alloc_id).is_some() {
264276
// Already interned.
265277
debug_assert!(!ecx.memory.alloc_map.contains_key(&alloc_id));
266278
continue;
267279
}
268-
just_interned.insert(alloc_id);
269280
// We always intern with `inner_mutability`, and furthermore we ensured above that if
270281
// that is "immutable", then there are *no* mutable pointers anywhere in the newly
271282
// interned memory -- justifying that we can indeed intern immutably. However this also
@@ -276,6 +287,7 @@ pub fn intern_const_alloc_recursive<'tcx, M: CompileTimeMachine<'tcx, const_eval
276287
// pointers before deciding which allocations can be made immutable; but for now we are
277288
// okay with losing some potential for immutability here. This can anyway only affect
278289
// `static mut`.
290+
just_interned.insert(alloc_id);
279291
match intern_shallow(ecx, alloc_id, inner_mutability) {
280292
Ok(nested) => todo.extend(nested),
281293
Err(()) => {

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,14 @@ use hir::def::DefKind;
1313
use rustc_ast::Mutability;
1414
use rustc_data_structures::fx::FxHashSet;
1515
use rustc_hir as hir;
16-
use rustc_middle::bug;
1716
use rustc_middle::mir::interpret::ValidationErrorKind::{self, *};
1817
use rustc_middle::mir::interpret::{
1918
ExpectedKind, InterpError, InvalidMetaKind, Misalignment, PointerKind, Provenance,
2019
UnsupportedOpInfo, ValidationErrorInfo,
2120
};
2221
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
2322
use rustc_middle::ty::{self, Ty};
23+
use rustc_middle::{bug, span_bug};
2424
use rustc_span::symbol::{sym, Symbol};
2525
use rustc_target::abi::{
2626
Abi, FieldIdx, Scalar as ScalarAbi, Size, VariantIdx, Variants, WrappingRange,
@@ -519,6 +519,13 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
519519
if ptr_expected_mutbl == Mutability::Mut
520520
&& alloc_actual_mutbl == Mutability::Not
521521
{
522+
if !self.ecx.tcx.sess.opts.unstable_opts.unleash_the_miri_inside_of_you
523+
{
524+
span_bug!(
525+
self.ecx.tcx.span,
526+
"the static const safety checks accepted mutable references they should not have accepted"
527+
);
528+
}
522529
throw_validation_failure!(self.path, MutableRefToImmutable);
523530
}
524531
// In a const, everything must be completely immutable.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Version of `tests/ui/consts/const-eval/heap/alloc_intrinsic_untyped.rs` without the flag that
2+
// suppresses the ICE.
3+
//@ known-bug: #129233
4+
#![feature(core_intrinsics)]
5+
#![feature(const_heap)]
6+
#![feature(const_mut_refs)]
7+
use std::intrinsics;
8+
9+
const BAR: *mut i32 = unsafe { intrinsics::const_allocate(4, 4) as *mut i32 };
10+
11+
fn main() {}

‎tests/ui/consts/const-eval/heap/alloc_intrinsic_untyped.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// We unleash Miri here since this test demonstrates code that bypasses the checks against interning
2+
// mutable pointers, which currently ICEs. Unleashing Miri silence the ICE.
3+
//@ compile-flags: -Zunleash-the-miri-inside-of-you
14
#![feature(core_intrinsics)]
25
#![feature(const_heap)]
36
#![feature(const_mut_refs)]

‎tests/ui/consts/const-eval/heap/alloc_intrinsic_untyped.stderr‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: encountered mutable pointer in final value of constant
2-
--> $DIR/alloc_intrinsic_untyped.rs:6:1
2+
--> $DIR/alloc_intrinsic_untyped.rs:9:1
33
|
44
LL | const BAR: *mut i32 = unsafe { intrinsics::const_allocate(4, 4) as *mut i32 };
55
| ^^^^^^^^^^^^^^^^^^^

0 commit comments

Comments
(0)

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