Skip to content

Commit

Permalink
Compute transmutability from rustc_target::abi::Layout
Browse files Browse the repository at this point in the history
In its first step of computing transmutability, `rustc_transmutability`
constructs a byte-level representation of type layout (`Tree`). Previously, this
representation was computed for ADTs by inspecting the ADT definition and
performing our own layout computations. This process was error-prone, verbose,
and limited our ability to analyze many types (particularly default-repr types).

In this PR, we instead construct `Tree`s from `rustc_target::abi::Layout`s. This
helps ensure that layout optimizations are reflected our analyses, and increases
the kinds of types we can now analyze, including:
- default repr ADTs
- transparent unions
- `UnsafeCell`-containing types

Overall, this PR expands the expressvity of `rustc_transmutability` to be much
closer to the transmutability analysis performed by miri. Future PRs will work
to close the remaining gaps (e.g., support for `Box`, raw pointers, `NonZero*`,
coroutines, etc.).
  • Loading branch information
jswrenn committed Apr 2, 2024
1 parent d6eb0f5 commit c9a581a
Show file tree
Hide file tree
Showing 31 changed files with 989 additions and 775 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub struct ImplCandidate<'tcx> {

enum GetSafeTransmuteErrorAndReason {
Silent,
Error { err_msg: String, safe_transmute_explanation: String },
Error { err_msg: String, safe_transmute_explanation: Option<String> },
}

struct UnsatisfiedConst(pub bool);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
GetSafeTransmuteErrorAndReason::Error {
err_msg,
safe_transmute_explanation,
} => (err_msg, Some(safe_transmute_explanation)),
} => (err_msg, safe_transmute_explanation),
}
} else {
(err_msg, None)
Expand Down Expand Up @@ -3061,15 +3061,16 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
return GetSafeTransmuteErrorAndReason::Silent;
};

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}`");

match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable(
obligation.cause,
src_and_dst,
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}`");
let safe_transmute_explanation = match reason {
rustc_transmute::Reason::SrcIsNotYetSupported => {
format!("analyzing the transmutability of `{src}` is not yet supported.")
Expand All @@ -3083,6 +3084,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
format!("at least one value of `{src}` isn't a bit-valid value of `{dst}`")
}

rustc_transmute::Reason::DstUninhabited => {
format!("`{dst}` is uninhabited")
}

rustc_transmute::Reason::DstMayHaveSafetyInvariants => {
format!("`{dst}` may carry safety invariants")
}
Expand Down Expand Up @@ -3128,14 +3133,23 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
format!("`{dst}` has an unknown layout")
}
};
GetSafeTransmuteErrorAndReason::Error { err_msg, safe_transmute_explanation }
GetSafeTransmuteErrorAndReason::Error {
err_msg,
safe_transmute_explanation: Some(safe_transmute_explanation),
}
}
// Should never get a Yes at this point! We already ran it before, and did not get a Yes.
Answer::Yes => span_bug!(
span,
"Inconsistent rustc_transmute::is_transmutable(...) result, got Yes",
),
other => span_bug!(span, "Unsupported rustc_transmute::Answer variant: `{other:?}`"),
// Reached when a different obligation (namely `Freeze`) causes the
// transmutability analysis to fail. In this case, silence the
// transmutability error message in favor of that more specific
// error.
Answer::If(_) => {
GetSafeTransmuteErrorAndReason::Error { err_msg, safe_transmute_explanation: None }
}
}
}

Expand Down
42 changes: 35 additions & 7 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,12 +314,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
.flat_map(|cond| flatten_answer_tree(tcx, obligation, predicate, cond))
.collect(),
Condition::IfTransmutable { src, dst } => {
let trait_def_id = obligation.predicate.def_id();
let transmute_trait = obligation.predicate.def_id();
let assume_const = predicate.trait_ref.args.const_at(2);
let make_obl = |from_ty, to_ty| {
let trait_ref1 = ty::TraitRef::new(
let make_transmute_obl = |from_ty, to_ty| {
let trait_ref = ty::TraitRef::new(
tcx,
trait_def_id,
transmute_trait,
[
ty::GenericArg::from(to_ty),
ty::GenericArg::from(from_ty),
Expand All @@ -331,17 +331,45 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation.param_env,
trait_ref1,
trait_ref,
)
};

let make_freeze_obl = |ty| {
let trait_ref = ty::TraitRef::new(
tcx,
tcx.lang_items().freeze_trait().unwrap(),
[ty::GenericArg::from(ty)],
);
Obligation::with_depth(
tcx,
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation.param_env,
trait_ref,
)
};

let mut obls = vec![];

// If the source is a shared reference, it must be `Freeze`;
// otherwise, transmuting could lead to data races.
if src.mutability == Mutability::Not {
obls.extend([make_freeze_obl(src.ty), make_freeze_obl(dst.ty)])
}

// If Dst is mutable, check bidirectionally.
// For example, transmuting bool -> u8 is OK as long as you can't update that u8
// to be > 1, because you could later transmute the u8 back to a bool and get UB.
match dst.mutability {
Mutability::Not => vec![make_obl(src.ty, dst.ty)],
Mutability::Mut => vec![make_obl(src.ty, dst.ty), make_obl(dst.ty, src.ty)],
Mutability::Not => obls.push(make_transmute_obl(src.ty, dst.ty)),
Mutability::Mut => obls.extend([
make_transmute_obl(src.ty, dst.ty),
make_transmute_obl(dst.ty, src.ty),
]),
}

obls
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_transmute/src/layout/dfa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ impl<R> Transitions<R>
where
R: Ref,
{
#[allow(dead_code)]
fn insert(&mut self, transition: Transition<R>, state: State) {
match transition {
Transition::Byte(b) => {
Expand Down Expand Up @@ -82,6 +83,7 @@ impl<R> Dfa<R>
where
R: Ref,
{
#[allow(dead_code)]
pub(crate) fn unit() -> Self {
let transitions: Map<State, Transitions<R>> = Map::default();
let start = State::new();
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_transmute/src/layout/nfa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ where
Self { transitions, start, accepting }
}

#[allow(dead_code)]
pub(crate) fn edges_from(&self, start: State) -> Option<&Map<Transition<R>, Set<State>>> {
self.transitions.get(&start)
}
Expand Down
Loading

0 comments on commit c9a581a

Please sign in to comment.