Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safe Transmute: Require that source referent is smaller than destination #122438

Merged
merged 1 commit into from
Mar 14, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3091,6 +3091,13 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
rustc_transmute::Reason::DstIsTooBig => {
format!("The size of `{src}` is smaller than the size of `{dst}`")
}
rustc_transmute::Reason::DstRefIsTooBig { src, dst } => {
let src_size = src.size;
let dst_size = dst.size;
format!(
"The referent size of `{src}` ({src_size} bytes) is smaller than that of `{dst}` ({dst_size} bytes)"
)
}
rustc_transmute::Reason::SrcSizeOverflow => {
format!(
"values of the type `{src}` are too big for the current architecture"
Expand Down
21 changes: 21 additions & 0 deletions compiler/rustc_transmute/src/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ pub(crate) trait Def: Debug + Hash + Eq + PartialEq + Copy + Clone {
pub trait Ref: Debug + Hash + Eq + PartialEq + Copy + Clone {
fn min_align(&self) -> usize;

fn size(&self) -> usize;

fn is_mutable(&self) -> bool;
}

Expand All @@ -48,6 +50,9 @@ impl Ref for ! {
fn min_align(&self) -> usize {
unreachable!()
}
fn size(&self) -> usize {
unreachable!()
}
fn is_mutable(&self) -> bool {
unreachable!()
}
Expand All @@ -57,6 +62,7 @@ impl Ref for ! {
pub mod rustc {
use rustc_middle::mir::Mutability;
use rustc_middle::ty::{self, Ty};
use std::fmt::{self, Write};

/// A reference in the layout.
#[derive(Debug, Hash, Eq, PartialEq, PartialOrd, Ord, Clone, Copy)]
Expand All @@ -65,13 +71,18 @@ pub mod rustc {
pub ty: Ty<'tcx>,
pub mutability: Mutability,
pub align: usize,
pub size: usize,
}

impl<'tcx> super::Ref for Ref<'tcx> {
fn min_align(&self) -> usize {
self.align
}

fn size(&self) -> usize {
self.size
}

fn is_mutable(&self) -> bool {
match self.mutability {
Mutability::Mut => true,
Expand All @@ -81,6 +92,16 @@ pub mod rustc {
}
impl<'tcx> Ref<'tcx> {}

impl<'tcx> fmt::Display for Ref<'tcx> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_char('&')?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not printing the region here too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but it's not relevant to the error message this Display routine is currently used in, and I don't anticipate it will be relevant to other transmute error messages, since regions do not impact layout.

if self.mutability == Mutability::Mut {
f.write_str("mut ")?;
}
self.ty.fmt(f)
}
}

/// A visibility node in the layout.
#[derive(Debug, Hash, Eq, PartialEq, Clone, Copy)]
pub enum Def<'tcx> {
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_transmute/src/layout/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,12 +372,15 @@ pub(crate) mod rustc {
}

ty::Ref(lifetime, ty, mutability) => {
let align = layout_of(tcx, *ty)?.align();
let layout = layout_of(tcx, *ty)?;
let align = layout.align();
let size = layout.size();
Ok(Tree::Ref(Ref {
lifetime: *lifetime,
ty: *ty,
mutability: *mutability,
align,
size,
}))
}

Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_transmute/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub struct Assume {
#[derive(Debug, Hash, Eq, PartialEq, Clone)]
pub enum Answer<R> {
Yes,
No(Reason),
No(Reason<R>),
If(Condition<R>),
}

Expand All @@ -42,7 +42,7 @@ pub enum Condition<R> {

/// Answers "why wasn't the source type transmutable into the destination type?"
#[derive(Debug, Hash, Eq, PartialEq, PartialOrd, Ord, Clone)]
pub enum Reason {
pub enum Reason<T> {
/// The layout of the source type is unspecified.
SrcIsUnspecified,
/// The layout of the destination type is unspecified.
Expand All @@ -53,6 +53,13 @@ pub enum Reason {
DstMayHaveSafetyInvariants,
/// `Dst` is larger than `Src`, and the excess bytes were not exclusively uninitialized.
DstIsTooBig,
/// A referent of `Dst` is larger than a referent in `Src`.
DstRefIsTooBig {
/// The referent of the source type.
src: T,
/// The too-large referent of the destination type.
dst: T,
},
/// Src should have a stricter alignment than Dst, but it does not.
DstHasStricterAlignment { src_min_align: usize, dst_min_align: usize },
/// Can't go from shared pointer to unique pointer
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_transmute/src/maybe_transmutable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,11 @@ where
src_min_align: src_ref.min_align(),
dst_min_align: dst_ref.min_align(),
})
} else if dst_ref.size() > src_ref.size() {
Answer::No(Reason::DstRefIsTooBig {
src: src_ref,
dst: dst_ref,
})
} else {
// ...such that `src` is transmutable into `dst`, if
// `src_ref` is transmutability into `dst_ref`.
Expand Down
49 changes: 49 additions & 0 deletions tests/ui/transmutability/references/reject_extension.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
//@ check-fail

//! Reject extensions behind references.

#![crate_type = "lib"]
#![feature(transmutability)]

mod assert {
use std::mem::{Assume, BikeshedIntrinsicFrom};

pub fn is_transmutable<Src, Dst>()
where
Dst: BikeshedIntrinsicFrom<
Src,
{
Assume {
alignment: true,
lifetimes: true,
safety: true,
validity: true,
}
},
>,
{
}
}

#[repr(C, packed)]
struct Packed<T>(T);

fn reject_extension() {
#[repr(C, align(2))]
struct Two(u8);

#[repr(C, align(4))]
struct Four(u8);

// These two types differ in the number of trailing padding bytes they have.
type Src = Packed<Two>;
type Dst = Packed<Four>;

const _: () = {
use std::mem::size_of;
assert!(size_of::<Src>() == 2);
assert!(size_of::<Dst>() == 4);
};

assert::is_transmutable::<&Src, &Dst>(); //~ ERROR cannot be safely transmuted
}
25 changes: 25 additions & 0 deletions tests/ui/transmutability/references/reject_extension.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error[E0277]: `&Packed<Two>` cannot be safely transmuted into `&Packed<Four>`
--> $DIR/reject_extension.rs:48:37
|
LL | assert::is_transmutable::<&Src, &Dst>();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that only the second parameter is ^^^^, but that's a general issue with all of our error messages that I'll file a follow-up PR for.

Otherwise, this error message LGTM.

| ^^^^ The referent size of `&Packed<Two>` (2 bytes) is smaller than that of `&Packed<Four>` (4 bytes)
|
note: required by a bound in `is_transmutable`
--> $DIR/reject_extension.rs:13:14
|
LL | pub fn is_transmutable<Src, Dst>()
| --------------- required by a bound in this function
LL | where
LL | Dst: BikeshedIntrinsicFrom<
| ______________^
LL | | Src,
LL | | {
LL | | Assume {
... |
LL | | },
LL | | >,
| |_________^ required by this bound in `is_transmutable`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.
4 changes: 2 additions & 2 deletions tests/ui/transmutability/references/unit-to-u8.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: `Unit` cannot be safely transmuted into `u8`
error[E0277]: `&Unit` cannot be safely transmuted into `&u8`
--> $DIR/unit-to-u8.rs:22:52
|
LL | assert::is_maybe_transmutable::<&'static Unit, &'static u8>();
| ^^^^^^^^^^^ The size of `Unit` is smaller than the size of `u8`
| ^^^^^^^^^^^ The referent size of `&Unit` (0 bytes) is smaller than that of `&u8` (1 bytes)
|
note: required by a bound in `is_maybe_transmutable`
--> $DIR/unit-to-u8.rs:9:14
Expand Down