Skip to content

Commit

Permalink
Rollup merge of #121681 - jswrenn:nix-visibility-analysis, r=compiler…
Browse files Browse the repository at this point in the history
…-errors

Safe Transmute: Revise safety analysis

This PR migrates `BikeshedIntrinsicFrom` to a simplified safety analysis (described [here](rust-lang/project-safe-transmute#15)) that does not rely on analyzing the visibility of types and fields.

The revised analysis treats primitive types as safe, and user-defined types as potentially carrying safety invariants. If Rust gains explicit (un)safe fields, this PR is structured so that it will be fairly easy to thread support for those annotations into the analysis.

Notably, this PR removes the `Context` type parameter from `BikeshedIntrinsicFrom`. Most of the files changed by this PR are just UI tests tweaked to accommodate the removed parameter.

r? `@compiler-errors`
  • Loading branch information
matthiaskrgr authored Feb 29, 2024
2 parents 5099720 + 23ab1bd commit 419f7ae
Show file tree
Hide file tree
Showing 127 changed files with 1,379 additions and 1,940 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 @@ -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 @@ -549,14 +549,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 @@ -2960,11 +2960,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 @@ -2976,15 +2975,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 @@ -2998,9 +2994,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
Loading

0 comments on commit 419f7ae

Please sign in to comment.