Skip to content

Commit a7b8081

Browse files
RalfJungRalfJung
committed
interpret: add sanity check in dyn upcast to double-check what codegen does
1 parent 4cd8dc6 commit a7b8081

15 files changed

+228
-237
lines changed

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

+36-5
Original file line numberDiff line numberDiff line change
@@ -401,15 +401,46 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
401401
}
402402
(ty::Dynamic(data_a, _, ty::Dyn), ty::Dynamic(data_b, _, ty::Dyn)) => {
403403
let val = self.read_immediate(src)?;
404-
if data_a.principal() == data_b.principal() {
405-
// A NOP cast that doesn't actually change anything, should be allowed even with mismatching vtables.
406-
// (But currently mismatching vtables violate the validity invariant so UB is triggered anyway.)
407-
return self.write_immediate(*val, dest);
408-
}
404+
// Take apart the old pointer, and find the dynamic type.
409405
let (old_data, old_vptr) = val.to_scalar_pair();
410406
let old_data = old_data.to_pointer(self)?;
411407
let old_vptr = old_vptr.to_pointer(self)?;
412408
let ty = self.get_ptr_vtable_ty(old_vptr, Some(data_a))?;
409+
410+
// Sanity-check that `supertrait_vtable_slot` in this type's vtable indeed produces
411+
// our destination trait.
412+
if cfg!(debug_assertions) {
413+
let vptr_entry_idx =
414+
self.tcx.supertrait_vtable_slot((src_pointee_ty, dest_pointee_ty));
415+
let vtable_entries = self.vtable_entries(data_a.principal(), ty);
416+
if let Some(entry_idx) = vptr_entry_idx {
417+
let Some(&ty::VtblEntry::TraitVPtr(upcast_trait_ref)) =
418+
vtable_entries.get(entry_idx)
419+
else {
420+
span_bug!(
421+
self.cur_span(),
422+
"invalid vtable entry index in {} -> {} upcast",
423+
src_pointee_ty,
424+
dest_pointee_ty
425+
);
426+
};
427+
let erased_trait_ref = upcast_trait_ref
428+
.map_bound(|r| ty::ExistentialTraitRef::erase_self_ty(*self.tcx, r));
429+
assert!(
430+
data_b
431+
.principal()
432+
.is_some_and(|b| self.eq_in_param_env(erased_trait_ref, b))
433+
);
434+
} else {
435+
// In this case codegen would keep using the old vtable. We don't want to do
436+
// that as it has the wrong trait. The reason codegen can do this is that
437+
// one vtable is a prefix of the other, so we double-check that.
438+
let vtable_entries_b = self.vtable_entries(data_b.principal(), ty);
439+
assert!(&vtable_entries[..vtable_entries_b.len()] == vtable_entries_b);
440+
};
441+
}
442+
443+
// Get the destination trait vtable and return that.
413444
let new_vptr = self.get_vtable_ptr(ty, data_b.principal())?;
414445
self.write_immediate(Immediate::new_dyn_trait(old_data, new_vptr, self), dest)
415446
}

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

+30
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@ use std::cell::Cell;
22
use std::{fmt, mem};
33

44
use either::{Either, Left, Right};
5+
use rustc_infer::infer::at::ToTrace;
6+
use rustc_infer::traits::ObligationCause;
7+
use rustc_trait_selection::traits::ObligationCtxt;
58
use tracing::{debug, info, info_span, instrument, trace};
69

710
use rustc_errors::DiagCtxtHandle;
811
use rustc_hir::{self as hir, def_id::DefId, definitions::DefPathData};
912
use rustc_index::IndexVec;
13+
use rustc_infer::infer::TyCtxtInferExt;
1014
use rustc_middle::mir;
1115
use rustc_middle::mir::interpret::{
1216
CtfeProvenance, ErrorHandled, InvalidMetaKind, ReportedErrorInfo,
@@ -640,6 +644,32 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
640644
}
641645
}
642646

647+
/// Check if the two things are equal in the current param_env, using an infctx to get proper
648+
/// equality checks.
649+
pub(super) fn eq_in_param_env<T>(&self, a: T, b: T) -> bool
650+
where
651+
T: PartialEq + TypeFoldable<TyCtxt<'tcx>> + ToTrace<'tcx>,
652+
{
653+
// Fast path: compare directly.
654+
if a == b {
655+
return true;
656+
}
657+
// Slow path: spin up an inference context to check if these traits are sufficiently equal.
658+
let infcx = self.tcx.infer_ctxt().build();
659+
let ocx = ObligationCtxt::new(&infcx);
660+
let cause = ObligationCause::dummy_with_span(self.cur_span());
661+
// equate the two trait refs after normalization
662+
let a = ocx.normalize(&cause, self.param_env, a);
663+
let b = ocx.normalize(&cause, self.param_env, b);
664+
if ocx.eq(&cause, self.param_env, a, b).is_ok() {
665+
if ocx.select_all_or_error().is_empty() {
666+
// All good.
667+
return true;
668+
}
669+
}
670+
return false;
671+
}
672+
643673
/// Walks up the callstack from the intrinsic's callsite, searching for the first callsite in a
644674
/// frame which is not `#[track_caller]`. This matches the `caller_location` intrinsic,
645675
/// and is primarily intended for the panic machinery.

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

+6-11
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use std::borrow::Cow;
22

33
use either::Either;
4-
use rustc_middle::ty::TyCtxt;
54
use tracing::trace;
65

76
use rustc_middle::{
@@ -867,7 +866,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
867866
};
868867

869868
// Obtain the underlying trait we are working on, and the adjusted receiver argument.
870-
let (dyn_trait, dyn_ty, adjusted_recv) = if let ty::Dynamic(data, _, ty::DynStar) =
869+
let (trait_, dyn_ty, adjusted_recv) = if let ty::Dynamic(data, _, ty::DynStar) =
871870
receiver_place.layout.ty.kind()
872871
{
873872
let recv = self.unpack_dyn_star(&receiver_place, data)?;
@@ -898,20 +897,16 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
898897
(receiver_trait.principal(), dyn_ty, receiver_place.ptr())
899898
};
900899

901-
// Now determine the actual method to call. We can do that in two different ways and
902-
// compare them to ensure everything fits.
903-
let vtable_entries = if let Some(dyn_trait) = dyn_trait {
904-
let trait_ref = dyn_trait.with_self_ty(*self.tcx, dyn_ty);
905-
let trait_ref = self.tcx.erase_regions(trait_ref);
906-
self.tcx.vtable_entries(trait_ref)
907-
} else {
908-
TyCtxt::COMMON_VTABLE_ENTRIES
909-
};
900+
// Now determine the actual method to call. Usually we use the easy way of just
901+
// looking up the method at index `idx`.
902+
let vtable_entries = self.vtable_entries(trait_, dyn_ty);
910903
let Some(ty::VtblEntry::Method(fn_inst)) = vtable_entries.get(idx).copied() else {
911904
// FIXME(fee1-dead) these could be variants of the UB info enum instead of this
912905
throw_ub_custom!(fluent::const_eval_dyn_call_not_a_method);
913906
};
914907
trace!("Virtual call dispatches to {fn_inst:#?}");
908+
// We can also do the lookup based on `def_id` and `dyn_ty`, and check that that
909+
// produces the same result.
915910
if cfg!(debug_assertions) {
916911
let tcx = *self.tcx;
917912

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

+23-25
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
use rustc_infer::infer::TyCtxtInferExt;
2-
use rustc_infer::traits::ObligationCause;
31
use rustc_middle::mir::interpret::{InterpResult, Pointer};
42
use rustc_middle::ty::layout::LayoutOf;
5-
use rustc_middle::ty::{self, Ty};
3+
use rustc_middle::ty::{self, Ty, TyCtxt, VtblEntry};
64
use rustc_target::abi::{Align, Size};
7-
use rustc_trait_selection::traits::ObligationCtxt;
85
use tracing::trace;
96

107
use super::util::ensure_monomorphic_enough;
@@ -47,35 +44,36 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
4744
Ok((layout.size, layout.align.abi))
4845
}
4946

47+
pub(super) fn vtable_entries(
48+
&self,
49+
trait_: Option<ty::PolyExistentialTraitRef<'tcx>>,
50+
dyn_ty: Ty<'tcx>,
51+
) -> &'tcx [VtblEntry<'tcx>] {
52+
if let Some(trait_) = trait_ {
53+
let trait_ref = trait_.with_self_ty(*self.tcx, dyn_ty);
54+
let trait_ref = self.tcx.erase_regions(trait_ref);
55+
self.tcx.vtable_entries(trait_ref)
56+
} else {
57+
TyCtxt::COMMON_VTABLE_ENTRIES
58+
}
59+
}
60+
5061
/// Check that the given vtable trait is valid for a pointer/reference/place with the given
5162
/// expected trait type.
5263
pub(super) fn check_vtable_for_type(
5364
&self,
5465
vtable_trait: Option<ty::PolyExistentialTraitRef<'tcx>>,
5566
expected_trait: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
5667
) -> InterpResult<'tcx> {
57-
// Fast path: if they are equal, it's all fine.
58-
if expected_trait.principal() == vtable_trait {
59-
return Ok(());
60-
}
61-
if let (Some(expected_trait), Some(vtable_trait)) =
62-
(expected_trait.principal(), vtable_trait)
63-
{
64-
// Slow path: spin up an inference context to check if these traits are sufficiently equal.
65-
let infcx = self.tcx.infer_ctxt().build();
66-
let ocx = ObligationCtxt::new(&infcx);
67-
let cause = ObligationCause::dummy_with_span(self.cur_span());
68-
// equate the two trait refs after normalization
69-
let expected_trait = ocx.normalize(&cause, self.param_env, expected_trait);
70-
let vtable_trait = ocx.normalize(&cause, self.param_env, vtable_trait);
71-
if ocx.eq(&cause, self.param_env, expected_trait, vtable_trait).is_ok() {
72-
if ocx.select_all_or_error().is_empty() {
73-
// All good.
74-
return Ok(());
75-
}
76-
}
68+
let eq = match (expected_trait.principal(), vtable_trait) {
69+
(Some(a), Some(b)) => self.eq_in_param_env(a, b),
70+
(None, None) => true,
71+
_ => false,
72+
};
73+
if !eq {
74+
throw_ub!(InvalidVTableTrait { expected_trait, vtable_trait });
7775
}
78-
throw_ub!(InvalidVTableTrait { expected_trait, vtable_trait });
76+
Ok(())
7977
}
8078

8179
/// Turn a place with a `dyn Trait` type into a place with the actual dynamic type.

‎compiler/rustc_trait_selection/src/traits/vtable.rs

+12-8
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,9 @@ pub(crate) fn first_method_vtable_slot<'tcx>(tcx: TyCtxt<'tcx>, key: ty::TraitRe
364364
}
365365

366366
/// Given a `dyn Subtrait` and `dyn Supertrait` trait object, find the slot of
367-
/// // the trait vptr in the subtrait's vtable.
367+
/// the trait vptr in the subtrait's vtable.
368+
///
369+
/// A return value of `None` means that the original vtable can be reused.
368370
pub(crate) fn supertrait_vtable_slot<'tcx>(
369371
tcx: TyCtxt<'tcx>,
370372
key: (
@@ -373,20 +375,22 @@ pub(crate) fn supertrait_vtable_slot<'tcx>(
373375
),
374376
) -> Option<usize> {
375377
debug_assert!(!key.has_non_region_infer() && !key.has_non_region_param());
376-
377378
let (source, target) = key;
378-
let ty::Dynamic(source, _, _) = *source.kind() else {
379+
380+
// If the target principal is `None`, we can just return `None`.
381+
let ty::Dynamic(target, _, _) = *target.kind() else {
379382
bug!();
380383
};
381-
let source_principal = tcx
382-
.normalize_erasing_regions(ty::ParamEnv::reveal_all(), source.principal().unwrap())
384+
let target_principal = tcx
385+
.normalize_erasing_regions(ty::ParamEnv::reveal_all(), target.principal()?)
383386
.with_self_ty(tcx, tcx.types.trait_object_dummy_self);
384387

385-
let ty::Dynamic(target, _, _) = *target.kind() else {
388+
// Given that we have a target principal, it is a bug for there not to be a source principal.
389+
let ty::Dynamic(source, _, _) = *source.kind() else {
386390
bug!();
387391
};
388-
let target_principal = tcx
389-
.normalize_erasing_regions(ty::ParamEnv::reveal_all(), target.principal().unwrap())
392+
let source_principal = tcx
393+
.normalize_erasing_regions(ty::ParamEnv::reveal_all(), source.principal().unwrap())
390394
.with_self_ty(tcx, tcx.types.trait_object_dummy_self);
391395

392396
let vtable_segment_callback = {

‎src/tools/miri/tests/fail/dyn-upcast-trait-mismatch.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,10 @@ impl Baz for i32 {
5959
}
6060

6161
fn main() {
62-
let baz: &dyn Baz = &1;
63-
let baz_fake: *const dyn Bar = unsafe { std::mem::transmute(baz) };
64-
let _err = baz_fake as *const dyn Foo;
65-
//~^ERROR: using vtable for trait `Baz` but trait `Bar` was expected
62+
unsafe {
63+
let baz: &dyn Baz = &1;
64+
let baz_fake: *const dyn Bar = std::mem::transmute(baz);
65+
let _err = baz_fake as *const dyn Foo;
66+
//~^ERROR: using vtable for trait `Baz` but trait `Bar` was expected
67+
}
6668
}

‎src/tools/miri/tests/fail/dyn-upcast-trait-mismatch.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: Undefined Behavior: using vtable for trait `Baz` but trait `Bar` was expected
22
--> $DIR/dyn-upcast-trait-mismatch.rs:LL:CC
33
|
4-
LL | let _err = baz_fake as *const dyn Foo;
5-
| ^^^^^^^^ using vtable for trait `Baz` but trait `Bar` was expected
4+
LL | let _err = baz_fake as *const dyn Foo;
5+
| ^^^^^^^^ using vtable for trait `Baz` but trait `Bar` was expected
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

‎tests/ui/consts/const-eval/raw-bytes.32bit.stderr

+16-26
Original file line numberDiff line numberDiff line change
@@ -436,30 +436,20 @@ LL | const TRAIT_OBJ_CONTENT_INVALID: &dyn Trait = unsafe { mem::transmute::<_,
436436
╾ALLOC_ID╼ ╾ALLOC_ID╼ │ ╾──╼╾──╼
437437
}
438438

439-
error[E0080]: it is undefined behavior to use this value
440-
--> $DIR/raw-bytes.rs:196:1
439+
error[E0080]: evaluation of constant value failed
440+
--> $DIR/raw-bytes.rs:196:62
441441
|
442442
LL | const RAW_TRAIT_OBJ_VTABLE_NULL: *const dyn Trait = unsafe { mem::transmute((&92u8, 0usize)) };
443-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered null pointer, but expected a vtable pointer
444-
|
445-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
446-
= note: the raw bytes of the constant (size: 8, align: 4) {
447-
╾ALLOC_ID╼ 00 00 00 00 │ ╾──╼....
448-
}
443+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
449444

450-
error[E0080]: it is undefined behavior to use this value
451-
--> $DIR/raw-bytes.rs:198:1
445+
error[E0080]: evaluation of constant value failed
446+
--> $DIR/raw-bytes.rs:199:65
452447
|
453448
LL | const RAW_TRAIT_OBJ_VTABLE_INVALID: *const dyn Trait = unsafe { mem::transmute((&92u8, &3u64)) };
454-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered ALLOC27<imm>, but expected a vtable pointer
455-
|
456-
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
457-
= note: the raw bytes of the constant (size: 8, align: 4) {
458-
╾ALLOC_ID╼ ╾ALLOC_ID╼ │ ╾──╼╾──╼
459-
}
449+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ using ALLOC32 as vtable pointer but it does not point to a vtable
460450

461451
error[E0080]: it is undefined behavior to use this value
462-
--> $DIR/raw-bytes.rs:202:1
452+
--> $DIR/raw-bytes.rs:204:1
463453
|
464454
LL | const _: &[!; 1] = unsafe { &*(1_usize as *const [!; 1]) };
465455
| ^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to uninhabited type [!; 1]
@@ -470,7 +460,7 @@ LL | const _: &[!; 1] = unsafe { &*(1_usize as *const [!; 1]) };
470460
}
471461

472462
error[E0080]: it is undefined behavior to use this value
473-
--> $DIR/raw-bytes.rs:203:1
463+
--> $DIR/raw-bytes.rs:205:1
474464
|
475465
LL | const _: &[!] = unsafe { &*(1_usize as *const [!; 1]) };
476466
| ^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered a value of the never type `!`
@@ -481,7 +471,7 @@ LL | const _: &[!] = unsafe { &*(1_usize as *const [!; 1]) };
481471
}
482472

483473
error[E0080]: it is undefined behavior to use this value
484-
--> $DIR/raw-bytes.rs:204:1
474+
--> $DIR/raw-bytes.rs:206:1
485475
|
486476
LL | const _: &[!] = unsafe { &*(1_usize as *const [!; 42]) };
487477
| ^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered a value of the never type `!`
@@ -492,7 +482,7 @@ LL | const _: &[!] = unsafe { &*(1_usize as *const [!; 42]) };
492482
}
493483

494484
error[E0080]: it is undefined behavior to use this value
495-
--> $DIR/raw-bytes.rs:208:1
485+
--> $DIR/raw-bytes.rs:210:1
496486
|
497487
LL | pub static S4: &[u8] = unsafe { from_raw_parts((&D1) as *const _ as _, 1) };
498488
| ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered uninitialized memory, but expected an integer
@@ -503,7 +493,7 @@ LL | pub static S4: &[u8] = unsafe { from_raw_parts((&D1) as *const _ as _, 1) }
503493
}
504494

505495
error[E0080]: it is undefined behavior to use this value
506-
--> $DIR/raw-bytes.rs:211:1
496+
--> $DIR/raw-bytes.rs:213:1
507497
|
508498
LL | pub static S5: &[u8] = unsafe { from_raw_parts((&D3) as *const _ as _, mem::size_of::<&u32>()) };
509499
| ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered a pointer, but expected an integer
@@ -516,7 +506,7 @@ LL | pub static S5: &[u8] = unsafe { from_raw_parts((&D3) as *const _ as _, mem:
516506
= help: the absolute address of a pointer is not known at compile-time, so such operations are not supported
517507

518508
error[E0080]: it is undefined behavior to use this value
519-
--> $DIR/raw-bytes.rs:214:1
509+
--> $DIR/raw-bytes.rs:216:1
520510
|
521511
LL | pub static S6: &[bool] = unsafe { from_raw_parts((&D0) as *const _ as _, 4) };
522512
| ^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered 0x11, but expected a boolean
@@ -527,7 +517,7 @@ LL | pub static S6: &[bool] = unsafe { from_raw_parts((&D0) as *const _ as _, 4)
527517
}
528518

529519
error[E0080]: it is undefined behavior to use this value
530-
--> $DIR/raw-bytes.rs:218:1
520+
--> $DIR/raw-bytes.rs:220:1
531521
|
532522
LL | pub static S7: &[u16] = unsafe {
533523
| ^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[1]: encountered uninitialized memory, but expected an integer
@@ -538,7 +528,7 @@ LL | pub static S7: &[u16] = unsafe {
538528
}
539529

540530
error[E0080]: it is undefined behavior to use this value
541-
--> $DIR/raw-bytes.rs:225:1
531+
--> $DIR/raw-bytes.rs:227:1
542532
|
543533
LL | pub static R4: &[u8] = unsafe {
544534
| ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered uninitialized memory, but expected an integer
@@ -549,7 +539,7 @@ LL | pub static R4: &[u8] = unsafe {
549539
}
550540

551541
error[E0080]: it is undefined behavior to use this value
552-
--> $DIR/raw-bytes.rs:230:1
542+
--> $DIR/raw-bytes.rs:232:1
553543
|
554544
LL | pub static R5: &[u8] = unsafe {
555545
| ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered a pointer, but expected an integer
@@ -562,7 +552,7 @@ LL | pub static R5: &[u8] = unsafe {
562552
= help: the absolute address of a pointer is not known at compile-time, so such operations are not supported
563553

564554
error[E0080]: it is undefined behavior to use this value
565-
--> $DIR/raw-bytes.rs:235:1
555+
--> $DIR/raw-bytes.rs:237:1
566556
|
567557
LL | pub static R6: &[bool] = unsafe {
568558
| ^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered 0x11, but expected a boolean

0 commit comments

Comments
 (0)