Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ fn const_validate_mplace<'tcx>(
cid: GlobalId<'tcx>,
) -> Result<(), ErrorHandled> {
let alloc_id = mplace.ptr().provenance.unwrap().alloc_id();
let mut ref_tracking = RefTracking::new(mplace.clone());
let mut ref_tracking = RefTracking::new(mplace.clone(), mplace.layout.ty);
let mut inner = false;
while let Some((mplace, path)) = ref_tracking.next() {
let mode = match ecx.tcx.static_mutability(cid.instance.def_id()) {
Expand Down
91 changes: 55 additions & 36 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ use super::UnsupportedOpInfo::*;
macro_rules! err_validation_failure {
($where:expr, $msg:expr ) => {{
let where_ = &$where;
let path = if !where_.is_empty() {
let path = if !where_.projs.is_empty() {
let mut path = String::new();
write_path(&mut path, where_);
write_path(&mut path, &where_.projs);
Some(path)
} else {
None
Expand All @@ -59,6 +59,7 @@ macro_rules! err_validation_failure {
use ValidationErrorKind::*;
let msg = ValidationErrorKind::from($msg);
err_ub!(ValidationError {
orig_ty: where_.orig_ty,
path,
ptr_bytes_warning: msg.ptr_bytes_warning(),
msg: msg.to_string(),
Expand Down Expand Up @@ -234,7 +235,7 @@ fn fmt_range(r: WrappingRange, max_hi: u128) -> String {
/// So we track a `Vec<PathElem>` where `PathElem` contains all the data we
/// need to later print something for the user.
#[derive(Copy, Clone, Debug)]
pub enum PathElem {
pub enum PathElem<'tcx> {
Field(Symbol),
Variant(Symbol),
CoroutineState(VariantIdx),
Expand All @@ -244,10 +245,22 @@ pub enum PathElem {
Deref,
EnumTag,
CoroutineTag,
DynDowncast,
DynDowncast(Ty<'tcx>),
Vtable,
}

#[derive(Clone, Debug)]
pub struct Path<'tcx> {
orig_ty: Ty<'tcx>,
projs: Vec<PathElem<'tcx>>,
}

impl<'tcx> Path<'tcx> {
fn new(ty: Ty<'tcx>) -> Self {
Self { orig_ty: ty, projs: vec![] }
}
}

/// Extra things to check for during validation of CTFE results.
#[derive(Copy, Clone)]
pub enum CtfeValidationMode {
Expand Down Expand Up @@ -280,16 +293,10 @@ pub struct RefTracking<T, PATH = ()> {
todo: Vec<(T, PATH)>,
}

impl<T: Clone + Eq + Hash + std::fmt::Debug, PATH: Default> RefTracking<T, PATH> {
impl<T: Clone + Eq + Hash + std::fmt::Debug, PATH> RefTracking<T, PATH> {
pub fn empty() -> Self {
RefTracking { seen: FxHashSet::default(), todo: vec![] }
}
pub fn new(val: T) -> Self {
let mut ref_tracking_for_consts =
RefTracking { seen: FxHashSet::default(), todo: vec![(val.clone(), PATH::default())] };
ref_tracking_for_consts.seen.insert(val);
ref_tracking_for_consts
}
pub fn next(&mut self) -> Option<(T, PATH)> {
self.todo.pop()
}
Expand All @@ -304,8 +311,17 @@ impl<T: Clone + Eq + Hash + std::fmt::Debug, PATH: Default> RefTracking<T, PATH>
}
}

impl<'tcx, T: Clone + Eq + Hash + std::fmt::Debug> RefTracking<T, Path<'tcx>> {
pub fn new(val: T, ty: Ty<'tcx>) -> Self {
let mut ref_tracking_for_consts =
RefTracking { seen: FxHashSet::default(), todo: vec![(val.clone(), Path::new(ty))] };
ref_tracking_for_consts.seen.insert(val);
ref_tracking_for_consts
}
}

/// Format a path
fn write_path(out: &mut String, path: &[PathElem]) {
fn write_path(out: &mut String, path: &[PathElem<'_>]) {
use self::PathElem::*;

for elem in path.iter() {
Expand All @@ -323,7 +339,7 @@ fn write_path(out: &mut String, path: &[PathElem]) {
// even use the usual syntax because we are just showing the projections,
// not the root.
Deref => write!(out, ".<deref>"),
DynDowncast => write!(out, ".<dyn-downcast>"),
DynDowncast(ty) => write!(out, ".<dyn-downcast({ty})>"),
Vtable => write!(out, ".<vtable>"),
}
.unwrap()
Expand Down Expand Up @@ -382,10 +398,9 @@ impl RangeSet {

struct ValidityVisitor<'rt, 'tcx, M: Machine<'tcx>> {
/// The `path` may be pushed to, but the part that is present when a function
/// starts must not be changed! `visit_fields` and `visit_array` rely on
/// this stack discipline.
path: Vec<PathElem>,
ref_tracking: Option<&'rt mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Vec<PathElem>>>,
/// starts must not be changed! `with_elem` relies on this stack discipline.
path: Path<'tcx>,
ref_tracking: Option<&'rt mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Path<'tcx>>>,
/// `None` indicates this is not validating for CTFE (but for runtime).
ctfe_mode: Option<CtfeValidationMode>,
ecx: &'rt mut InterpCx<'tcx, M>,
Expand All @@ -404,7 +419,12 @@ struct ValidityVisitor<'rt, 'tcx, M: Machine<'tcx>> {
}

impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
fn aggregate_field_path_elem(&mut self, layout: TyAndLayout<'tcx>, field: usize) -> PathElem {
fn aggregate_field_path_elem(
&mut self,
layout: TyAndLayout<'tcx>,
field: usize,
field_ty: Ty<'tcx>,
) -> PathElem<'tcx> {
// First, check if we are projecting to a variant.
match layout.variants {
Variants::Multiple { tag_field, .. } => {
Expand Down Expand Up @@ -474,7 +494,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
// dyn traits
ty::Dynamic(..) => {
assert_eq!(field, 0);
PathElem::DynDowncast
PathElem::DynDowncast(field_ty)
}

// nothing else has an aggregate layout
Expand All @@ -484,17 +504,17 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {

fn with_elem<R>(
&mut self,
elem: PathElem,
elem: PathElem<'tcx>,
f: impl FnOnce(&mut Self) -> InterpResult<'tcx, R>,
) -> InterpResult<'tcx, R> {
// Remember the old state
let path_len = self.path.len();
let path_len = self.path.projs.len();
// Record new element
self.path.push(elem);
self.path.projs.push(elem);
// Perform operation
let r = f(self)?;
// Undo changes
self.path.truncate(path_len);
self.path.projs.truncate(path_len);
// Done
interp_ok(r)
}
Expand Down Expand Up @@ -796,10 +816,10 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
ref_tracking.track(place, || {
// We need to clone the path anyway, make sure it gets created
// with enough space for the additional `Deref`.
let mut new_path = Vec::with_capacity(path.len() + 1);
new_path.extend(path);
new_path.push(PathElem::Deref);
new_path
let mut new_projs = Vec::with_capacity(path.projs.len() + 1);
new_projs.extend(&path.projs);
new_projs.push(PathElem::Deref);
Path { projs: new_projs, orig_ty: path.orig_ty }
});
}
interp_ok(())
Expand Down Expand Up @@ -1220,7 +1240,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
field: usize,
new_val: &PlaceTy<'tcx, M::Provenance>,
) -> InterpResult<'tcx> {
let elem = self.aggregate_field_path_elem(old_val.layout, field);
let elem = self.aggregate_field_path_elem(old_val.layout, field, new_val.layout.ty);
self.with_elem(elem, move |this| this.visit_value(new_val))
}

Expand Down Expand Up @@ -1385,7 +1405,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
access.bad.start.bytes() / layout.size.bytes(),
)
.unwrap();
self.path.push(PathElem::ArrayElem(i));
self.path.projs.push(PathElem::ArrayElem(i));

if matches!(kind, Ub(InvalidUninitBytes(_))) {
err_validation_failure!(self.path, Uninit { expected })
Expand Down Expand Up @@ -1515,8 +1535,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
fn validate_operand_internal(
&mut self,
val: &PlaceTy<'tcx, M::Provenance>,
path: Vec<PathElem>,
ref_tracking: Option<&mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Vec<PathElem>>>,
path: Path<'tcx>,
ref_tracking: Option<&mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Path<'tcx>>>,
ctfe_mode: Option<CtfeValidationMode>,
reset_provenance_and_padding: bool,
) -> InterpResult<'tcx> {
Expand Down Expand Up @@ -1569,8 +1589,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
pub(crate) fn const_validate_operand(
&mut self,
val: &PlaceTy<'tcx, M::Provenance>,
path: Vec<PathElem>,
ref_tracking: &mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Vec<PathElem>>,
path: Path<'tcx>,
ref_tracking: &mut RefTracking<MPlaceTy<'tcx, M::Provenance>, Path<'tcx>>,
ctfe_mode: CtfeValidationMode,
) -> InterpResult<'tcx> {
self.validate_operand_internal(
Expand Down Expand Up @@ -1599,14 +1619,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
reset_provenance_and_padding,
?val,
);

// Note that we *could* actually be in CTFE here with `-Zextra-const-ub-checks`, but it's
// still correct to not use `ctfe_mode`: that mode is for validation of the final constant
// value, it rules out things like `UnsafeCell` in awkward places.
if !recursive {
return self.validate_operand_internal(
val,
vec![],
Path::new(val.layout.ty),
None,
None,
reset_provenance_and_padding,
Expand All @@ -1616,7 +1635,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
let mut ref_tracking = RefTracking::empty();
self.validate_operand_internal(
val,
vec![],
Path::new(val.layout.ty),
Some(&mut ref_tracking),
None,
reset_provenance_and_padding,
Expand Down
34 changes: 18 additions & 16 deletions compiler/rustc_data_structures/src/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,22 +181,24 @@ impl<D: Decoder> Decodable<D> for Fingerprint {
}
}

// `PackedFingerprint` wraps a `Fingerprint`. Its purpose is to, on certain
// architectures, behave like a `Fingerprint` without alignment requirements.
// This behavior is only enabled on x86 and x86_64, where the impact of
// unaligned accesses is tolerable in small doses.
//
// This may be preferable to use in large collections of structs containing
// fingerprints, as it can reduce memory consumption by preventing the padding
// that the more strictly-aligned `Fingerprint` can introduce. An application of
// this is in the query dependency graph, which contains a large collection of
// `DepNode`s. As of this writing, the size of a `DepNode` decreases by ~30%
// (from 24 bytes to 17) by using the packed representation here, which
// noticeably decreases total memory usage when compiling large crates.
//
// The wrapped `Fingerprint` is private to reduce the chance of a client
// invoking undefined behavior by taking a reference to the packed field.
#[cfg_attr(any(target_arch = "x86", target_arch = "x86_64"), repr(packed))]
/// `PackedFingerprint` wraps a `Fingerprint`.
/// Its purpose is to behave like a `Fingerprint` without alignment requirements.
///
/// This may be preferable to use in large collections of structs containing
/// fingerprints, as it can reduce memory consumption by preventing the padding
/// that the more strictly-aligned `Fingerprint` can introduce. An application of
/// this is in the query dependency graph, which contains a large collection of
/// `DepNode`s. As of this writing, the size of a `DepNode` decreases by 25%
/// (from 24 bytes to 18) by using the packed representation here, which
/// noticeably decreases total memory usage when compiling large crates.
///
/// (Unalignment was previously restricted to `x86` and `x86_64` hosts, but is
/// now enabled by default for all host architectures, in the hope that the
/// memory and cache savings should outweigh any unaligned access penalty.)
///
/// The wrapped `Fingerprint` is private to reduce the chance of a client
/// invoking undefined behavior by taking a reference to the packed field.
#[repr(packed)]
#[derive(Eq, PartialEq, Ord, PartialOrd, Debug, Clone, Copy, Hash)]
pub struct PackedFingerprint(Fingerprint);

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2947,7 +2947,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
);
if def_id.is_local() {
let name = tcx.def_path_str(def_id);
err.span_suggestion(
err.span_suggestion_verbose(
tcx.def_span(def_id).shrink_to_lo(),
format!("add `type` before `const` for `{name}`"),
format!("type "),
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_middle/src/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,9 +412,6 @@ mod size_asserts {
use super::*;
// tidy-alphabetical-start
static_assert_size!(DepKind, 2);
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
static_assert_size!(DepNode, 18);
#[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))]
static_assert_size!(DepNode, 24);
// tidy-alphabetical-end
}
15 changes: 10 additions & 5 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,12 @@ pub enum UndefinedBehaviorInfo<'tcx> {
/// Free-form case. Only for errors that are never caught! Used by miri
Ub(String),
/// Validation error.
ValidationError { path: Option<String>, msg: String, ptr_bytes_warning: bool },
ValidationError {
orig_ty: Ty<'tcx>,
path: Option<String>,
msg: String,
ptr_bytes_warning: bool,
},

/// Unreachable code was executed.
Unreachable,
Expand Down Expand Up @@ -457,11 +462,11 @@ impl<'tcx> fmt::Display for UndefinedBehaviorInfo<'tcx> {
match self {
Ub(msg) => write!(f, "{msg}"),

ValidationError { path: None, msg, .. } => {
write!(f, "constructing invalid value: {msg}")
ValidationError { orig_ty, path: None, msg, .. } => {
write!(f, "constructing invalid value of type {orig_ty}: {msg}")
}
ValidationError { path: Some(path), msg, .. } => {
write!(f, "constructing invalid value at {path}: {msg}")
ValidationError { orig_ty, path: Some(path), msg, .. } => {
write!(f, "constructing invalid value of type {orig_ty}: at {path}, {msg}")
}

Unreachable => write!(f, "entering unreachable code"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -797,12 +797,16 @@ pub(in crate::solve) fn const_conditions_for_destruct<I: Interner>(
| ty::Infer(ty::InferTy::FloatVar(_) | ty::InferTy::IntVar(_))
| ty::Error(_) => Ok(vec![]),

// Coroutines and closures could implement `[const] Drop`,
// Closures are [const] Destruct when all of their upvars (captures) are [const] Destruct.
ty::Closure(_, args) => {
let closure_args = args.as_closure();
Ok(vec![ty::TraitRef::new(cx, destruct_def_id, [closure_args.tupled_upvars_ty()])])
}
// Coroutines could implement `[const] Drop`,
// but they don't really need to right now.
ty::Closure(_, _)
| ty::CoroutineClosure(_, _)
| ty::Coroutine(_, _)
| ty::CoroutineWitness(_, _) => Err(NoSolution),
ty::CoroutineClosure(_, _) | ty::Coroutine(_, _) | ty::CoroutineWitness(_, _) => {
Err(NoSolution)
}

// FIXME(unsafe_binders): Unsafe binders could implement `[const] Drop`
// if their inner type implements it.
Expand Down
15 changes: 10 additions & 5 deletions compiler/rustc_trait_selection/src/traits/effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,12 +472,17 @@ fn evaluate_host_effect_for_destruct_goal<'tcx>(
| ty::Infer(ty::InferTy::FloatVar(_) | ty::InferTy::IntVar(_))
| ty::Error(_) => thin_vec![],

// Coroutines and closures could implement `[const] Drop`,
// Closures are [const] Destruct when all of their upvars (captures) are [const] Destruct.
ty::Closure(_, args) => {
let closure_args = args.as_closure();
thin_vec![ty::TraitRef::new(tcx, destruct_def_id, [closure_args.tupled_upvars_ty()])]
}

// Coroutines could implement `[const] Drop`,
// but they don't really need to right now.
ty::Closure(_, _)
| ty::CoroutineClosure(_, _)
| ty::Coroutine(_, _)
| ty::CoroutineWitness(_, _) => return Err(EvaluationFailure::NoSolution),
ty::CoroutineClosure(_, _) | ty::Coroutine(_, _) | ty::CoroutineWitness(_, _) => {
return Err(EvaluationFailure::NoSolution);
}

// FIXME(unsafe_binders): Unsafe binders could implement `[const] Drop`
// if their inner type implements it.
Expand Down
Loading
Loading