Skip to content

Commit

Permalink
Check vtable projections for validity in miri
Browse files Browse the repository at this point in the history
  • Loading branch information
compiler-errors committed Sep 23, 2024
1 parent 6c6d210 commit ffa70dd
Show file tree
Hide file tree
Showing 22 changed files with 135 additions and 82 deletions.
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,13 @@ pub(crate) fn codegen_const_value<'tcx>(
fx.module.declare_func_in_func(func_id, &mut fx.bcx.func);
fx.bcx.ins().func_addr(fx.pointer_type, local_func_id)
}
GlobalAlloc::VTable(ty, trait_ref) => {
GlobalAlloc::VTable(ty, vtable) => {
let data_id = data_id_for_vtable(
fx.tcx,
&mut fx.constants_cx,
fx.module,
ty,
trait_ref,
vtable.principal(),
);
let local_data_id =
fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
Expand Down Expand Up @@ -456,8 +456,8 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant
GlobalAlloc::Memory(target_alloc) => {
data_id_for_alloc_id(cx, module, alloc_id, target_alloc.inner().mutability)
}
GlobalAlloc::VTable(ty, trait_ref) => {
data_id_for_vtable(tcx, cx, module, ty, trait_ref)
GlobalAlloc::VTable(ty, vtable) => {
data_id_for_vtable(tcx, cx, module, ty, vtable.principal())
}
GlobalAlloc::Static(def_id) => {
if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::THREAD_LOCAL)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_gcc/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,10 @@ impl<'gcc, 'tcx> ConstCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
value
}
GlobalAlloc::Function { instance, .. } => self.get_fn_addr(instance),
GlobalAlloc::VTable(ty, trait_ref) => {
GlobalAlloc::VTable(ty, vtable) => {
let alloc = self
.tcx
.global_alloc(self.tcx.vtable_allocation((ty, trait_ref)))
.global_alloc(self.tcx.vtable_allocation((ty, vtable.principal())))
.unwrap_memory();
let init = const_alloc_to_gcc(self, alloc);
self.static_addr_of(init, alloc.inner().align, None)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,10 +290,10 @@ impl<'ll, 'tcx> ConstCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> {
self.get_fn_addr(instance.polymorphize(self.tcx)),
self.data_layout().instruction_address_space,
),
GlobalAlloc::VTable(ty, trait_ref) => {
GlobalAlloc::VTable(ty, vtable) => {
let alloc = self
.tcx
.global_alloc(self.tcx.vtable_allocation((ty, trait_ref)))
.global_alloc(self.tcx.vtable_allocation((ty, vtable.principal())))
.unwrap_memory();
let init = const_alloc_to_llvm(self, alloc, /*static*/ false);
let value = self.static_addr_of(init, alloc.inner().align, None);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ const_eval_invalid_vtable_pointer =
using {$pointer} as vtable pointer but it does not point to a vtable
const_eval_invalid_vtable_trait =
using vtable for trait `{$vtable_trait}` but trait `{$expected_trait}` was expected
using vtable for `{$allocated_vtable}` but `{$expected_vtable}` was expected
const_eval_lazy_lock =
consider wrapping this expression in `std::sync::LazyLock::new(|| ...)`
Expand Down Expand Up @@ -459,7 +459,7 @@ const_eval_validation_invalid_fn_ptr = {$front_matter}: encountered {$value}, bu
const_eval_validation_invalid_ref_meta = {$front_matter}: encountered invalid reference metadata: total size is bigger than largest supported object
const_eval_validation_invalid_ref_slice_meta = {$front_matter}: encountered invalid reference metadata: slice is bigger than largest supported object
const_eval_validation_invalid_vtable_ptr = {$front_matter}: encountered {$value}, but expected a vtable pointer
const_eval_validation_invalid_vtable_trait = {$front_matter}: wrong trait in wide pointer vtable: expected `{$ref_trait}`, but encountered `{$vtable_trait}`
const_eval_validation_invalid_vtable_trait = {$front_matter}: wrong trait in wide pointer vtable: expected `{$expected_vtable}`, but encountered `{$allocated_vtable}`
const_eval_validation_mutable_ref_to_immutable = {$front_matter}: encountered mutable reference or box pointing to read-only memory
const_eval_validation_never_val = {$front_matter}: encountered a value of the never type `!`
const_eval_validation_null_box = {$front_matter}: encountered a null box
Expand Down
18 changes: 6 additions & 12 deletions compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,12 +525,9 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
UnterminatedCString(ptr) | InvalidFunctionPointer(ptr) | InvalidVTablePointer(ptr) => {
diag.arg("pointer", ptr);
}
InvalidVTableTrait { expected_trait, vtable_trait } => {
diag.arg("expected_trait", expected_trait.to_string());
diag.arg(
"vtable_trait",
vtable_trait.map(|t| t.to_string()).unwrap_or_else(|| format!("<trivial>")),
);
InvalidVTableTrait { expected_vtable, allocated_vtable } => {
diag.arg("expected_vtable", expected_vtable.to_string());
diag.arg("allocated_vtable", allocated_vtable.to_string());
}
PointerUseAfterFree(alloc_id, msg) => {
diag.arg("alloc_id", alloc_id)
Expand Down Expand Up @@ -780,12 +777,9 @@ impl<'tcx> ReportErrorExt for ValidationErrorInfo<'tcx> {
DanglingPtrNoProvenance { pointer, .. } => {
err.arg("pointer", pointer);
}
InvalidMetaWrongTrait { expected_trait: ref_trait, vtable_trait } => {
err.arg("ref_trait", ref_trait.to_string());
err.arg(
"vtable_trait",
vtable_trait.map(|t| t.to_string()).unwrap_or_else(|| format!("<trivial>")),
);
InvalidMetaWrongTrait { allocated_vtable, expected_vtable } => {
err.arg("allocated_vtable", allocated_vtable.to_string());
err.arg("expected_vtable", expected_vtable.to_string());
}
NullPtr { .. }
| ConstRefToMutable
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_const_eval/src/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
CastKind::DynStar => {
if let ty::Dynamic(data, _, ty::DynStar) = cast_ty.kind() {
// Initial cast from sized to dyn trait
let vtable = self.get_vtable_ptr(src.layout.ty, data.principal())?;
let vtable = self.get_vtable_ptr(src.layout.ty, data)?;
let vtable = Scalar::from_maybe_pointer(vtable, self);
let data = self.read_immediate(src)?.to_scalar();
let _assert_pointer_like = data.to_pointer(self)?;
Expand Down Expand Up @@ -446,12 +446,12 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
}

// Get the destination trait vtable and return that.
let new_vptr = self.get_vtable_ptr(ty, data_b.principal())?;
let new_vptr = self.get_vtable_ptr(ty, data_b)?;
self.write_immediate(Immediate::new_dyn_trait(old_data, new_vptr, self), dest)
}
(_, &ty::Dynamic(data, _, ty::Dyn)) => {
// Initial cast from sized to dyn trait
let vtable = self.get_vtable_ptr(src_pointee_ty, data.principal())?;
let vtable = self.get_vtable_ptr(src_pointee_ty, data)?;
let ptr = self.read_pointer(src)?;
let val = Immediate::new_dyn_trait(ptr, vtable, &*self.tcx);
self.write_immediate(val, dest)
Expand Down
14 changes: 6 additions & 8 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -943,12 +943,13 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
if offset.bytes() != 0 {
throw_ub!(InvalidVTablePointer(Pointer::new(alloc_id, offset)))
}
let Some(GlobalAlloc::VTable(ty, vtable_trait)) = self.tcx.try_get_global_alloc(alloc_id)
let Some(GlobalAlloc::VTable(ty, allocated_vtable)) =
self.tcx.try_get_global_alloc(alloc_id)
else {
throw_ub!(InvalidVTablePointer(Pointer::new(alloc_id, offset)))
};
if let Some(expected_trait) = expected_trait {
self.check_vtable_for_type(vtable_trait, expected_trait)?;
if let Some(expected_vtable) = expected_trait {
self.check_vtable_for_type(allocated_vtable, expected_vtable)?;
}
Ok(ty)
}
Expand Down Expand Up @@ -1113,11 +1114,8 @@ impl<'a, 'tcx, M: Machine<'tcx>> std::fmt::Debug for DumpAllocs<'a, 'tcx, M> {
Some(GlobalAlloc::Function { instance, .. }) => {
write!(fmt, " (fn: {instance})")?;
}
Some(GlobalAlloc::VTable(ty, Some(trait_ref))) => {
write!(fmt, " (vtable: impl {trait_ref} for {ty})")?;
}
Some(GlobalAlloc::VTable(ty, None)) => {
write!(fmt, " (vtable: impl <auto trait> for {ty})")?;
Some(GlobalAlloc::VTable(ty, vtable)) => {
write!(fmt, " (vtable: impl {vtable} for {ty})")?;
}
Some(GlobalAlloc::Static(did)) => {
write!(fmt, " (static: {})", self.ecx.tcx.def_path_str(did))?;
Expand Down
56 changes: 40 additions & 16 deletions compiler/rustc_const_eval/src/interpret/traits.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustc_middle::mir::interpret::{InterpResult, Pointer};
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::{self, Ty, TyCtxt, VtblEntry};
use rustc_middle::ty::{self, ExistentialPredicateStableCmpExt, Ty, TyCtxt, VtblEntry};
use rustc_target::abi::{Align, Size};
use tracing::trace;

Expand All @@ -18,19 +18,18 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
pub fn get_vtable_ptr(
&self,
ty: Ty<'tcx>,
poly_trait_ref: Option<ty::PolyExistentialTraitRef<'tcx>>,
vtable: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> {
trace!("get_vtable(trait_ref={:?})", poly_trait_ref);
trace!("get_vtable(ty={ty:?}, vtable={vtable:?})");

let (ty, poly_trait_ref) = self.tcx.erase_regions((ty, poly_trait_ref));
let (ty, vtable) = self.tcx.erase_regions((ty, vtable));

// All vtables must be monomorphic, bail out otherwise.
ensure_monomorphic_enough(*self.tcx, ty)?;
ensure_monomorphic_enough(*self.tcx, poly_trait_ref)?;
ensure_monomorphic_enough(*self.tcx, vtable)?;

let salt = M::get_global_alloc_salt(self, None);
let vtable_symbolic_allocation =
self.tcx.reserve_and_set_vtable_alloc(ty, poly_trait_ref, salt);
let vtable_symbolic_allocation = self.tcx.reserve_and_set_vtable_alloc(ty, vtable, salt);
let vtable_ptr = self.global_root_pointer(Pointer::from(vtable_symbolic_allocation))?;
Ok(vtable_ptr.into())
}
Expand Down Expand Up @@ -64,17 +63,42 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
/// expected trait type.
pub(super) fn check_vtable_for_type(
&self,
vtable_trait: Option<ty::PolyExistentialTraitRef<'tcx>>,
expected_trait: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
allocated_vtable: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
expected_vtable: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
) -> InterpResult<'tcx> {
let eq = match (expected_trait.principal(), vtable_trait) {
(Some(a), Some(b)) => self.eq_in_param_env(a, b),
(None, None) => true,
_ => false,
};
if !eq {
throw_ub!(InvalidVTableTrait { expected_trait, vtable_trait });
let mut sorted_allocated_v: Vec<_> = allocated_vtable.without_auto_traits().collect();
let mut sorted_expected_v: Vec<_> = expected_vtable.without_auto_traits().collect();
// `skip_binder` here is okay because `stable_cmp` doesn't look at binders
sorted_allocated_v.sort_by(|a, b| a.skip_binder().stable_cmp(*self.tcx, &b.skip_binder()));
sorted_allocated_v.dedup();
sorted_expected_v.sort_by(|a, b| a.skip_binder().stable_cmp(*self.tcx, &b.skip_binder()));
sorted_expected_v.dedup();

if sorted_allocated_v.len() != sorted_expected_v.len() {
throw_ub!(InvalidVTableTrait { allocated_vtable, expected_vtable });
}

for (a_pred, b_pred) in std::iter::zip(sorted_allocated_v, sorted_expected_v) {
let is_eq = match (a_pred.skip_binder(), b_pred.skip_binder()) {
(
ty::ExistentialPredicate::Trait(a_data),
ty::ExistentialPredicate::Trait(b_data),
) => self.eq_in_param_env(a_pred.rebind(a_data), b_pred.rebind(b_data)),

(
ty::ExistentialPredicate::Projection(a_data),
ty::ExistentialPredicate::Projection(b_data),
) => self.eq_in_param_env(a_pred.rebind(a_data), b_pred.rebind(b_data)),

_ => false,
};
if !is_eq {
throw_ub!(InvalidVTableTrait { allocated_vtable, expected_vtable });
}
}

// FIXME: Should we validate auto traits here?

Ok(())
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,8 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
self.path,
Ub(DanglingIntPointer{ .. } | InvalidVTablePointer(..)) =>
InvalidVTablePtr { value: format!("{vtable}") },
Ub(InvalidVTableTrait { expected_trait, vtable_trait }) => {
InvalidMetaWrongTrait { expected_trait, vtable_trait: *vtable_trait }
Ub(InvalidVTableTrait { allocated_vtable, expected_vtable }) => {
InvalidMetaWrongTrait { allocated_vtable, expected_vtable }
},
);
}
Expand Down Expand Up @@ -1280,8 +1280,8 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
self.path,
// It's not great to catch errors here, since we can't give a very good path,
// but it's better than ICEing.
Ub(InvalidVTableTrait { expected_trait, vtable_trait }) => {
InvalidMetaWrongTrait { expected_trait, vtable_trait: *vtable_trait }
Ub(InvalidVTableTrait { allocated_vtable, expected_vtable }) => {
InvalidMetaWrongTrait { allocated_vtable, expected_vtable: *expected_vtable }
},
);
}
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,10 @@ pub enum UndefinedBehaviorInfo<'tcx> {
InvalidVTablePointer(Pointer<AllocId>),
/// Using a vtable for the wrong trait.
InvalidVTableTrait {
expected_trait: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
vtable_trait: Option<ty::PolyExistentialTraitRef<'tcx>>,
/// The vtable that was actually allocated into global memory.
allocated_vtable: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
/// The vtable that was expected at the point in MIR that it was accessed.
expected_vtable: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
},
/// Using a string that is not valid UTF-8,
InvalidStr(std::str::Utf8Error),
Expand Down Expand Up @@ -479,8 +481,10 @@ pub enum ValidationErrorKind<'tcx> {
value: String,
},
InvalidMetaWrongTrait {
expected_trait: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
vtable_trait: Option<ty::PolyExistentialTraitRef<'tcx>>,
/// The vtable that was actually allocated into global memory.
allocated_vtable: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
/// The vtable that was expected at the point in MIR that it was accessed.
expected_vtable: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
},
InvalidMetaSliceTooLarge {
ptr_kind: PointerKind,
Expand Down
11 changes: 5 additions & 6 deletions compiler/rustc_middle/src/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,8 @@ impl<'s> AllocDecodingSession<'s> {
}
AllocDiscriminant::VTable => {
trace!("creating vtable alloc ID");
let ty = <Ty<'_> as Decodable<D>>::decode(decoder);
let poly_trait_ref =
<Option<ty::PolyExistentialTraitRef<'_>> as Decodable<D>>::decode(decoder);
let ty = Decodable::decode(decoder);
let poly_trait_ref = Decodable::decode(decoder);
trace!("decoded vtable alloc instance: {ty:?}, {poly_trait_ref:?}");
decoder.interner().reserve_and_set_vtable_alloc(ty, poly_trait_ref, CTFE_ALLOC_SALT)
}
Expand All @@ -259,7 +258,7 @@ pub enum GlobalAlloc<'tcx> {
/// The alloc ID is used as a function pointer.
Function { instance: Instance<'tcx> },
/// This alloc ID points to a symbolic (not-reified) vtable.
VTable(Ty<'tcx>, Option<ty::PolyExistentialTraitRef<'tcx>>),
VTable(Ty<'tcx>, &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>),
/// The alloc ID points to a "lazy" static variable that did not get computed (yet).
/// This is also used to break the cycle in recursive statics.
Static(DefId),
Expand Down Expand Up @@ -293,7 +292,7 @@ impl<'tcx> GlobalAlloc<'tcx> {
#[inline]
pub fn unwrap_vtable(&self) -> (Ty<'tcx>, Option<ty::PolyExistentialTraitRef<'tcx>>) {
match *self {
GlobalAlloc::VTable(ty, poly_trait_ref) => (ty, poly_trait_ref),
GlobalAlloc::VTable(ty, vtable) => (ty, vtable.principal()),
_ => bug!("expected vtable, got {:?}", self),
}
}
Expand Down Expand Up @@ -398,7 +397,7 @@ impl<'tcx> TyCtxt<'tcx> {
pub fn reserve_and_set_vtable_alloc(
self,
ty: Ty<'tcx>,
poly_trait_ref: Option<ty::PolyExistentialTraitRef<'tcx>>,
poly_trait_ref: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
salt: usize,
) -> AllocId {
self.reserve_and_set_dedup(GlobalAlloc::VTable(ty, poly_trait_ref), salt)
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1536,11 +1536,8 @@ pub fn write_allocations<'tcx>(
// gracefully handle it and allow buggy rustc to be debugged via allocation printing.
None => write!(w, " (deallocated)")?,
Some(GlobalAlloc::Function { instance, .. }) => write!(w, " (fn: {instance})")?,
Some(GlobalAlloc::VTable(ty, Some(trait_ref))) => {
write!(w, " (vtable: impl {trait_ref} for {ty})")?
}
Some(GlobalAlloc::VTable(ty, None)) => {
write!(w, " (vtable: impl <auto trait> for {ty})")?
Some(GlobalAlloc::VTable(ty, vtable)) => {
write!(w, " (vtable: impl {vtable} for {ty})")?
}
Some(GlobalAlloc::Static(did)) if !tcx.is_foreign_item(did) => {
match tcx.eval_static_initializer(did) {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1175,8 +1175,8 @@ fn collect_alloc<'tcx>(tcx: TyCtxt<'tcx>, alloc_id: AllocId, output: &mut MonoIt
output.push(create_fn_mono_item(tcx, instance, DUMMY_SP));
}
}
GlobalAlloc::VTable(ty, trait_ref) => {
let alloc_id = tcx.vtable_allocation((ty, trait_ref));
GlobalAlloc::VTable(ty, vtable) => {
let alloc_id = tcx.vtable_allocation((ty, vtable.principal()));
collect_alloc(tcx, alloc_id, output)
}
}
Expand Down
Loading

0 comments on commit ffa70dd

Please sign in to comment.