Skip to content

Commit

Permalink
Auto merge of rust-lang#125479 - scottmcm:validate-vtable-projections…
Browse files Browse the repository at this point in the history
…, r=Nilstrieb

Validate the special layout restriction on `DynMetadata`

If you look at <https://stdrs.dev/nightly/x86_64-unknown-linux-gnu/std/ptr/struct.DynMetadata.html>, you'd think that `DynMetadata` is a struct with fields.

But it's actually not, because the lang item is special-cased in rustc_middle layout:

https://github.com/rust-lang/rust/blob/7601adcc764d42c9f2984082b49948af652df986/compiler/rustc_middle/src/ty/layout.rs#L861-L864

That explains the very confusing codegen ICEs I was getting in rust-lang#124251 (comment)

> Tried to extract_field 0 from primitive OperandRef(Immediate((ptr:  %5 = load ptr, ptr %4, align 8, !nonnull !3, !align !5, !noundef !3)) @ TyAndLayout { ty: DynMetadata<dyn Callsite>, layout: Layout { size: Size(8 bytes), align: AbiAndPrefAlign { abi: Align(8 bytes), pref: Align(8 bytes) }, abi: Scalar(Initialized { value: Pointer(AddressSpace(0)), valid_range: 1..=18446744073709551615 }), fields: Primitive, largest_niche: Some(Niche { offset: Size(0 bytes), value: Pointer(AddressSpace(0)), valid_range: 1..=18446744073709551615 }), variants: Single { index: 0 }, max_repr_align: None, unadjusted_abi_align: Align(8 bytes) } })

because there was a `Field` projection despite the layout clearly saying it's [`Primitive`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_target/abi/enum.FieldsShape.html#variant.Primitive).

Thus this PR updates the MIR validator to check for such a projection, and changes `libcore` to not ever emit any projections into `DynMetadata`, just to transmute the whole thing when it wants a pointer.
  • Loading branch information
bors committed May 24, 2024
2 parents 7c54789 + d83f3ca commit 4649877
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
9 changes: 9 additions & 0 deletions compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
check_equal(self, location, *f_ty);
}
ty::Adt(adt_def, args) => {
// see <https://github.com/rust-lang/rust/blob/7601adcc764d42c9f2984082b49948af652df986/compiler/rustc_middle/src/ty/layout.rs#L861-L864>
if Some(adt_def.did()) == self.tcx.lang_items().dyn_metadata() {
self.fail(
location,
format!("You can't project to field {f:?} of `DynMetadata` because \
layout is weird and thinks it doesn't have fields."),
);
}

let var = parent_ty.variant_index.unwrap_or(FIRST_VARIANT);
let Some(field) = adt_def.variant(var).fields.get(f) else {
fail_out_of_bounds(self, location);
Expand Down
27 changes: 19 additions & 8 deletions library/core/src/ptr/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ impl<T: ?Sized> Clone for PtrComponents<T> {
/// compare equal (since identical vtables can be deduplicated within a codegen unit).
#[lang = "dyn_metadata"]
pub struct DynMetadata<Dyn: ?Sized> {
vtable_ptr: &'static VTable,
phantom: crate::marker::PhantomData<Dyn>,
_vtable_ptr: &'static VTable,
_phantom: crate::marker::PhantomData<Dyn>,
}

extern "C" {
Expand All @@ -191,6 +191,17 @@ extern "C" {
}

impl<Dyn: ?Sized> DynMetadata<Dyn> {
/// One of the things that rustc_middle does with this being a lang item is
/// give it `FieldsShape::Primitive`, which means that as far as codegen can
/// tell, it *is* a reference, and thus doesn't have any fields.
/// That means we can't use field access, and have to transmute it instead.
#[inline]
fn vtable_ptr(self) -> *const VTable {
// SAFETY: this layout assumption is hard-coded into the compiler.
// If it's somehow not a size match, the transmute will error.
unsafe { crate::mem::transmute::<Self, &'static VTable>(self) }
}

/// Returns the size of the type associated with this vtable.
#[inline]
pub fn size_of(self) -> usize {
Expand All @@ -199,7 +210,7 @@ impl<Dyn: ?Sized> DynMetadata<Dyn> {
// `Send` part!
// SAFETY: DynMetadata always contains a valid vtable pointer
return unsafe {
crate::intrinsics::vtable_size(self.vtable_ptr as *const VTable as *const ())
crate::intrinsics::vtable_size(self.vtable_ptr() as *const ())
};
}

Expand All @@ -208,7 +219,7 @@ impl<Dyn: ?Sized> DynMetadata<Dyn> {
pub fn align_of(self) -> usize {
// SAFETY: DynMetadata always contains a valid vtable pointer
return unsafe {
crate::intrinsics::vtable_align(self.vtable_ptr as *const VTable as *const ())
crate::intrinsics::vtable_align(self.vtable_ptr() as *const ())
};
}

Expand All @@ -226,7 +237,7 @@ unsafe impl<Dyn: ?Sized> Sync for DynMetadata<Dyn> {}

impl<Dyn: ?Sized> fmt::Debug for DynMetadata<Dyn> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_tuple("DynMetadata").field(&(self.vtable_ptr as *const VTable)).finish()
f.debug_tuple("DynMetadata").field(&self.vtable_ptr()).finish()
}
}

Expand All @@ -248,15 +259,15 @@ impl<Dyn: ?Sized> Eq for DynMetadata<Dyn> {}
impl<Dyn: ?Sized> PartialEq for DynMetadata<Dyn> {
#[inline]
fn eq(&self, other: &Self) -> bool {
crate::ptr::eq::<VTable>(self.vtable_ptr, other.vtable_ptr)
crate::ptr::eq::<VTable>(self.vtable_ptr(), other.vtable_ptr())
}
}

impl<Dyn: ?Sized> Ord for DynMetadata<Dyn> {
#[inline]
#[allow(ambiguous_wide_pointer_comparisons)]
fn cmp(&self, other: &Self) -> crate::cmp::Ordering {
(self.vtable_ptr as *const VTable).cmp(&(other.vtable_ptr as *const VTable))
<*const VTable>::cmp(&self.vtable_ptr(), &other.vtable_ptr())
}
}

Expand All @@ -270,6 +281,6 @@ impl<Dyn: ?Sized> PartialOrd for DynMetadata<Dyn> {
impl<Dyn: ?Sized> Hash for DynMetadata<Dyn> {
#[inline]
fn hash<H: Hasher>(&self, hasher: &mut H) {
crate::ptr::hash::<VTable, _>(self.vtable_ptr, hasher)
crate::ptr::hash::<VTable, _>(self.vtable_ptr(), hasher)
}
}

0 comments on commit 4649877

Please sign in to comment.