Skip to content

Commit

Permalink
Auto merge of rust-lang#116564 - oli-obk:evaluated_static_in_metadata…
Browse files Browse the repository at this point in the history
…, r=<try>

Store static initializers in metadata instead of the MIR of statics.

This means that adding generic statics would be even more difficult, as we can't evaluate statics from other crates anymore, but the subtle issue I have encountered make me think that having this be an explicit problem is better.

The issue is that

```rust
static mut FOO: &mut u32 = &mut 42;
static mut BAR = unsafe { FOO };
```

gets different allocations, instead of referring to the same one. This is also true for non-static mut, but promotion makes `static FOO: &u32 = &42;` annoying to demo.

## Why is this being done?

In order to ensure all crates see the same nested allocations (which is the last issue that needs fixing before we can stabilize [`const_mut_refs`](rust-lang#57349)), I am working on creating anonymous (from the Rust side, to LLVM it's like a regular static item) static items for the nested allocations in a static. If we evaluate the static item in a downstream crate again, we will end up duplicating its nested allocations (and in some cases, like the `match` case, even duplicate the main allocation).
  • Loading branch information
bors committed Feb 11, 2024
2 parents 9aa232e + 2178136 commit cc542ae
Show file tree
Hide file tree
Showing 49 changed files with 502 additions and 214 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_const_eval/messages.ftl
Expand Up @@ -309,6 +309,8 @@ const_eval_realloc_or_alloc_with_offset =
*[other] {""}
} {$ptr} which does not point to the beginning of an object
const_eval_recursive_static = encountered static that tried to initialize itself with itself
const_eval_remainder_by_zero =
calculating the remainder with a divisor of zero
const_eval_remainder_overflow =
Expand Down
62 changes: 45 additions & 17 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Expand Up @@ -2,12 +2,12 @@ use either::{Left, Right};

use rustc_hir::def::DefKind;
use rustc_middle::mir::interpret::{AllocId, ErrorHandled, InterpErrorInfo};
use rustc_middle::mir::pretty::write_allocation_bytes;
use rustc_middle::mir::{self, ConstAlloc, ConstValue};
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, TyCtxt};
use rustc_span::def_id::LocalDefId;
use rustc_span::Span;
use rustc_target::abi::{self, Abi};

Expand All @@ -17,8 +17,9 @@ use crate::errors;
use crate::errors::ConstEvalError;
use crate::interpret::eval_nullary_intrinsic;
use crate::interpret::{
intern_const_alloc_recursive, CtfeValidationMode, GlobalId, Immediate, InternKind, InterpCx,
InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking, StackPopCleanup,
create_static_alloc, intern_const_alloc_recursive, take_static_root_alloc, CtfeValidationMode,
GlobalId, Immediate, InternKind, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind,
OpTy, RefTracking, StackPopCleanup,
};

// Returns a pointer to where the result lives
Expand Down Expand Up @@ -46,7 +47,11 @@ fn eval_body_using_ecx<'mir, 'tcx>(
);
let layout = ecx.layout_of(body.bound_return_ty().instantiate(tcx, cid.instance.args))?;
assert!(layout.is_sized());
let ret = ecx.allocate(layout, MemoryKind::Stack)?;
let ret = if tcx.is_static(cid.instance.def_id()) && cid.promoted.is_none() {
create_static_alloc(ecx, cid.instance.def_id(), layout)?
} else {
ecx.allocate(layout, MemoryKind::Stack)?
};

trace!(
"eval_body_using_ecx: pushing stack frame for global: {}{}",
Expand Down Expand Up @@ -249,11 +254,37 @@ pub fn eval_to_const_value_raw_provider<'tcx>(
tcx.eval_to_allocation_raw(key).map(|val| turn_into_const_value(tcx, val, key))
}

#[instrument(skip(tcx), level = "debug")]
pub fn eval_static_initializer_provider<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
) -> ::rustc_middle::mir::interpret::EvalStaticInitializerRawResult<'tcx> {
assert!(tcx.is_static(def_id.to_def_id()));

let instance = ty::Instance::mono(tcx, def_id.to_def_id());
let cid = rustc_middle::mir::interpret::GlobalId { instance, promoted: None };
let mut ecx = InterpCx::new(
tcx,
tcx.def_span(def_id),
ty::ParamEnv::reveal_all(),
// Statics (and promoteds inside statics) may access other statics, because unlike consts
// they do not have to behave "as if" they were evaluated at runtime.
CompileTimeInterpreter::new(CanAccessMutGlobal::Yes, CheckAlignment::Error),
);
let alloc_id = eval_in_interpreter(&mut ecx, cid, true)?.alloc_id;
let alloc = take_static_root_alloc(&mut ecx, alloc_id);
let alloc = tcx.mk_const_alloc(alloc);
Ok(alloc)
}

#[instrument(skip(tcx), level = "debug")]
pub fn eval_to_allocation_raw_provider<'tcx>(
tcx: TyCtxt<'tcx>,
key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
) -> ::rustc_middle::mir::interpret::EvalToAllocationRawResult<'tcx> {
// This shouldn't be used for statics, since statics are conceptually places,
// not values -- so what we do here could break pointer identity.
assert!(key.value.promoted.is_some() || !tcx.is_static(key.value.instance.def_id()));
// Const eval always happens in Reveal::All mode in order to be able to use the hidden types of
// opaque types. This is needed for trivial things like `size_of`, but also for using associated
// types that are not specified in the opaque type.
Expand All @@ -273,7 +304,7 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
let def = cid.instance.def.def_id();
let is_static = tcx.is_static(def);

let ecx = InterpCx::new(
let mut ecx = InterpCx::new(
tcx,
tcx.def_span(def),
key.param_env,
Expand All @@ -283,19 +314,19 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
// so we have to reject reading mutable global memory.
CompileTimeInterpreter::new(CanAccessMutGlobal::from(is_static), CheckAlignment::Error),
);
eval_in_interpreter(ecx, cid, is_static)
eval_in_interpreter(&mut ecx, cid, is_static)
}

pub fn eval_in_interpreter<'mir, 'tcx>(
mut ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>,
ecx: &mut InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>,
cid: GlobalId<'tcx>,
is_static: bool,
) -> ::rustc_middle::mir::interpret::EvalToAllocationRawResult<'tcx> {
// `is_static` just means "in static", it could still be a promoted!
debug_assert_eq!(is_static, ecx.tcx.static_mutability(cid.instance.def_id()).is_some());

let res = ecx.load_mir(cid.instance.def, cid.promoted);
match res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, body)) {
match res.and_then(|body| eval_body_using_ecx(ecx, cid, body)) {
Err(error) => {
let (error, backtrace) = error.into_parts();
backtrace.print_backtrace();
Expand Down Expand Up @@ -331,7 +362,10 @@ pub fn eval_in_interpreter<'mir, 'tcx>(
Ok(mplace) => {
// Since evaluation had no errors, validate the resulting constant.
// This is a separate `try` block to provide more targeted error reporting.
assert!(ecx.machine.check_for_self_initializing_statics);
ecx.machine.check_for_self_initializing_statics = false;
let validation = const_validate_mplace(&ecx, &mplace, cid);
ecx.machine.check_for_self_initializing_statics = true;

let alloc_id = mplace.ptr().provenance.unwrap().alloc_id();

Expand Down Expand Up @@ -392,15 +426,9 @@ pub fn const_report_error<'mir, 'tcx>(

let ub_note = matches!(error, InterpError::UndefinedBehavior(_)).then(|| {});

let alloc = ecx.tcx.global_alloc(alloc_id).unwrap_memory().inner();
let mut bytes = String::new();
if alloc.size() != abi::Size::ZERO {
bytes = "\n".into();
// FIXME(translation) there might be pieces that are translatable.
write_allocation_bytes(*ecx.tcx, alloc, &mut bytes, " ").unwrap();
}
let raw_bytes =
errors::RawBytesNote { size: alloc.size().bytes(), align: alloc.align.bytes(), bytes };
let bytes = ecx.print_alloc_for_diagnostics(alloc_id);
let (size, align, _) = ecx.get_alloc_info(alloc_id);
let raw_bytes = errors::RawBytesNote { size: size.bytes(), align: align.bytes(), bytes };

crate::const_eval::report(
*ecx.tcx,
Expand Down
26 changes: 26 additions & 0 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Expand Up @@ -58,6 +58,10 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {

/// Whether to check alignment during evaluation.
pub(super) check_alignment: CheckAlignment,

/// In validation mode, we permit statics that read from their own base place.
/// See the documentation on the uses of this attribute for why we need it.
pub(super) check_for_self_initializing_statics: bool,
}

#[derive(Copy, Clone)]
Expand Down Expand Up @@ -91,6 +95,7 @@ impl<'mir, 'tcx> CompileTimeInterpreter<'mir, 'tcx> {
stack: Vec::new(),
can_access_mut_global,
check_alignment,
check_for_self_initializing_statics: true,
}
}
}
Expand Down Expand Up @@ -159,12 +164,17 @@ pub(crate) type CompileTimeEvalContext<'mir, 'tcx> =
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
pub enum MemoryKind {
Heap,
/// The base allocation of a static.
/// When this is read from, we need to error, because otherwise we could
/// invent values from nothing (e.g. `static FOO: Thing = FOO;`).
StaticRoot,
}

impl fmt::Display for MemoryKind {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
MemoryKind::Heap => write!(f, "heap allocation"),
MemoryKind::StaticRoot => write!(f, "static"),
}
}
}
Expand All @@ -174,6 +184,7 @@ impl interpret::MayLeak for MemoryKind {
fn may_leak(self) -> bool {
match self {
MemoryKind::Heap => false,
MemoryKind::StaticRoot => true,
}
}
}
Expand Down Expand Up @@ -745,6 +756,21 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
// Everything else is fine.
Ok(())
}

fn before_memory_read(
ecx: &InterpCx<'mir, 'tcx, Self>,
_alloc_extra: &Self::AllocExtra,
_prov: (AllocId, Self::ProvenanceExtra),
_range: AllocRange,
kind: Option<interpret::MemoryKind<MemoryKind>>,
) -> InterpResult<'tcx> {
if ecx.machine.check_for_self_initializing_statics {
if let Some(interpret::MemoryKind::Machine(MemoryKind::StaticRoot)) = kind {
ecx.tcx.dcx().emit_err(crate::errors::RecursiveStatic { span: ecx.tcx.span });
}
}
Ok(())
}
}

// Please do not add any code below the above `Machine` trait impl. I (oli-obk) plan more cleanups
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_const_eval/src/errors.rs
Expand Up @@ -35,6 +35,13 @@ pub(crate) struct MutablePtrInFinal {
pub kind: InternKind,
}

#[derive(Diagnostic)]
#[diag(const_eval_recursive_static)]
pub(crate) struct RecursiveStatic {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(const_eval_unstable_in_stable)]
pub(crate) struct UnstableInStable {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/cast.rs
Expand Up @@ -153,7 +153,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
);
}

self.copy_op(src, dest, /*allow_transmute*/ true)?;
self.copy_op_allow_transmute(src, dest)?;
}
}
Ok(())
Expand Down Expand Up @@ -441,7 +441,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
if src_field.layout.is_1zst() && cast_ty_field.is_1zst() {
// Skip 1-ZST fields.
} else if src_field.layout.ty == cast_ty_field.ty {
self.copy_op(&src_field, &dst_field, /*allow_transmute*/ false)?;
self.copy_op(&src_field, &dst_field)?;
} else {
if found_cast_field {
span_bug!(self.cur_span(), "unsize_into: more than one field to cast");
Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_const_eval/src/interpret/eval_context.rs
Expand Up @@ -896,7 +896,19 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
.local_to_op(self.frame(), mir::RETURN_PLACE, None)
.expect("return place should always be live");
let dest = self.frame().return_place.clone();
let err = self.copy_op(&op, &dest, /*allow_transmute*/ true);
let err = if self.stack().len() == 1 {
// The initializer of constants and statics will get validated separately
// after the constant has been fully evaluated. While we could fall back to the default
// code path, that will cause -Zenforce-validity to cycle on static initializers.
// Reading from a static's memory is not allowed during its evaluation, and will always
// trigger a cycle error. Validation must read from the memory of the current item.
// For Miri this means we do not validate the root frame return value,
// but Miri anyway calls `read_target_isize` on that so separate validation
// is not needed.
self.copy_op_no_dest_validation(&op, &dest)
} else {
self.copy_op_allow_transmute(&op, &dest)
};
trace!("return value: {:?}", self.dump_place(&dest));
// We delay actually short-circuiting on this error until *after* the stack frame is
// popped, since we want this error to be attributed to the caller, whose type defines
Expand Down
38 changes: 29 additions & 9 deletions compiler/rustc_const_eval/src/interpret/intern.rs
Expand Up @@ -17,7 +17,7 @@ use rustc_ast::Mutability;
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_errors::ErrorGuaranteed;
use rustc_hir as hir;
use rustc_middle::mir::interpret::{CtfeProvenance, InterpResult};
use rustc_middle::mir::interpret::{CtfeProvenance, GlobalAlloc, InterpResult};
use rustc_middle::ty::layout::TyAndLayout;

use super::{AllocId, Allocation, InterpCx, MPlaceTy, Machine, MemoryKind, PlaceTy};
Expand Down Expand Up @@ -96,12 +96,12 @@ pub fn intern_const_alloc_recursive<
) -> Result<(), ErrorGuaranteed> {
// We are interning recursively, and for mutability we are distinguishing the "root" allocation
// that we are starting in, and all other allocations that we are encountering recursively.
let (base_mutability, inner_mutability) = match intern_kind {
let (base_mutability, inner_mutability, is_static) = match intern_kind {
InternKind::Constant | InternKind::Promoted => {
// Completely immutable. Interning anything mutably here can only lead to unsoundness,
// since all consts are conceptually independent values but share the same underlying
// memory.
(Mutability::Not, Mutability::Not)
(Mutability::Not, Mutability::Not, false)
}
InternKind::Static(Mutability::Not) => {
(
Expand All @@ -114,22 +114,31 @@ pub fn intern_const_alloc_recursive<
// Inner allocations are never mutable. They can only arise via the "tail
// expression" / "outer scope" rule, and we treat them consistently with `const`.
Mutability::Not,
true,
)
}
InternKind::Static(Mutability::Mut) => {
// Just make everything mutable. We accept code like
// `static mut X = &mut [42]`, so even inner allocations need to be mutable.
(Mutability::Mut, Mutability::Mut)
(Mutability::Mut, Mutability::Mut, true)
}
};

// Intern the base allocation, and initialize todo list for recursive interning.
let base_alloc_id = ret.ptr().provenance.unwrap().alloc_id();
trace!(?base_alloc_id, ?base_mutability);
// First we intern the base allocation, as it requires a different mutability.
// This gives us the initial set of nested allocations, which will then all be processed
// recursively in the loop below.
let mut todo: Vec<_> =
intern_shallow(ecx, base_alloc_id, base_mutability).unwrap().map(|prov| prov).collect();
let mut todo: Vec<_> = if is_static {
// Do not steal the root allocation, we need it later for `take_static_root_alloc`
// But still change its mutability to match the requested one.
let alloc = ecx.memory.alloc_map.get_mut(&base_alloc_id).unwrap();
alloc.1.mutability = base_mutability;
alloc.1.provenance().ptrs().iter().map(|&(_, prov)| prov).collect()
} else {
intern_shallow(ecx, base_alloc_id, base_mutability).unwrap().map(|prov| prov).collect()
};
// We need to distinguish "has just been interned" from "was already in `tcx`",
// so we track this in a separate set.
let mut just_interned: FxHashSet<_> = std::iter::once(base_alloc_id).collect();
Expand All @@ -147,6 +156,7 @@ pub fn intern_const_alloc_recursive<
// before validation, and interning doesn't know the type of anything, this means we can't show
// better errors. Maybe we should consider doing validation before interning in the future.
while let Some(prov) = todo.pop() {
trace!(?prov);
let alloc_id = prov.alloc_id();
// Crucially, we check this *before* checking whether the `alloc_id`
// has already been interned. The point of this check is to ensure that when
Expand All @@ -159,6 +169,13 @@ pub fn intern_const_alloc_recursive<
&& inner_mutability == Mutability::Not
&& !prov.immutable()
{
if base_alloc_id == alloc_id && is_static {
// This is a pointer to the static itself. It's ok for a static to refer to itself,
// even mutably. Whether that mutable pointer is legal at all is checked in validation.
// See tests/ui/statics/recursive_interior_mut.rs for how such a situation can occur.
continue;
}

if ecx.tcx.try_get_global_alloc(alloc_id).is_some()
&& !just_interned.contains(&alloc_id)
{
Expand All @@ -175,11 +192,14 @@ pub fn intern_const_alloc_recursive<
// `&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
// promoteds as immutable.
trace!("found bad mutable pointer");
found_bad_mutable_pointer = true;
}
if ecx.tcx.try_get_global_alloc(alloc_id).is_some() {
// Already interned.
debug_assert!(!ecx.memory.alloc_map.contains_key(&alloc_id));
if let Some(global) = ecx.tcx.try_get_global_alloc(alloc_id) {
if let GlobalAlloc::Memory(_) = global {
// Already interned.
debug_assert!(!ecx.memory.alloc_map.contains_key(&alloc_id));
}
continue;
}
just_interned.insert(alloc_id);
Expand Down

0 comments on commit cc542ae

Please sign in to comment.