diff --git a/compiler/rustc_transmute/src/layout/dfa.rs b/compiler/rustc_transmute/src/layout/dfa.rs index 92f1898658b07..ef42bbcae1f56 100644 --- a/compiler/rustc_transmute/src/layout/dfa.rs +++ b/compiler/rustc_transmute/src/layout/dfa.rs @@ -5,6 +5,9 @@ use std::sync::atomic::{AtomicU32, Ordering}; use super::{Byte, Reference, Region, Tree, Type, Uninhabited}; use crate::{Map, Set}; +#[cfg(test)] +mod tests; + #[derive(PartialEq)] #[cfg_attr(test, derive(Clone))] pub(crate) struct Dfa @@ -37,7 +40,9 @@ where } } -/// The states in a [`Dfa`] represent byte offsets. +/// An identifier for a node in a [`Dfa`]. +/// +/// The numeric identifier does not encode a byte offset. #[derive(Hash, Eq, PartialEq, PartialOrd, Ord, Copy, Clone)] pub(crate) struct State(pub(crate) u32); @@ -308,7 +313,8 @@ where { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { writeln!(f, "digraph {{")?; - writeln!(f, " {:?} [shape = doublecircle]", self.start)?; + writeln!(f, " start [shape = point, style = invis]")?; + writeln!(f, " start -> {:?}", self.start)?; writeln!(f, " {:?} [shape = doublecircle]", self.accept)?; for (src, transitions) in self.transitions.iter() { @@ -380,15 +386,7 @@ mod edge_set { } pub(crate) fn map_states(self, mut f: impl FnMut(S) -> SS) -> EdgeSet { - EdgeSet { - // NOTE: It appears as through ` as - // IntoIterator>::IntoIter` and `std::iter::Map` both implement - // `TrustedLen`, which in turn means that this `.collect()` - // allocates the correct number of elements once up-front [1]. - // - // [1] https://doc.rust-lang.org/1.85.0/src/alloc/vec/spec_from_iter_nested.rs.html#47 - runs: self.runs.into_iter().map(|(b, s)| (b, f(s))).collect(), - } + EdgeSet { runs: self.runs.into_iter().map(|(b, s)| (b, f(s))).collect() } } /// Unions two edge sets together. @@ -428,7 +426,13 @@ mod edge_set { } } -/// Merges two sorted sequences into one sorted sequence. +/// Partitions two sequences of byte edges into sorted, non-overlapping ranges. +/// +/// Within each input, ranges must be non-empty, non-overlapping, and sorted by +/// ascending start. Each output item contains a range and the destination of the +/// edge from each input that covers it. An input with no edge covering the range +/// contributes `None`. Ranges covered by neither input are omitted. Adjacent +/// output ranges are not coalesced. pub(crate) fn union, Y: Iterator>( xs: X, ys: Y, diff --git a/compiler/rustc_transmute/src/layout/dfa/tests.rs b/compiler/rustc_transmute/src/layout/dfa/tests.rs new file mode 100644 index 0000000000000..b9c1580ac6165 --- /dev/null +++ b/compiler/rustc_transmute/src/layout/dfa/tests.rs @@ -0,0 +1,131 @@ +use std::ops::Range; + +use super::{Byte, Dfa, EdgeSet, union}; + +fn bytes(range: Range) -> Byte { + Byte { start: range.start, end: range.end } +} + +fn assert_union( + xs: &[(Byte, u8)], + ys: &[(Byte, u8)], + expected: &[(Byte, (Option, Option))], +) { + let actual: Vec<_> = union(xs.iter().copied(), ys.iter().copied()).collect(); + assert_eq!(actual, expected); + + let actual: Vec<_> = union(ys.iter().copied(), xs.iter().copied()).collect(); + let expected: Vec<_> = expected.iter().map(|&(range, (x, y))| (range, (y, x))).collect(); + assert_eq!(actual, expected); +} + +#[test] +fn union_empty_inputs() { + assert_union(&[], &[], &[]); + assert_union( + &[(bytes(1..3), 1), (bytes(5..7), 2)], + &[], + &[(bytes(1..3), (Some(1), None)), (bytes(5..7), (Some(2), None))], + ); +} + +#[test] +fn union_disjoint_and_adjacent_ranges() { + assert_union( + &[(bytes(1..3), 1), (bytes(6..8), 2)], + &[(bytes(3..5), 3), (bytes(8..9), 4)], + &[ + (bytes(1..3), (Some(1), None)), + (bytes(3..5), (None, Some(3))), + (bytes(6..8), (Some(2), None)), + (bytes(8..9), (None, Some(4))), + ], + ); +} + +#[test] +fn union_equal_ranges() { + assert_union(&[(bytes(1..4), 1)], &[(bytes(1..4), 2)], &[(bytes(1..4), (Some(1), Some(2)))]); +} + +#[test] +fn union_partially_overlapping_ranges() { + assert_union( + &[(bytes(1..4), 1)], + &[(bytes(3..6), 2)], + &[ + (bytes(1..3), (Some(1), None)), + (bytes(3..4), (Some(1), Some(2))), + (bytes(4..6), (None, Some(2))), + ], + ); +} + +#[test] +fn union_contained_ranges() { + assert_union( + &[(bytes(0..8), 1)], + &[(bytes(2..4), 2), (bytes(4..6), 3)], + &[ + (bytes(0..2), (Some(1), None)), + (bytes(2..4), (Some(1), Some(2))), + (bytes(4..6), (Some(1), Some(3))), + (bytes(6..8), (Some(1), None)), + ], + ); +} + +#[test] +fn union_matching_starts_and_ends() { + assert_union( + &[(bytes(0..4), 1), (bytes(6..10), 2)], + &[(bytes(0..2), 3), (bytes(8..10), 4)], + &[ + (bytes(0..2), (Some(1), Some(3))), + (bytes(2..4), (Some(1), None)), + (bytes(6..8), (Some(2), None)), + (bytes(8..10), (Some(2), Some(4))), + ], + ); +} + +#[test] +fn union_uninit_boundary() { + let uninit = Byte::UNINIT; + assert_union( + &[(Byte::uninit(), 1)], + &[(bytes(255..uninit), 2), (bytes(uninit..uninit + 1), 3)], + &[ + (bytes(0..255), (Some(1), None)), + (bytes(255..uninit), (Some(1), Some(2))), + (bytes(uninit..uninit + 1), (Some(1), Some(3))), + ], + ); +} + +#[test] +fn edge_set_union_coalesces_only_adjacent_ranges_with_equal_destinations() { + let xs = EdgeSet::from_edges(vec![(bytes(0..4), 1), (bytes(8..10), 1)]); + let ys = EdgeSet::from_edges(vec![(bytes(2..6), 2), (bytes(10..12), 3)]); + let merged = xs.union(&ys, |_, y| if y == Some(3) { 8 } else { 7 }); + + assert_eq!( + merged.iter().collect::>(), + [(bytes(0..6), 7), (bytes(8..10), 7), (bytes(10..12), 8)], + ); +} + +#[test] +fn dot_distinguishes_start_and_accept() { + for dfa in [Dfa::::from_edges(0, 1, &[(0, 0u8, 1)]), Dfa::unit()] { + let dot = format!("{dfa:?}"); + assert!(dot.lines().any(|line| line == " start [shape = point, style = invis]")); + let start_edge = format!(" start -> {:?}", dfa.start); + assert!(dot.lines().any(|line| line == start_edge)); + + let accept_marker = format!(" {:?} [shape = doublecircle]", dfa.accept); + let accept_markers: Vec<_> = + dot.lines().filter(|line| line.contains("doublecircle")).collect(); + assert_eq!(accept_markers, [accept_marker.as_str()]); + } +} diff --git a/compiler/rustc_transmute/src/layout/mod.rs b/compiler/rustc_transmute/src/layout/mod.rs index 4e548ff8fe20c..54cd37a8f50dd 100644 --- a/compiler/rustc_transmute/src/layout/mod.rs +++ b/compiler/rustc_transmute/src/layout/mod.rs @@ -11,12 +11,14 @@ pub(crate) use dfa::{Dfa, union}; #[derive(Debug)] pub(crate) struct Uninhabited; -/// A range of byte values (including an uninit byte value). +/// A half-open range of byte values, which may include an uninitialized byte. +/// +/// The range is `[start, end)`. Values `0..=255` represent initialized bytes, +/// and `256` represents an uninitialized byte. A range containing that value +/// therefore has an exclusive upper bound of `257`. #[derive(Hash, Eq, PartialEq, Ord, PartialOrd, Clone, Copy)] pub(crate) struct Byte { - // An inclusive-exclusive range. We use this instead of `Range` because `Range: !Copy`. - // - // Uninit byte value is represented by 256. + // Store the endpoints separately because `Range` is not `Copy`. pub(crate) start: u16, pub(crate) end: u16, } @@ -37,6 +39,7 @@ impl Byte { Byte { start: val, end: val + 1 } } + /// A byte that may be initialized to any value or uninitialized. #[inline] fn uninit() -> Byte { Byte { start: 0, end: Self::UNINIT + 1 } @@ -136,7 +139,7 @@ pub mod rustc { use rustc_middle::ty::layout::{HasTyCtxt, LayoutCx, LayoutError}; use rustc_middle::ty::{self, Region, Ty}; - /// A visibility node in the layout. + /// A layout annotation used to check for safety invariants during pruning. #[derive(Debug, Hash, Eq, PartialEq, Clone, Copy)] pub enum Def<'tcx> { Adt(ty::AdtDef<'tcx>), @@ -147,9 +150,9 @@ pub mod rustc { 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. + // Conservatively treat every ADT, variant, and field definition as + // potentially carrying safety invariants. This check does not + // inspect `unsafe` field annotations. self != &Self::Primitive } } diff --git a/compiler/rustc_transmute/src/layout/tree.rs b/compiler/rustc_transmute/src/layout/tree.rs index d7aeb6b06b82f..43e2008c5a441 100644 --- a/compiler/rustc_transmute/src/layout/tree.rs +++ b/compiler/rustc_transmute/src/layout/tree.rs @@ -7,6 +7,14 @@ mod tests; /// A tree-based representation of a type layout. /// +/// A `Seq` concatenates layouts, while an `Alt` chooses among them. An empty +/// `Seq` represents an inhabited, zero-sized layout; an empty `Alt` represents +/// an uninhabited layout. +/// +/// `Def` nodes are zero-width annotations used by [`Tree::prune`] to decide +/// which branches to retain. Pruning removes these annotations and produces +/// a `Tree` for conversion with [`super::Dfa::from_tree`]. +/// /// Invariants: /// 1. All paths through the layout have the same length (in bytes). /// @@ -25,7 +33,7 @@ where Seq(Vec), /// A choice between alternative layouts. Alt(Vec), - /// A definition node. + /// A zero-width definition annotation used during pruning. Def(D), /// A reference node. Ref(Reference), @@ -70,7 +78,7 @@ where Self::Seq(Vec::new()) } - /// A `Tree` containing a single, uninitialized byte. + /// A `Tree` containing one byte that may be initialized to any value or uninitialized. pub(crate) fn uninit() -> Self { Self::Byte(Byte::uninit()) } @@ -134,12 +142,18 @@ where } /// A `Tree` whose layout is entirely padding of the given width. + /// + /// Each padding byte may be initialized to any value or uninitialized. pub(crate) fn padding(width_in_bytes: usize) -> Self { Self::Seq(vec![Self::uninit(); width_in_bytes]) } - /// Remove all `Def` nodes, and all branches of the layout for which `f` - /// produces `true`. + /// Removes all `Def` nodes and rejects branches whose definitions make `f` return `true`. + /// + /// A sequence becomes uninhabited if any of its elements becomes uninhabited; + /// alternatives retain their surviving branches. Retained definitions become + /// empty sequences, so the result contains no definition nodes, as expressed + /// by its `Tree` type. pub(crate) fn prune(self, f: &F) -> Tree where F: Fn(D) -> bool, @@ -473,7 +487,7 @@ pub(crate) mod rustc { }; // When this function is invoked with enum variants, - // `ty_and_layout.size` does not encompass the entire size of the + // `layout.size` does not encompass the entire size of the // enum. We rely on `total_size` for this. assert!(layout.size <= total_size); diff --git a/compiler/rustc_transmute/src/layout/tree/tests.rs b/compiler/rustc_transmute/src/layout/tree/tests.rs index bc47b19c681f4..43150369374be 100644 --- a/compiler/rustc_transmute/src/layout/tree/tests.rs +++ b/compiler/rustc_transmute/src/layout/tree/tests.rs @@ -34,13 +34,48 @@ mod prune { Tree::byte(0x00).then(Tree::byte(0x01)) ); } + + #[test] + fn single_retained_alternative() { + let layout: Tree = Tree::alt([ + Tree::def(Def::HasSafetyInvariants).then(Tree::byte(0x00)), + Tree::def(Def::NoSafetyInvariants).then(Tree::byte(0x01)), + Tree::def(Def::HasSafetyInvariants).then(Tree::byte(0x02)), + ]); + + assert_eq!(layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)), Tree::byte(0x01)); + } + + #[test] + fn mixed_nested_alternatives_in_seq() { + let layout: Tree = Tree::seq([ + Tree::byte(0x00), + Tree::Alt(vec![ + Tree::def(Def::NoSafetyInvariants).then(Tree::byte(0x01)), + Tree::Alt(vec![ + Tree::def(Def::HasSafetyInvariants).then(Tree::byte(0x02)), + Tree::def(Def::NoSafetyInvariants).then(Tree::byte(0x03)), + ]), + ]), + Tree::byte(0x04), + ]); + + assert_eq!( + layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)), + Tree::seq([ + Tree::byte(0x00), + Tree::alt([Tree::byte(0x01), Tree::byte(0x03)]), + Tree::byte(0x04), + ]) + ); + } } mod should_reject { use super::*; #[test] - fn invisible_def() { + fn def_with_safety_invariants() { let layout: Tree = Tree::def(Def::HasSafetyInvariants); assert_eq!( layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)), @@ -49,7 +84,7 @@ mod prune { } #[test] - fn invisible_def_in_seq_len_2() { + fn def_with_safety_invariants_in_seq_len_2() { let layout: Tree = Tree::def(Def::NoSafetyInvariants).then(Tree::def(Def::HasSafetyInvariants)); assert_eq!( @@ -59,7 +94,7 @@ mod prune { } #[test] - fn invisible_def_in_seq_len_3() { + fn def_with_safety_invariants_in_seq_len_3() { let layout: Tree = Tree::def(Def::NoSafetyInvariants) .then(Tree::byte(0x00)) .then(Tree::def(Def::HasSafetyInvariants)); @@ -68,26 +103,56 @@ mod prune { Tree::uninhabited() ); } + + #[test] + fn all_alternatives_pruned() { + let layout: Tree = Tree::alt([ + Tree::def(Def::HasSafetyInvariants).then(Tree::byte(0x00)), + Tree::def(Def::HasSafetyInvariants).then(Tree::byte(0x01)), + ]); + + assert_eq!( + layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)), + Tree::uninhabited() + ); + } + + #[test] + fn all_alternatives_pruned_in_seq() { + let layout: Tree = Tree::seq([ + Tree::byte(0x00), + Tree::alt([ + Tree::def(Def::HasSafetyInvariants).then(Tree::byte(0x01)), + Tree::def(Def::HasSafetyInvariants).then(Tree::byte(0x02)), + ]), + Tree::byte(0x03), + ]); + + assert_eq!( + layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)), + Tree::uninhabited() + ); + } } mod should_accept { use super::*; #[test] - fn visible_def() { + fn def_without_safety_invariants() { let layout: Tree = Tree::def(Def::NoSafetyInvariants); assert_eq!(layout.prune(&|d| matches!(d, Def::HasSafetyInvariants)), Tree::unit()); } #[test] - fn visible_def_in_seq_len_2() { + fn def_without_safety_invariants_in_seq_len_2() { let layout: Tree = 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() { + fn def_without_safety_invariants_in_seq_len_3() { let layout: Tree = Tree::def(Def::NoSafetyInvariants) .then(Tree::byte(0x00)) .then(Tree::def(Def::NoSafetyInvariants)); diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index b49836dd0b8ea..e2e1b0a08e15a 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -1,3 +1,11 @@ +//! Checks whether one type's representation can be reinterpreted as another. +//! +//! The analysis converts compiler layouts into trees of bytes, references, and +//! definition markers. It prunes destination paths that may carry safety invariants unless +//! safety is assumed, then converts the trees into deterministic finite automata. +//! Comparing the automata produces an [`Answer`]; reference transitions can leave +//! [`Condition`]s for the trait solver to discharge. + // tidy-alphabetical-start #![cfg_attr(bootstrap, feature(never_type))] #![cfg_attr(test, feature(test))] @@ -9,6 +17,10 @@ pub(crate) use rustc_data_structures::fx::{FxIndexMap as Map, FxIndexSet as Set} pub mod layout; mod maybe_transmutable; +/// Proof obligations supplied by the caller rather than checked by the analysis. +/// +/// This mirrors `core::mem::Assume`. A `true` field transfers the corresponding +/// obligation to the caller; the default leaves all four obligations to the compiler. #[derive(Copy, Clone, Debug, Default)] pub struct Assume { pub alignment: bool, @@ -17,12 +29,14 @@ pub struct Assume { pub validity: bool, } -/// Either transmutation is allowed, we have an error, or we have an optional -/// Condition that must hold. +/// The result of a transmutability query under the supplied [`Assume`] options. #[derive(Debug, Hash, Eq, PartialEq, Clone)] pub enum Answer { + /// The analysis requires no further conditions. Yes, + /// The analysis could not establish transmutability. No(Reason), + /// Transmutability depends on conditions that the trait solver must discharge. If(Condition), } @@ -35,7 +49,7 @@ pub enum Condition { /// The region `long` must outlive `short`. Outlives { long: R, short: R }, - /// The `ty` is immutable. + /// The type `ty` must satisfy `Freeze`. Immutable { ty: T }, /// `Src` is transmutable into `Dst`, if all of the enclosed requirements are met. @@ -60,7 +74,7 @@ pub enum Reason { DstMayHaveSafetyInvariants, /// `Dst` is larger than `Src`, and the excess bytes were not exclusively uninitialized. DstIsTooBig, - /// `Dst` is larger `Src`. + /// The destination referent is larger than the source referent. DstRefIsTooBig { /// The referent of the source type. src: T, @@ -71,7 +85,7 @@ pub enum Reason { /// The size of the destination type's referent. dst_size: usize, }, - /// Src should have a stricter alignment than Dst, but it does not. + /// The destination referent requires stricter alignment than the source referent. DstHasStricterAlignment { src_min_align: usize, dst_min_align: usize }, /// Can't go from shared pointer to unique pointer DstIsMoreUnique, diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index d677e88220d96..e9351d4081b6e 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -27,7 +27,6 @@ where } } -// FIXME: Nix this cfg, so we can write unit tests independently of rustc #[cfg(feature = "rustc")] mod rustc { use rustc_middle::ty::layout::LayoutCx; @@ -79,18 +78,17 @@ where { /// Answers whether a `Tree` is transmutable into another `Tree`. /// - /// This method begins by de-def'ing `src` and `dst`, and prunes private paths from `dst`, - /// then converts `src` and `dst` to `Dfa`s, and computes an answer using those DFAs. + /// Removes definition markers from both trees and, unless safety is assumed, + /// prunes destination paths that may carry safety invariants. It then converts + /// the remaining layouts to `Dfa`s and compares them. #[inline(always)] #[instrument(level = "debug", skip(self), fields(src = ?self.src, dst = ?self.dst))] pub(crate) fn answer(self) -> Answer<::Region, ::Type> { let Self { src, dst, assume, context } = self; - // Unconditionally remove all `Def` nodes from `src`, without pruning away the - // branches they appear in. This is valid to do for value-to-value - // transmutations, but not for `&mut T` to `&mut U`; we will need to be - // more sophisticated to handle transmutations between mutable - // references. + // Keep every source representation while removing its definition markers. + // Reference nodes remain intact; mutable destination references also generate + // a reverse transmutability obligation for their referents. let src = src.prune(&|_def| false); if src.is_inhabited() && !dst.is_inhabited() { @@ -99,20 +97,17 @@ where trace!(?src, "pruned src"); - // Remove all `Def` nodes from `dst`, additionally... + // Remove destination definition markers. Unless the caller assumes safety, + // prune paths whose definitions may carry safety invariants. let dst = if assume.safety { - // ...if safety is assumed, don't check if they carry safety - // invariants; retain all paths. dst.prune(&|_def| false) } else { - // ...otherwise, prune away all paths with safety invariants from - // the `Dst` layout. dst.prune(&|def| def.has_safety_invariants()) }; trace!(?dst, "pruned dst"); - // Convert `src` from a tree-based representation to an DFA-based + // Convert `src` from a tree-based representation to a DFA-based // representation. If the conversion fails because `src` is uninhabited, // conclude that the transmutation is acceptable, because instances of // the `src` type do not exist. @@ -121,12 +116,9 @@ where Err(layout::Uninhabited) => return Answer::Yes, }; - // Convert `dst` from a tree-based representation to an DFA-based - // representation. If the conversion fails because `src` is uninhabited, - // conclude that the transmutation is unacceptable. Valid instances of - // the `dst` type do not exist, either because it's genuinely - // uninhabited, or because there are no branches of the tree that are - // free of safety invariants. + // An inhabited source and an originally uninhabited destination were + // rejected above. If the pruned destination is now uninhabited, no path + // remains whose definitions are known to be free of safety invariants. let dst = match Dfa::from_tree(dst) { Ok(dst) => dst, Err(layout::Uninhabited) => return Answer::No(Reason::DstMayHaveSafetyInvariants), @@ -142,11 +134,17 @@ where { /// Answers whether a `Dfa` is transmutable into another `Dfa`. pub(crate) fn answer(self) -> Answer<::Region, ::Type> { + debug!(src = ?self.src); + debug!(dst = ?self.dst); + debug!( + src_transitions_len = self.src.transitions.len(), + dst_transitions_len = self.dst.transitions.len() + ); self.answer_memo(&mut Map::default(), self.src.start, self.dst.start) } #[inline(always)] - #[instrument(level = "debug", skip(self))] + #[instrument(level = "debug", skip(self, cache))] fn answer_memo( &self, cache: &mut Map< @@ -177,28 +175,11 @@ where dst_state: dfa::State, ) -> Answer<::Region, ::Type> { debug!(?src_state, ?dst_state); - debug!(src = ?self.src); - debug!(dst = ?self.dst); - debug!( - src_transitions_len = self.src.transitions.len(), - dst_transitions_len = self.dst.transitions.len() - ); if dst_state == self.dst.accept { - // truncation: `size_of(Src) >= size_of(Dst)` - // - // Why is truncation OK to do? Because even though the Src is bigger, all we care about - // is whether we have enough data for the Dst to be valid in accordance with what its - // type dictates. - // For example, in a u8 to `()` transmutation, we have enough data available from the u8 - // to transmute it to a `()` (though in this case does `()` really need any data to - // begin with? It doesn't). Same thing with u8 to fieldless struct. - // Now then, why is something like u8 to bool not allowed? That is not because the bool - // is smaller in size, but rather because those 2 bits that we are re-interpreting from - // the u8 could introduce invalid states for the bool type. - // - // So, if it's possible to transmute to a smaller Dst by truncating, and we can guarantee - // that none of the actually-used data can introduce an invalid state for Dst's type, we - // are able to safely transmute, even with truncation. + // The destination needs no more input. Union transmutation permits + // truncating the remaining source bytes: for example, `u8` to `()`. + // Compatibility of the consumed prefix is checked by the preceding + // transitions, including any conditions they generate for references. Answer::Yes } else if src_state == self.src.accept { // extension: `size_of(Src) <= size_of(Dst)` @@ -209,14 +190,13 @@ where } } else { let src_quantifier = if self.assume.validity { - // if the compiler may assume that the programmer is doing additional validity checks, - // (e.g.: that `src != 3u8` when the destination type is `bool`) - // then there must exist at least one transition out of `src_state` such that the transmute is viable... + // The caller checks validity (for example, `src <= 1u8` for a + // `u8`-to-`bool` transmutation), so at least one source transition + // must admit a compatible continuation. Quantifier::ThereExists } else { - // if the compiler cannot assume that the programmer is doing additional validity checks, - // then for all transitions out of `src_state`, such that the transmute is viable... - // then there must exist at least one transition out of `dst_state` such that the transmute is viable... + // Every source transition must admit a compatible continuation + // in the destination when the caller does not assume validity. Quantifier::ForAll }; @@ -320,10 +300,12 @@ where } impl Answer { + /// Requires both answers, combining their conditions into a conjunction. fn and(self, rhs: Answer) -> Answer { let lhs = self; match (lhs, rhs) { - // If both are errors, then we should return the more specific one + // Prefer a specific reason over generic bit incompatibility; + // otherwise, retain the left-hand reason. (Answer::No(Reason::DstIsBitIncompatible), Answer::No(reason)) | (Answer::No(reason), Answer::No(_)) // If either is an error, return it @@ -346,10 +328,15 @@ impl Answer { } } + /// Combines alternative answers and collects their conditions in an `IfAny`. + /// + /// Currently, combining `Yes` with `If` retains the condition. This differs + /// from Boolean disjunction, where unconditional success would suffice. fn or(self, rhs: Answer) -> Answer { let lhs = self; match (lhs, rhs) { - // If both are errors, then we should return the more specific one + // Prefer a specific reason over generic bit incompatibility; + // otherwise, retain the left-hand reason. (Answer::No(Reason::DstIsBitIncompatible), Answer::No(reason)) | (Answer::No(reason), Answer::No(_)) => Answer::No(reason), // Otherwise, errors can be ignored for the rest of the pattern matching @@ -379,6 +366,9 @@ enum Quantifier { } impl Quantifier { + /// Folds answers with `or` or `and`, stopping on `Yes` or `No`, respectively. + /// An empty iterator yields bit incompatibility for `ThereExists` and `Yes` + /// for `ForAll`. fn apply(&self, iter: I) -> Answer where R: layout::Region, diff --git a/compiler/rustc_transmute/src/maybe_transmutable/tests.rs b/compiler/rustc_transmute/src/maybe_transmutable/tests.rs index 8440ace260883..371a362c09ff3 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/tests.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/tests.rs @@ -43,12 +43,103 @@ fn is_transmutable( ) -> crate::Answer { let src = src.clone(); let dst = dst.clone(); - // The only dimension of the transmutability analysis we want to test - // here is the safety analysis. To ensure this, we disable all other - // toggleable aspects of the transmutability analysis. R::is_transmutable(src, dst, assume) } +mod answers { + use crate::maybe_transmutable::Quantifier; + use crate::{Condition, Reason}; + + type Answer = crate::Answer; + + const OUTLIVES: Condition = Condition::Outlives { long: 1, short: 0 }; + const IMMUTABLE: Condition = Condition::Immutable { ty: () }; + const TRANSMUTABLE: Condition = Condition::Transmutable { src: (), dst: () }; + + #[test] + fn and_yes_is_identity() { + for answer in [Answer::Yes, Answer::No(Reason::DstIsTooBig), Answer::If(IMMUTABLE)] { + assert_eq!(Answer::Yes.and(answer.clone()), answer); + assert_eq!(answer.clone().and(Answer::Yes), answer); + } + } + + #[test] + fn and_no_absorbs_conditions() { + let no = Answer::No(Reason::DstIsTooBig); + let conditional = Answer::If(IMMUTABLE); + assert_eq!(no.clone().and(conditional.clone()), no); + assert_eq!(conditional.and(no.clone()), no); + } + + #[test] + fn or_bit_incompatibility_is_identity() { + for answer in [Answer::Yes, Answer::No(Reason::DstIsTooBig), Answer::If(IMMUTABLE)] { + let no = Answer::No(Reason::DstIsBitIncompatible); + assert_eq!(no.clone().or(answer.clone()), answer); + assert_eq!(answer.clone().or(no), answer); + } + } + + #[test] + fn or_yes_preserves_conditions() { + let conditional = Answer::If(IMMUTABLE); + assert_eq!(Answer::Yes.or(conditional.clone()), conditional); + assert_eq!(conditional.clone().or(Answer::Yes), conditional); + } + + #[test] + fn and_flattens_condition_groups() { + let pair = Answer::If(OUTLIVES).and(Answer::If(IMMUTABLE)); + assert_eq!(pair, Answer::If(Condition::IfAll(vec![OUTLIVES, IMMUTABLE]))); + + let expected = Answer::If(Condition::IfAll(vec![OUTLIVES, IMMUTABLE, TRANSMUTABLE])); + assert_eq!(pair.clone().and(Answer::If(TRANSMUTABLE)), expected); + assert_eq!(Answer::If(TRANSMUTABLE).and(pair.clone()), expected); + assert_eq!(pair.and(Answer::If(Condition::IfAll(vec![TRANSMUTABLE]))), expected); + } + + #[test] + fn or_flattens_condition_groups() { + let pair = Answer::If(OUTLIVES).or(Answer::If(IMMUTABLE)); + assert_eq!(pair, Answer::If(Condition::IfAny(vec![OUTLIVES, IMMUTABLE]))); + + let expected = Answer::If(Condition::IfAny(vec![OUTLIVES, IMMUTABLE, TRANSMUTABLE])); + assert_eq!(pair.clone().or(Answer::If(TRANSMUTABLE)), expected); + assert_eq!(Answer::If(TRANSMUTABLE).or(pair.clone()), expected); + assert_eq!(pair.or(Answer::If(Condition::IfAny(vec![TRANSMUTABLE]))), expected); + } + + #[test] + fn specific_errors_take_precedence_over_bit_incompatibility() { + let combinators: [fn(Answer, Answer) -> Answer; 2] = [Answer::and, Answer::or]; + for combine in combinators { + let generic = Answer::No(Reason::DstIsBitIncompatible); + let specific = Answer::No(Reason::DstIsTooBig); + assert_eq!(combine(generic.clone(), specific.clone()), specific); + assert_eq!(combine(specific.clone(), generic), specific); + } + } + + #[test] + fn empty_quantifiers() { + assert_eq!(Quantifier::ForAll.apply([]), Answer::Yes); + assert_eq!(Quantifier::ThereExists.apply([]), Answer::No(Reason::DstIsBitIncompatible)); + } + + #[test] + fn quantifiers_short_circuit_after_a_decisive_answer() { + let no = Answer::No(Reason::DstIsTooBig); + let mut answers = [Answer::If(IMMUTABLE), no.clone(), Answer::Yes].into_iter(); + assert_eq!(Quantifier::ForAll.apply(&mut answers), no); + assert_eq!(answers.next(), Some(Answer::Yes)); + + let mut answers = [no, Answer::Yes, Answer::If(IMMUTABLE)].into_iter(); + assert_eq!(Quantifier::ThereExists.apply(&mut answers), Answer::Yes); + assert_eq!(answers.next(), Some(Answer::If(IMMUTABLE))); + } +} + mod safety { use super::*; use crate::Answer; @@ -205,33 +296,22 @@ mod bool { let dst_layout = into_layout(dst_alts.clone()); let dst_set = into_set(dst_alts.clone()); - if src_set.is_subset(&dst_set) { - assert_eq!( - Answer::Yes, - is_transmutable(&src_layout, &dst_layout, Assume::default()), - "{:?} SHOULD be transmutable into {:?}", - src_layout, - dst_layout - ); - } else if !src_set.is_disjoint(&dst_set) { - assert_eq!( - Answer::Yes, - is_transmutable( - &src_layout, - &dst_layout, - Assume { validity: true, ..Assume::default() } - ), - "{:?} SHOULD be transmutable (assuming validity) into {:?}", - src_layout, - dst_layout - ); - } else { + for validity in [false, true] { + let permitted = if validity { + src_set.is_empty() || !src_set.is_disjoint(&dst_set) + } else { + src_set.is_subset(&dst_set) + }; + let expected = if permitted { + Answer::Yes + } else { + Answer::No(Reason::DstIsBitIncompatible) + }; + let assume = Assume { validity, ..Assume::default() }; assert_eq!( - Answer::No(Reason::DstIsBitIncompatible), - is_transmutable(&src_layout, &dst_layout, Assume::default()), - "{:?} should NOT be transmutable into {:?}", - src_layout, - dst_layout + is_transmutable(&src_layout, &dst_layout, assume), + expected, + "src: {src_layout:?}, dst: {dst_layout:?}, validity: {validity}" ); } } diff --git a/tests/ui/transmutability/uninhabited.rs b/tests/ui/transmutability/uninhabited.rs index 4e48b590c428c..ca78874366660 100644 --- a/tests/ui/transmutability/uninhabited.rs +++ b/tests/ui/transmutability/uninhabited.rs @@ -41,12 +41,10 @@ fn yawning_void_struct() { assert!(false); //~ ERROR: evaluation panicked: assertion failed: false }; - // This transmutation is vacuously acceptable; since one cannot construct a - // `Void`, unsoundness cannot directly arise from transmuting a void into - // anything else. + // No value of `YawningVoid` exists, so transmutation from it is vacuously acceptable. assert::is_maybe_transmutable::(); - assert::is_maybe_transmutable::<(), Void>(); //~ ERROR: cannot be safely transmuted + assert::is_maybe_transmutable::(); //~ ERROR: cannot be safely transmuted } // Non-ZST uninhabited types are, nonetheless, uninhabited. @@ -63,12 +61,10 @@ fn yawning_void_enum() { assert!(false); //~ ERROR: evaluation panicked: assertion failed: false }; - // This transmutation is vacuously acceptable; since one cannot construct a - // `Void`, unsoundness cannot directly arise from transmuting a void into - // anything else. + // No value of `YawningVoid` exists, so transmutation from it is vacuously acceptable. assert::is_maybe_transmutable::(); - assert::is_maybe_transmutable::<(), Void>(); //~ ERROR: cannot be safely transmuted + assert::is_maybe_transmutable::(); //~ ERROR: cannot be safely transmuted } // References to uninhabited types are, logically, uninhabited, but for layout diff --git a/tests/ui/transmutability/uninhabited.stderr b/tests/ui/transmutability/uninhabited.stderr index 4757daec9978a..ac4b342ca990c 100644 --- a/tests/ui/transmutability/uninhabited.stderr +++ b/tests/ui/transmutability/uninhabited.stderr @@ -19,11 +19,11 @@ LL | | lifetimes: true, LL | | }> | |__________^ required by this bound in `is_maybe_transmutable` -error[E0277]: `()` cannot be safely transmuted into `yawning_void_struct::Void` - --> $DIR/uninhabited.rs:49:41 +error[E0277]: `u128` cannot be safely transmuted into `yawning_void_struct::YawningVoid` + --> $DIR/uninhabited.rs:47:43 | -LL | assert::is_maybe_transmutable::<(), Void>(); - | ^^^^ `yawning_void_struct::Void` is uninhabited +LL | assert::is_maybe_transmutable::(); + | ^^^^^^^^^^^ `yawning_void_struct::YawningVoid` is uninhabited | note: required by a bound in `is_maybe_transmutable` --> $DIR/uninhabited.rs:10:14 @@ -46,11 +46,11 @@ error[E0080]: evaluation panicked: assertion failed: false LL | assert!(false); | ^^^^^^^^^^^^^^ evaluation of `yawning_void_struct::_` failed here -error[E0277]: `()` cannot be safely transmuted into `yawning_void_enum::Void` - --> $DIR/uninhabited.rs:71:41 +error[E0277]: `u128` cannot be safely transmuted into `yawning_void_enum::YawningVoid` + --> $DIR/uninhabited.rs:67:43 | -LL | assert::is_maybe_transmutable::<(), Void>(); - | ^^^^ `yawning_void_enum::Void` is uninhabited +LL | assert::is_maybe_transmutable::(); + | ^^^^^^^^^^^ `yawning_void_enum::YawningVoid` is uninhabited | note: required by a bound in `is_maybe_transmutable` --> $DIR/uninhabited.rs:10:14 @@ -68,13 +68,13 @@ LL | | }> | |__________^ required by this bound in `is_maybe_transmutable` error[E0080]: evaluation panicked: assertion failed: false - --> $DIR/uninhabited.rs:63:9 + --> $DIR/uninhabited.rs:61:9 | LL | assert!(false); | ^^^^^^^^^^^^^^ evaluation of `yawning_void_enum::_` failed here error[E0277]: `u128` cannot be safely transmuted into `DistantVoid` - --> $DIR/uninhabited.rs:92:43 + --> $DIR/uninhabited.rs:88:43 | LL | assert::is_maybe_transmutable::(); | ^^^^^^^^^^^ at least one value of `u128` isn't a bit-valid value of `DistantVoid` @@ -95,13 +95,13 @@ LL | | }> | |__________^ required by this bound in `is_maybe_transmutable` error[E0080]: evaluation panicked: assertion failed: false - --> $DIR/uninhabited.rs:87:9 + --> $DIR/uninhabited.rs:83:9 | LL | assert!(false); | ^^^^^^^^^^^^^^ evaluation of `distant_void::_` failed here error[E0277]: `Src` cannot be safely transmuted into `issue_126267::Error` - --> $DIR/uninhabited.rs:108:42 + --> $DIR/uninhabited.rs:104:42 | LL | assert::is_maybe_transmutable::(); | ^^^ the size of `Src` is smaller than the size of `issue_126267::Error`