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: Revise safety analysis #121681

Merged
merged 1 commit into from
Mar 1, 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
2 changes: 1 addition & 1 deletion compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ language_item_table! {

// language items relating to transmutability
TransmuteOpts, sym::transmute_opts, transmute_opts, Target::Struct, GenericRequirement::Exact(0);
TransmuteTrait, sym::transmute_trait, transmute_trait, Target::Trait, GenericRequirement::Exact(3);
TransmuteTrait, sym::transmute_trait, transmute_trait, Target::Trait, GenericRequirement::Exact(2);

Add, sym::add, add_trait, Target::Trait, GenericRequirement::Exact(1);
Sub, sym::sub, sub_trait, Target::Trait, GenericRequirement::Exact(1);
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -874,15 +874,13 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
pub(super) fn is_transmutable(
&self,
src_and_dst: rustc_transmute::Types<'tcx>,
scope: Ty<'tcx>,
assume: rustc_transmute::Assume,
) -> Result<Certainty, NoSolution> {
use rustc_transmute::Answer;
// FIXME(transmutability): This really should be returning nested goals for `Answer::If*`
match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable(
ObligationCause::dummy(),
src_and_dst,
scope,
assume,
) {
Answer::Yes => Ok(Certainty::Yes),
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,14 +543,13 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
let args = ecx.tcx().erase_regions(goal.predicate.trait_ref.args);

let Some(assume) =
rustc_transmute::Assume::from_const(ecx.tcx(), goal.param_env, args.const_at(3))
rustc_transmute::Assume::from_const(ecx.tcx(), goal.param_env, args.const_at(2))
else {
return Err(NoSolution);
};

let certainty = ecx.is_transmutable(
rustc_transmute::Types { dst: args.type_at(0), src: args.type_at(1) },
args.type_at(2),
assume,
)?;
ecx.evaluate_added_goals_and_make_canonical_response(certainty)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2970,11 +2970,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
dst: trait_ref.args.type_at(0),
src: trait_ref.args.type_at(1),
};
let scope = trait_ref.args.type_at(2);
let Some(assume) = rustc_transmute::Assume::from_const(
self.infcx.tcx,
obligation.param_env,
trait_ref.args.const_at(3),
trait_ref.args.const_at(2),
) else {
self.dcx().span_delayed_bug(
span,
Expand All @@ -2986,15 +2985,12 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable(
obligation.cause,
src_and_dst,
scope,
assume,
) {
Answer::No(reason) => {
let dst = trait_ref.args.type_at(0);
let src = trait_ref.args.type_at(1);
let err_msg = format!(
"`{src}` cannot be safely transmuted into `{dst}` in the defining scope of `{scope}`"
);
let err_msg = format!("`{src}` cannot be safely transmuted into `{dst}`");
let safe_transmute_explanation = match reason {
rustc_transmute::Reason::SrcIsUnspecified => {
format!("`{src}` does not have a well-specified layout")
Expand All @@ -3008,9 +3004,9 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
format!("At least one value of `{src}` isn't a bit-valid value of `{dst}`")
}

rustc_transmute::Reason::DstIsPrivate => format!(
"`{dst}` is or contains a type or field that is not visible in that scope"
),
rustc_transmute::Reason::DstMayHaveSafetyInvariants => {
format!("`{dst}` may carry safety invariants")
}
rustc_transmute::Reason::DstIsTooBig => {
format!("The size of `{src}` is smaller than the size of `{dst}`")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,16 +310,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
.collect(),
Condition::IfTransmutable { src, dst } => {
let trait_def_id = obligation.predicate.def_id();
let scope = predicate.trait_ref.args.type_at(2);
let assume_const = predicate.trait_ref.args.const_at(3);
let assume_const = predicate.trait_ref.args.const_at(2);
let make_obl = |from_ty, to_ty| {
let trait_ref1 = ty::TraitRef::new(
tcx,
trait_def_id,
[
ty::GenericArg::from(to_ty),
ty::GenericArg::from(from_ty),
ty::GenericArg::from(scope),
ty::GenericArg::from(assume_const),
],
);
Expand Down Expand Up @@ -355,7 +353,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let Some(assume) = rustc_transmute::Assume::from_const(
self.infcx.tcx,
obligation.param_env,
predicate.trait_ref.args.const_at(3),
predicate.trait_ref.args.const_at(2),
) else {
return Err(Unimplemented);
};
Expand All @@ -367,7 +365,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let maybe_transmutable = transmute_env.is_transmutable(
obligation.cause.clone(),
rustc_transmute::Types { dst, src },
predicate.trait_ref.args.type_at(2),
assume,
);

Expand Down
20 changes: 17 additions & 3 deletions compiler/rustc_transmute/src/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,21 @@ impl fmt::Debug for Byte {
}
}

pub(crate) trait Def: Debug + Hash + Eq + PartialEq + Copy + Clone {}
pub(crate) trait Def: Debug + Hash + Eq + PartialEq + Copy + Clone {
fn has_safety_invariants(&self) -> bool;
}
pub trait Ref: Debug + Hash + Eq + PartialEq + Copy + Clone {
fn min_align(&self) -> usize;

fn is_mutable(&self) -> bool;
}

impl Def for ! {}
impl Def for ! {
fn has_safety_invariants(&self) -> bool {
unreachable!()
}
}

impl Ref for ! {
fn min_align(&self) -> usize {
unreachable!()
Expand Down Expand Up @@ -83,5 +90,12 @@ pub mod rustc {
Primitive,
}

impl<'tcx> super::Def for Def<'tcx> {}
impl<'tcx> super::Def for Def<'tcx> {
fn has_safety_invariants(&self) -> bool {
// Rust presently has no notion of 'unsafe fields', so for now we
// make the conservative assumption that everything besides
// primitive types carry safety invariants.
self != &Self::Primitive
}
}
}
5 changes: 3 additions & 2 deletions compiler/rustc_transmute/src/layout/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ where
Self::Seq(vec![Self::uninit(); width_in_bytes])
}

/// Remove all `Def` nodes, and all branches of the layout for which `f` produces false.
/// Remove all `Def` nodes, and all branches of the layout for which `f`
/// produces `true`.
pub(crate) fn prune<F>(self, f: &F) -> Tree<!, R>
where
F: Fn(D) -> bool,
Expand All @@ -106,7 +107,7 @@ where
Self::Byte(b) => Tree::Byte(b),
Self::Ref(r) => Tree::Ref(r),
Self::Def(d) => {
if !f(d) {
if f(d) {
Tree::uninhabited()
} else {
Tree::unit()
Expand Down
69 changes: 47 additions & 22 deletions compiler/rustc_transmute/src/layout/tree/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@ use super::Tree;

#[derive(Debug, Hash, Eq, PartialEq, Clone, Copy)]
pub enum Def {
Visible,
Invisible,
NoSafetyInvariants,
HasSafetyInvariants,
}

impl super::Def for Def {}
impl super::Def for Def {
fn has_safety_invariants(&self) -> bool {
self == &Self::HasSafetyInvariants
}
}

mod prune {
use super::*;
Expand All @@ -16,17 +20,22 @@ mod prune {

#[test]
fn seq_1() {
let layout: Tree<Def, !> = Tree::def(Def::Visible).then(Tree::from_bits(0x00));
assert_eq!(layout.prune(&|d| matches!(d, Def::Visible)), Tree::from_bits(0x00));
let layout: Tree<Def, !> =
Tree::def(Def::NoSafetyInvariants).then(Tree::from_bits(0x00));
assert_eq!(
layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)),
Tree::from_bits(0x00)
);
}

#[test]
fn seq_2() {
let layout: Tree<Def, !> =
Tree::from_bits(0x00).then(Tree::def(Def::Visible)).then(Tree::from_bits(0x01));
let layout: Tree<Def, !> = Tree::from_bits(0x00)
.then(Tree::def(Def::NoSafetyInvariants))
.then(Tree::from_bits(0x01));

assert_eq!(
layout.prune(&|d| matches!(d, Def::Visible)),
layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)),
Tree::from_bits(0x00).then(Tree::from_bits(0x01))
);
}
Expand All @@ -37,21 +46,32 @@ mod prune {

#[test]
fn invisible_def() {
let layout: Tree<Def, !> = Tree::def(Def::Invisible);
assert_eq!(layout.prune(&|d| matches!(d, Def::Visible)), Tree::uninhabited());
let layout: Tree<Def, !> = Tree::def(Def::HasSafetyInvariants);
assert_eq!(
layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)),
Tree::uninhabited()
);
}

#[test]
fn invisible_def_in_seq_len_2() {
let layout: Tree<Def, !> = Tree::def(Def::Visible).then(Tree::def(Def::Invisible));
assert_eq!(layout.prune(&|d| matches!(d, Def::Visible)), Tree::uninhabited());
let layout: Tree<Def, !> =
Tree::def(Def::NoSafetyInvariants).then(Tree::def(Def::HasSafetyInvariants));
assert_eq!(
layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)),
Tree::uninhabited()
);
}

#[test]
fn invisible_def_in_seq_len_3() {
let layout: Tree<Def, !> =
Tree::def(Def::Visible).then(Tree::from_bits(0x00)).then(Tree::def(Def::Invisible));
assert_eq!(layout.prune(&|d| matches!(d, Def::Visible)), Tree::uninhabited());
let layout: Tree<Def, !> = Tree::def(Def::NoSafetyInvariants)
.then(Tree::from_bits(0x00))
.then(Tree::def(Def::HasSafetyInvariants));
assert_eq!(
layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)),
Tree::uninhabited()
);
}
}

Expand All @@ -60,21 +80,26 @@ mod prune {

#[test]
fn visible_def() {
let layout: Tree<Def, !> = Tree::def(Def::Visible);
assert_eq!(layout.prune(&|d| matches!(d, Def::Visible)), Tree::unit());
let layout: Tree<Def, !> = Tree::def(Def::NoSafetyInvariants);
assert_eq!(layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)), Tree::unit());
}

#[test]
fn visible_def_in_seq_len_2() {
let layout: Tree<Def, !> = Tree::def(Def::Visible).then(Tree::def(Def::Visible));
assert_eq!(layout.prune(&|d| matches!(d, Def::Visible)), Tree::unit());
let layout: Tree<Def, !> =
Tree::def(Def::NoSafetyInvariants).then(Tree::def(Def::NoSafetyInvariants));
assert_eq!(layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)), Tree::unit());
}

#[test]
fn visible_def_in_seq_len_3() {
let layout: Tree<Def, !> =
Tree::def(Def::Visible).then(Tree::from_bits(0x00)).then(Tree::def(Def::Visible));
assert_eq!(layout.prune(&|d| matches!(d, Def::Visible)), Tree::from_bits(0x00));
let layout: Tree<Def, !> = Tree::def(Def::NoSafetyInvariants)
.then(Tree::from_bits(0x00))
.then(Tree::def(Def::NoSafetyInvariants));
assert_eq!(
layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)),
Tree::from_bits(0x00)
);
}
}
}
6 changes: 2 additions & 4 deletions compiler/rustc_transmute/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ pub enum Reason {
DstIsUnspecified,
/// The layout of the destination type is bit-incompatible with the source type.
DstIsBitIncompatible,
/// There aren't any public constructors for `Dst`.
DstIsPrivate,
/// The destination type may carry safety invariants.
DstMayHaveSafetyInvariants,
/// `Dst` is larger than `Src`, and the excess bytes were not exclusively uninitialized.
DstIsTooBig,
/// Src should have a stricter alignment than Dst, but it does not.
Expand Down Expand Up @@ -106,13 +106,11 @@ mod rustc {
&mut self,
cause: ObligationCause<'tcx>,
types: Types<'tcx>,
scope: Ty<'tcx>,
assume: crate::Assume,
) -> crate::Answer<crate::layout::rustc::Ref<'tcx>> {
crate::maybe_transmutable::MaybeTransmutableQuery::new(
types.src,
types.dst,
scope,
assume,
self.infcx.tcx,
)
Expand Down