Skip to content

Commit

Permalink
Rollup merge of #127859 - RalfJung:ptr-dyn-metadata, r=scottmcm
Browse files Browse the repository at this point in the history
ptr::metadata: avoid references to extern types

References to `extern types` are somewhat dubious entities, since generally we say that references must be dereferenceable for their size as determined via `size_of_val`, but with `extern type` that is an ill-defined statement. I'd like to make Miri warn for such cases since it interacts poorly with Stacked Borrows. To avoid warnings people can't fix, this requires not using references to `extern type` in the standard library, and I think `DynMetadata` is the only currently remaining use. so this changes `DynMetadata` to use a NonNull raw pointer instead. Given that the alignment was 1, this shouldn't really change anything meaningful.

I also updated a comment added by `@scottmcm` in #125479, since I think the old comment is wrong. The `DynMetadata` type itself is not special, it is a normal aggregate. But computing field types for wide pointers (including references) is special.
  • Loading branch information
tgross35 authored Jul 18, 2024
2 parents 3c4f820 + f9c0d33 commit c36a39c
Showing 1 changed file with 10 additions and 6 deletions.
16 changes: 10 additions & 6 deletions library/core/src/ptr/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::hash::{Hash, Hasher};
use crate::intrinsics::aggregate_raw_ptr;
use crate::intrinsics::ptr_metadata;
use crate::marker::Freeze;
use crate::ptr::NonNull;

/// Provides the pointer metadata type of any pointed-to type.
///
Expand Down Expand Up @@ -153,7 +154,7 @@ pub const fn from_raw_parts_mut<T: ?Sized>(
/// compare equal (since identical vtables can be deduplicated within a codegen unit).
#[lang = "dyn_metadata"]
pub struct DynMetadata<Dyn: ?Sized> {
_vtable_ptr: &'static VTable,
_vtable_ptr: NonNull<VTable>,
_phantom: crate::marker::PhantomData<Dyn>,
}

Expand All @@ -166,15 +167,18 @@ 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.
/// When `DynMetadata` appears as the metadata field of a wide pointer, the rustc_middle layout
/// computation does magic and the resulting layout is *not* a `FieldsShape::Aggregate`, instead
/// it is a `FieldsShape::Primitive`. This means that the same type can have different layout
/// depending on whether it appears as the metadata field of a wide pointer or as a stand-alone
/// type, which understandably confuses codegen and leads to ICEs when trying to project to a
/// field of `DynMetadata`. To work around that issue, we use `transmute` instead of using a
/// field projection.
#[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) }
unsafe { crate::mem::transmute::<Self, *const VTable>(self) }
}

/// Returns the size of the type associated with this vtable.
Expand Down

0 comments on commit c36a39c

Please sign in to comment.