Skip to content

Use a trait to enforce field validity for union fields + unsafe fields + unsafe<> binder types #136660

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

Merged
merged 3 commits into from
Feb 13, 2025
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_cranelift/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ impl<T: ?Sized> LegacyReceiver for Box<T> {}
#[lang = "copy"]
pub trait Copy {}

#[lang = "bikeshed_guaranteed_no_drop"]
pub trait BikeshedGuaranteedNoDrop {}

impl Copy for bool {}
impl Copy for u8 {}
impl Copy for u16 {}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_gcc/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ impl<T: ?Sized, A: Allocator> LegacyReceiver for Box<T, A> {}
#[lang = "copy"]
pub trait Copy {}

#[lang = "bikeshed_guaranteed_no_drop"]
pub trait BikeshedGuaranteedNoDrop {}

impl Copy for bool {}
impl Copy for u8 {}
impl Copy for u16 {}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ language_item_table! {
PhantomData, sym::phantom_data, phantom_data, Target::Struct, GenericRequirement::Exact(1);

ManuallyDrop, sym::manually_drop, manually_drop, Target::Struct, GenericRequirement::None;
BikeshedGuaranteedNoDrop, sym::bikeshed_guaranteed_no_drop, bikeshed_guaranteed_no_drop, Target::Trait, GenericRequirement::Exact(0);

MaybeUninit, sym::maybe_uninit, maybe_uninit, Target::Union, GenericRequirement::None;

Expand Down
116 changes: 52 additions & 64 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_data_structures::unord::{UnordMap, UnordSet};
use rustc_errors::MultiSpan;
use rustc_errors::codes::*;
use rustc_hir::def::{CtorKind, DefKind};
use rustc_hir::{Node, intravisit};
use rustc_hir::{LangItem, Node, intravisit};
use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
use rustc_infer::traits::{Obligation, ObligationCauseCode};
use rustc_lint_defs::builtin::{
Expand All @@ -27,6 +27,7 @@ use rustc_session::lint::builtin::UNINHABITED_STATIC;
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
use rustc_trait_selection::error_reporting::traits::on_unimplemented::OnUnimplementedDirective;
use rustc_trait_selection::traits;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_type_ir::fold::TypeFoldable;
use tracing::{debug, instrument};
use ty::TypingMode;
Expand Down Expand Up @@ -87,89 +88,76 @@ fn allowed_union_or_unsafe_field<'tcx>(
typing_env: ty::TypingEnv<'tcx>,
span: Span,
) -> bool {
// We don't just accept all !needs_drop fields, due to semver concerns.
let allowed = match ty.kind() {
ty::Ref(..) => true, // references never drop (even mutable refs, which are non-Copy and hence fail the later check)
ty::Tuple(tys) => {
// allow tuples of allowed types
tys.iter().all(|ty| allowed_union_or_unsafe_field(tcx, ty, typing_env, span))
}
ty::Array(elem, _len) => {
// Like `Copy`, we do *not* special-case length 0.
allowed_union_or_unsafe_field(tcx, *elem, typing_env, span)
}
_ => {
// Fallback case: allow `ManuallyDrop` and things that are `Copy`,
// also no need to report an error if the type is unresolved.
ty.ty_adt_def().is_some_and(|adt_def| adt_def.is_manually_drop())
|| tcx.type_is_copy_modulo_regions(typing_env, ty)
|| ty.references_error()
}
};
if allowed && ty.needs_drop(tcx, typing_env) {
// This should never happen. But we can get here e.g. in case of name resolution errors.
tcx.dcx()
.span_delayed_bug(span, "we should never accept maybe-dropping union or unsafe fields");
// HACK (not that bad of a hack don't worry): Some codegen tests don't even define proper
// impls for `Copy`. Let's short-circuit here for this validity check, since a lot of them
// use unions. We should eventually fix all the tests to define that lang item or use
// minicore stubs.
if ty.is_trivially_pure_clone_copy() {
return true;
}
allowed
// If `BikeshedGuaranteedNoDrop` is not defined in a `#[no_core]` test, fall back to `Copy`.
// This is an underapproximation of `BikeshedGuaranteedNoDrop`,
let def_id = tcx
.lang_items()
.get(LangItem::BikeshedGuaranteedNoDrop)
.unwrap_or_else(|| tcx.require_lang_item(LangItem::Copy, Some(span)));
let Ok(ty) = tcx.try_normalize_erasing_regions(typing_env, ty) else {
tcx.dcx().span_delayed_bug(span, "could not normalize field type");
return true;
};
let (infcx, param_env) = tcx.infer_ctxt().build_with_typing_env(typing_env);
infcx.predicate_must_hold_modulo_regions(&Obligation::new(
tcx,
ObligationCause::dummy_with_span(span),
param_env,
ty::TraitRef::new(tcx, def_id, [ty]),
))
}

/// Check that the fields of the `union` do not need dropping.
fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> bool {
let item_type = tcx.type_of(item_def_id).instantiate_identity();
if let ty::Adt(def, args) = item_type.kind() {
assert!(def.is_union());

let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
for field in &def.non_enum_variant().fields {
let Ok(field_ty) = tcx.try_normalize_erasing_regions(typing_env, field.ty(tcx, args))
else {
tcx.dcx().span_delayed_bug(span, "could not normalize field type");
continue;
};
let def = tcx.adt_def(item_def_id);
assert!(def.is_union());

if !allowed_union_or_unsafe_field(tcx, field_ty, typing_env, span) {
let (field_span, ty_span) = match tcx.hir().get_if_local(field.did) {
// We are currently checking the type this field came from, so it must be local.
Some(Node::Field(field)) => (field.span, field.ty.span),
_ => unreachable!("mir field has to correspond to hir field"),
};
tcx.dcx().emit_err(errors::InvalidUnionField {
field_span,
sugg: errors::InvalidUnionFieldSuggestion {
lo: ty_span.shrink_to_lo(),
hi: ty_span.shrink_to_hi(),
},
note: (),
});
return false;
}
let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
let args = ty::GenericArgs::identity_for_item(tcx, item_def_id);

for field in &def.non_enum_variant().fields {
if !allowed_union_or_unsafe_field(tcx, field.ty(tcx, args), typing_env, span) {
let (field_span, ty_span) = match tcx.hir().get_if_local(field.did) {
// We are currently checking the type this field came from, so it must be local.
Some(Node::Field(field)) => (field.span, field.ty.span),
_ => unreachable!("mir field has to correspond to hir field"),
};
tcx.dcx().emit_err(errors::InvalidUnionField {
field_span,
sugg: errors::InvalidUnionFieldSuggestion {
lo: ty_span.shrink_to_lo(),
hi: ty_span.shrink_to_hi(),
},
note: (),
});
return false;
}
} else {
span_bug!(span, "unions must be ty::Adt, but got {:?}", item_type.kind());
}

true
}

/// Check that the unsafe fields do not need dropping.
fn check_unsafe_fields(tcx: TyCtxt<'_>, item_def_id: LocalDefId) {
let span = tcx.def_span(item_def_id);
let item_type = tcx.type_of(item_def_id).instantiate_identity();
let ty::Adt(def, args) = item_type.kind() else {
span_bug!(span, "structs/enums must be ty::Adt, but got {:?}", item_type.kind());
};
let def = tcx.adt_def(item_def_id);

let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
let args = ty::GenericArgs::identity_for_item(tcx, item_def_id);

for field in def.all_fields() {
if !field.safety.is_unsafe() {
continue;
}
let Ok(field_ty) = tcx.try_normalize_erasing_regions(typing_env, field.ty(tcx, args))
else {
tcx.dcx().span_delayed_bug(span, "could not normalize field type");
continue;
};

if !allowed_union_or_unsafe_field(tcx, field_ty, typing_env, span) {
if !allowed_union_or_unsafe_field(tcx, field.ty(tcx, args), typing_env, span) {
let hir::Node::Field(field) = tcx.hir_node_by_def_id(field.did.expect_local()) else {
unreachable!("field has to correspond to hir field")
};
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ pub enum SelectionCandidate<'tcx> {
BuiltinObjectCandidate,

BuiltinUnsizeCandidate,

BikeshedGuaranteedNoDropCandidate,
}

/// The result of trait evaluation. The order is important
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,10 @@ impl<'tcx> rustc_type_ir::inherent::AdtDef<TyCtxt<'tcx>> for AdtDef<'tcx> {
self.is_phantom_data()
}

fn is_manually_drop(self) -> bool {
self.is_manually_drop()
}

fn all_field_tys(
self,
tcx: TyCtxt<'tcx>,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,7 @@ bidirectional_lang_item_map! {
AsyncFnOnce,
AsyncFnOnceOutput,
AsyncIterator,
BikeshedGuaranteedNoDrop,
CallOnceFuture,
CallRefFuture,
Clone,
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,11 @@ where
goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution>;

fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
ecx: &mut EvalCtxt<'_, D>,
goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution>;

/// Consider (possibly several) candidates to upcast or unsize a type to another
/// type, excluding the coercion of a sized type into a `dyn Trait`.
///
Expand Down Expand Up @@ -478,6 +483,9 @@ where
Some(TraitSolverLangItem::TransmuteTrait) => {
G::consider_builtin_transmute_candidate(self, goal)
}
Some(TraitSolverLangItem::BikeshedGuaranteedNoDrop) => {
G::consider_builtin_bikeshed_guaranteed_no_drop_candidate(self, goal)
}
_ => Err(NoSolution),
}
};
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_next_trait_solver/src/solve/effect_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,13 @@ where
unreachable!("TransmuteFrom is not const")
}

fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
_ecx: &mut EvalCtxt<'_, D>,
_goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution> {
unreachable!("BikeshedGuaranteedNoDrop is not const");
}

fn consider_structural_builtin_unsize_candidates(
_ecx: &mut EvalCtxt<'_, D>,
_goal: Goal<I, Self>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,13 @@ where
) -> Result<Candidate<I>, NoSolution> {
panic!("`TransmuteFrom` does not have an associated type: {:?}", goal)
}

fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
_ecx: &mut EvalCtxt<'_, D>,
goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution> {
unreachable!("`BikeshedGuaranteedNoDrop` does not have an associated type: {:?}", goal)
}
}

impl<D, I> EvalCtxt<'_, D>
Expand Down
95 changes: 95 additions & 0 deletions compiler/rustc_next_trait_solver/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,101 @@ where
})
}

/// NOTE: This is implemented as a built-in goal and not a set of impls like:
///
/// ```rust,ignore (illustrative)
/// impl<T> BikeshedGuaranteedNoDrop for T where T: Copy {}
/// impl<T> BikeshedGuaranteedNoDrop for ManuallyDrop<T> {}
/// ```
///
/// because these impls overlap, and I'd rather not build a coherence hack for
/// this harmless overlap.
fn consider_builtin_bikeshed_guaranteed_no_drop_candidate(
ecx: &mut EvalCtxt<'_, D>,
goal: Goal<I, Self>,
) -> Result<Candidate<I>, NoSolution> {
if goal.predicate.polarity != ty::PredicatePolarity::Positive {
return Err(NoSolution);
}

let cx = ecx.cx();
ecx.probe_builtin_trait_candidate(BuiltinImplSource::Misc).enter(|ecx| {
let ty = goal.predicate.self_ty();
match ty.kind() {
// `&mut T` and `&T` always implement `BikeshedGuaranteedNoDrop`.
ty::Ref(..) => {}
// `ManuallyDrop<T>` always implements `BikeshedGuaranteedNoDrop`.
ty::Adt(def, _) if def.is_manually_drop() => {}
// Arrays and tuples implement `BikeshedGuaranteedNoDrop` only if
// their constituent types implement `BikeshedGuaranteedNoDrop`.
ty::Tuple(tys) => {
ecx.add_goals(
GoalSource::ImplWhereBound,
tys.iter().map(|elem_ty| {
goal.with(cx, ty::TraitRef::new(cx, goal.predicate.def_id(), [elem_ty]))
}),
);
}
ty::Array(elem_ty, _) => {
ecx.add_goal(
GoalSource::ImplWhereBound,
goal.with(cx, ty::TraitRef::new(cx, goal.predicate.def_id(), [elem_ty])),
);
}

// All other types implement `BikeshedGuaranteedNoDrop` only if
// they implement `Copy`. We could be smart here and short-circuit
// some trivially `Copy`/`!Copy` types, but there's no benefit.
ty::FnDef(..)
| ty::FnPtr(..)
| ty::Error(_)
| ty::Uint(_)
| ty::Int(_)
| ty::Infer(ty::IntVar(_) | ty::FloatVar(_))
| ty::Bool
| ty::Float(_)
| ty::Char
| ty::RawPtr(..)
| ty::Never
| ty::Pat(..)
| ty::Dynamic(..)
| ty::Str
| ty::Slice(_)
| ty::Foreign(..)
| ty::Adt(..)
| ty::Alias(..)
| ty::Param(_)
| ty::Placeholder(..)
| ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Coroutine(..)
| ty::UnsafeBinder(_)
| ty::CoroutineWitness(..) => {
ecx.add_goal(
GoalSource::ImplWhereBound,
goal.with(
cx,
ty::TraitRef::new(
cx,
cx.require_lang_item(TraitSolverLangItem::Copy),
[ty],
),
),
);
}

ty::Bound(..)
| ty::Infer(
ty::TyVar(_) | ty::FreshTy(_) | ty::FreshIntTy(_) | ty::FreshFloatTy(_),
) => {
panic!("unexpected type `{ty:?}`")
}
}

ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
})
}

/// ```ignore (builtin impl example)
/// trait Trait {
/// fn foo(&self);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ symbols! {
bang,
begin_panic,
bench,
bikeshed_guaranteed_no_drop,
bin,
binaryheap_iter,
bind_by_move_pattern_guards,
Expand Down
Loading
Loading