Skip to content

Commit

Permalink
safe transmute: revise safety analysis
Browse files Browse the repository at this point in the history
Migrate to a simplified safety analysis that does not use visibility.

Closes rust-lang/project-safe-transmute#15
  • Loading branch information
jswrenn committed Feb 27, 2024
1 parent cfb42e5 commit 13675b8
Show file tree
Hide file tree
Showing 124 changed files with 1,362 additions and 1,921 deletions.
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 @@ -856,15 +856,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 @@ -537,14 +537,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 @@ -3088,11 +3088,10 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for 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 {
span_bug!(
span,
Expand All @@ -3103,15 +3102,12 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for 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 @@ -3125,9 +3121,9 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for 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 @@ -303,16 +303,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 @@ -348,7 +346,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 @@ -360,7 +358,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 @@ -51,8 +51,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 @@ -108,13 +108,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

0 comments on commit 13675b8

Please sign in to comment.