Skip to content

GCI: Don't evaluate the initializer of free const items that have trivially unsatisfied predicates #142293

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+198 −195
Draft

GCI: Don't evaluate the initializer of free const items that have trivially unsatisfied predicates #142293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 12 additions & 6 deletions compiler/rustc_hir_analysis/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,18 @@ pub fn check_crate(tcx: TyCtxt<'_>) {
check::maybe_check_static_with_link_section(tcx, item_def_id);
}
DefKind::Const if !tcx.generics_of(item_def_id).own_requires_monomorphization() => {
// FIXME(generic_const_items): Passing empty instead of identity args is fishy but
// seems to be fine for now. Revisit this!
let instance = ty::Instance::new_raw(item_def_id.into(), ty::GenericArgs::empty());
let cid = GlobalId { instance, promoted: None };
let typing_env = ty::TypingEnv::fully_monomorphized();
tcx.ensure_ok().eval_to_const_value_raw(typing_env.as_query_input(cid));
let predicates = tcx.predicates_of(item_def_id);
let predicates = predicates.instantiate_identity(tcx).predicates;

if !traits::impossible_predicates(tcx, predicates) {
// FIXME(generic_const_items): Passing empty instead of identity args is fishy but
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be fixed yeah 🤔 I don't think ctfe should have to handle there being instances with wrong args

// seems to be fine for now. Revisit this!
let instance =
ty::Instance::new_raw(item_def_id.into(), ty::GenericArgs::empty());
let cid = GlobalId { instance, promoted: None };
let typing_env = ty::TypingEnv::fully_monomorphized();
tcx.ensure_ok().eval_to_const_value_raw(typing_env.as_query_input(cid));
}
}
_ => (),
}
Expand Down
16 changes: 10 additions & 6 deletions compiler/rustc_middle/src/mir/interpret/queries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ use crate::ty::{self, ConstToValTreeResult, GenericArgs, TyCtxt, TypeVisitableEx
use crate::{error, mir};

impl<'tcx> TyCtxt<'tcx> {
/// Evaluates a constant without providing any generic parameters. This is useful to evaluate consts
/// that can't take any generic arguments like const items or enum discriminants. If a
/// generic parameter is used within the constant `ErrorHandled::TooGeneric` will be returned.
/// Evaluates a constant without providing any generic parameters.
///
/// This is useful to evaluate consts that can't take any generic arguments like enum
/// discriminants. If a non-region generic parameter is used within the constant
/// `ErrorHandled::TooGeneric` will be returned.
#[instrument(skip(self), level = "debug")]
pub fn const_eval_poly(self, def_id: DefId) -> EvalToConstValueResult<'tcx> {
// In some situations def_id will have generic parameters within scope, but they aren't allowed
Expand All @@ -28,9 +30,11 @@ impl<'tcx> TyCtxt<'tcx> {
self.const_eval_global_id(typing_env, cid, DUMMY_SP)
}

/// Evaluates a constant without providing any generic parameters. This is useful to evaluate consts
/// that can't take any generic arguments like const items or enum discriminants. If a
/// generic parameter is used within the constant `ErrorHandled::TooGeneric` will be returned.
/// Evaluates a constant without providing any generic parameters.
///
/// This is useful to evaluate consts that can't take any generic arguments like enum
/// discriminants. If a non-region generic parameter is used within the constant
/// `ErrorHandled::TooGeneric` will be returned.
#[instrument(skip(self), level = "debug")]
pub fn const_eval_poly_to_alloc(self, def_id: DefId) -> EvalToAllocationRawResult<'tcx> {
// In some situations def_id will have generic parameters within scope, but they aren't allowed
Expand Down
140 changes: 69 additions & 71 deletions compiler/rustc_monomorphize/src/collector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1424,74 +1424,68 @@ struct RootCollector<'a, 'tcx> {
entry_fn: Option<(DefId, EntryFnType)>,
}

impl<'v> RootCollector<'_, 'v> {
impl<'tcx> RootCollector<'_, 'tcx> {
fn process_item(&mut self, id: hir::ItemId) {
match self.tcx.def_kind(id.owner_id) {
let tcx = self.tcx;
let def_id = id.owner_id.to_def_id();

match tcx.def_kind(def_id) {
DefKind::Enum | DefKind::Struct | DefKind::Union => {
if self.strategy == MonoItemCollectionStrategy::Eager
&& !self.tcx.generics_of(id.owner_id).requires_monomorphization(self.tcx)
{
debug!("RootCollector: ADT drop-glue for `{id:?}`",);
let id_args =
ty::GenericArgs::for_item(self.tcx, id.owner_id.to_def_id(), |param, _| {
match param.kind {
GenericParamDefKind::Lifetime => {
self.tcx.lifetimes.re_erased.into()
}
GenericParamDefKind::Type { .. }
| GenericParamDefKind::Const { .. } => {
unreachable!(
"`own_requires_monomorphization` check means that \
we should have no type/const params"
)
}
}
});

// This type is impossible to instantiate, so we should not try to
// generate a `drop_in_place` instance for it.
if self.tcx.instantiate_and_check_impossible_predicates((
id.owner_id.to_def_id(),
id_args,
)) {
return;
}
if self.strategy == MonoItemCollectionStrategy::Lazy {
return;
}

let ty =
self.tcx.type_of(id.owner_id.to_def_id()).instantiate(self.tcx, id_args);
assert!(!ty.has_non_region_param());
visit_drop_use(self.tcx, ty, true, DUMMY_SP, self.output);
if tcx.generics_of(def_id).own_requires_monomorphization() {
return;
}
let args = ty::GenericArgs::for_item(tcx, def_id, |param, _| {
expect_and_erase_regions(tcx, param)
});
if tcx.instantiate_and_check_impossible_predicates((def_id, args)) {
return;
}

let ty = tcx.type_of(def_id).instantiate(tcx, args);
debug_assert!(!ty.has_non_region_param());

debug!("RootCollector: ADT drop-glue for `{id:?}`");
visit_drop_use(tcx, ty, true, DUMMY_SP, self.output);
}
DefKind::GlobalAsm => {
debug!(
"RootCollector: ItemKind::GlobalAsm({})",
self.tcx.def_path_str(id.owner_id)
);
debug!("RootCollector: ItemKind::GlobalAsm({})", tcx.def_path_str(def_id));
self.output.push(dummy_spanned(MonoItem::GlobalAsm(id)));
}
DefKind::Static { .. } => {
let def_id = id.owner_id.to_def_id();
debug!("RootCollector: ItemKind::Static({})", self.tcx.def_path_str(def_id));
debug!("RootCollector: ItemKind::Static({})", tcx.def_path_str(def_id));
self.output.push(dummy_spanned(MonoItem::Static(def_id)));
}
DefKind::Const => {
// Const items only generate mono items if they are actually used somewhere.
// Just declaring them is insufficient.

// If we're collecting items eagerly, then recurse into all constants.
// Otherwise the value is only collected when explicitly mentioned in other items.
if self.strategy == MonoItemCollectionStrategy::Eager {
if !self.tcx.generics_of(id.owner_id).own_requires_monomorphization()
&& let Ok(val) = self.tcx.const_eval_poly(id.owner_id.to_def_id())
{
collect_const_value(self.tcx, val, self.output);
}
if self.strategy == MonoItemCollectionStrategy::Lazy {
return;
}

if tcx.generics_of(def_id).own_requires_monomorphization() {
return;
}

let args = ty::GenericArgs::for_item(tcx, def_id, |param, _| {
expect_and_erase_regions(tcx, param)
});
if tcx.instantiate_and_check_impossible_predicates((def_id, args)) {
return;
}

let Ok(val) = tcx.const_eval_poly(def_id) else { return };

collect_const_value(tcx, val, self.output);
}
DefKind::Impl { .. } => {
if self.strategy == MonoItemCollectionStrategy::Eager {
create_mono_items_for_default_impls(self.tcx, id, self.output);
create_mono_items_for_default_impls(tcx, id, self.output);
}
}
DefKind::Fn => {
Expand Down Expand Up @@ -1614,35 +1608,23 @@ fn create_mono_items_for_default_impls<'tcx>(
item: hir::ItemId,
output: &mut MonoItems<'tcx>,
) {
let Some(impl_) = tcx.impl_trait_header(item.owner_id) else {
return;
};

let impl_def_id = item.owner_id.to_def_id();
let Some(impl_) = tcx.impl_trait_header(impl_def_id) else { return };
if matches!(impl_.polarity, ty::ImplPolarity::Negative) {
return;
}

if tcx.generics_of(item.owner_id).own_requires_monomorphization() {
return;
}

// Lifetimes never affect trait selection, so we are allowed to eagerly
// instantiate an instance of an impl method if the impl (and method,
// which we check below) is only parameterized over lifetime. In that case,
// we use the ReErased, which has no lifetime information associated with
// it, to validate whether or not the impl is legal to instantiate at all.
let only_region_params = |param: &ty::GenericParamDef, _: &_| match param.kind {
GenericParamDefKind::Lifetime => tcx.lifetimes.re_erased.into(),
GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => {
unreachable!(
"`own_requires_monomorphization` check means that \
we should have no type/const params"
)
}
};
let impl_args = GenericArgs::for_item(tcx, item.owner_id.to_def_id(), only_region_params);
let trait_ref = impl_.trait_ref.instantiate(tcx, impl_args);

if tcx.generics_of(impl_def_id).own_requires_monomorphization() {
return;
}
let impl_args = ty::GenericArgs::for_item(tcx, impl_def_id, |param, _| {
expect_and_erase_regions(tcx, param)
});
// Unlike 'lazy' monomorphization that begins by collecting items transitively
// called by `main` or other global items, when eagerly monomorphizing impl
// items, we never actually check that the predicates of this impl are satisfied
Expand All @@ -1652,13 +1634,15 @@ fn create_mono_items_for_default_impls<'tcx>(
// consider higher-ranked predicates such as `for<'a> &'a mut [u8]: Copy` to
// be trivially false. We must now check that the impl has no impossible-to-satisfy
// predicates.
if tcx.instantiate_and_check_impossible_predicates((item.owner_id.to_def_id(), impl_args)) {
if tcx.instantiate_and_check_impossible_predicates((impl_def_id, impl_args)) {
return;
}

let trait_ref = impl_.trait_ref.instantiate(tcx, impl_args);

let typing_env = ty::TypingEnv::fully_monomorphized();
let trait_ref = tcx.normalize_erasing_regions(typing_env, trait_ref);
let overridden_methods = tcx.impl_item_implementor_ids(item.owner_id);
let overridden_methods = tcx.impl_item_implementor_ids(impl_def_id);
for method in tcx.provided_trait_methods(trait_ref.def_id) {
if overridden_methods.contains_key(&method.def_id) {
continue;
Expand All @@ -1671,7 +1655,9 @@ fn create_mono_items_for_default_impls<'tcx>(
// As mentioned above, the method is legal to eagerly instantiate if it
// only has lifetime generic parameters. This is validated by calling
// `own_requires_monomorphization` on both the impl and method.
let args = trait_ref.args.extend_to(tcx, method.def_id, only_region_params);
let args = trait_ref
.args
.extend_to(tcx, method.def_id, |param, _| expect_and_erase_regions(tcx, param));
let instance = ty::Instance::expect_resolve(tcx, typing_env, method.def_id, args, DUMMY_SP);

let mono_item = create_fn_mono_item(tcx, instance, DUMMY_SP);
Expand All @@ -1681,6 +1667,18 @@ fn create_mono_items_for_default_impls<'tcx>(
}
}

fn expect_and_erase_regions<'tcx>(
tcx: TyCtxt<'tcx>,
param: &ty::GenericParamDef,
) -> ty::GenericArg<'tcx> {
match param.kind {
GenericParamDefKind::Lifetime => tcx.lifetimes.re_erased.into(),
GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => {
bug!("unexpected non-region param")
}
}
}

//=-----------------------------------------------------------------------------
// Top-level entry point, tying it all together
//=-----------------------------------------------------------------------------
Expand Down
32 changes: 24 additions & 8 deletions compiler/rustc_passes/src/reachable.rs
View file
Open in desktop
Copy link
Member Author

@fmease fmease Jun 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should consider moving all of these call-site checks into ctfe itself, e.g. eval_to_const_value_raw_provider. This wont affect const patterns which go through _for_typeck versions of the CTFE entry points.

As discussed, long term, that would be the way to go. However, it makes me uncomfortable that eval_to_const_value_raw has so many transitive callers which I wasn't yet able to audit entirely. I know that you told me that this function should be safe to modify from a soundness perspective and you're probably right, I just haven't finished tracing the entire caller graph. WIP:

Caller graph (X called by Y) 
+ eval_to_const_value_raw  [eval_to_const_value_raw_provider]  (???)
+-- check_crate/par_hir_body_owners/DefKind::Const  (OK)
+-- const_eval_global_id  (???)
    +-- eval_intrinsic  (OK)
    +-- const_eval_poly  (OK)
    |   +-- codegen_global_asm  (OK)
    |   +-- check_type_defn/variants/fields [default_field_values]  (OK)
    |   +-- check_type_defn/ty::VariantDiscr::Explicit  (OK)
    |   +-- eval_explicit_discr  (OK)
    |   +-- monomorphize/RootCollector/process_item/DefKind::Const  (OK)
    +-- const_eval_resolve  (???)
    |   +-- Const::eval  (???)
    |       +-- eval_mir_constant  (???)
    |       |   +... [~20 call sites]  (TODO)
    |       +-- inline_to_global_operand/InlineAsmOperand::Const  (OK)
    |       +-- push_stack_frame_raw/body.required_consts()  (???)
    |       +-- try_eval_scalar...  (TODO)
    |       +-- MIR transform/simplify_repeated_aggregate  (???)
    |       +-- eval_constant  (???)
    |           +-- eval_operand...  (TODO)
    |           +-- MIR transform/ConstPropagator/visit_const_operand  (???)
    |           +-- collect_items_of_instance/body.requires_consts()  (???)
    +-- const_eval_instance  (???)
        +-- codegen_regular_intrinsic_call  (OK)
        +-- codegen_intrinsic_call  (OK)
        +-- eval_instance  (???)
            +-- try_const_eval [SMIR]  (???)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(As an aside, even if we enriched eval_to_const_value_raw_provider with a call to impossible_predicates, here in mod reachable it would be of no use to us as it uses const_eval_poly_to_alloc which doesn't depend on that function)

Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use rustc_middle::query::Providers;
use rustc_middle::ty::{self, ExistentialTraitRef, TyCtxt};
use rustc_privacy::DefIdVisitor;
use rustc_session::config::CrateType;
use rustc_trait_selection::traits;
use tracing::debug;

/// Determines whether this item is recursive for reachability. See `is_recursively_reachable_local`
Expand Down Expand Up @@ -205,19 +206,34 @@ impl<'tcx> ReachableContext<'tcx> {
}

hir::ItemKind::Const(_, _, _, init) => {
// Only things actually ending up in the final constant value are reachable
// for codegen. Everything else is only needed during const-eval, so even if
// const-eval happens in a downstream crate, all they need is
// `mir_for_ctfe`.
if self.tcx.generics_of(item.owner_id).own_requires_monomorphization() {
// In this case, we don't want to evaluate the const initializer.
// In lieu of that, we have to consider everything mentioned in it
// as reachable, since it *may* end up in the final value.
self.visit_nested_body(init);
return;
}

let predicates = self.tcx.predicates_of(item.owner_id);
let predicates = predicates.instantiate_identity(self.tcx).predicates;
if traits::impossible_predicates(self.tcx, predicates) {
// The constant is impossible to reference.
// Therefore nothing can be reachable via it.
return;
}

match self.tcx.const_eval_poly_to_alloc(item.owner_id.def_id.into()) {
Ok(alloc) => {
// Only things actually ending up in the final constant value are
// reachable for codegen. Everything else is only needed during
// const-eval, so even if const-eval happens in a downstream crate,
// all they need is `mir_for_ctfe`.
let alloc = self.tcx.global_alloc(alloc.alloc_id).unwrap_memory();
self.propagate_from_alloc(alloc);
}
// We can't figure out which value the constant will evaluate to. In
// lieu of that, we have to consider everything mentioned in the const
// initializer reachable, since it *may* end up in the final value.
Err(ErrorHandled::TooGeneric(_)) => self.visit_nested_body(init),
// We've checked at the start that there aren't any non-lifetime params
// in scope. The const initializer can't possibly be too generic.
Err(ErrorHandled::TooGeneric(_)) => bug!(),
// If there was an error evaluating the const, nothing can be reachable
// via it, and anyway compilation will fail.
Err(ErrorHandled::Reported(..)) => {}
Expand Down
22 changes: 12 additions & 10 deletions compiler/rustc_trait_selection/src/traits/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,8 +721,13 @@ fn replace_param_and_infer_args_with_placeholder<'tcx>(
/// returns true, then either normalize encountered an error or one of the predicates did not
/// hold. Used when creating vtables to check for unsatisfiable methods. This should not be
/// used during analysis.
#[instrument(level = "debug", skip(tcx))]
pub fn impossible_predicates<'tcx>(tcx: TyCtxt<'tcx>, predicates: Vec<ty::Clause<'tcx>>) -> bool {
debug!("impossible_predicates(predicates={:?})", predicates);
debug_assert!(!predicates.has_non_region_param());
if predicates.is_empty() {
return false;
}

let (infcx, param_env) = tcx
.infer_ctxt()
.with_next_trait_solver(true)
Expand All @@ -749,26 +754,23 @@ pub fn impossible_predicates<'tcx>(tcx: TyCtxt<'tcx>, predicates: Vec<ty::Clause
false
}

#[instrument(level = "debug", skip(tcx), ret)]
fn instantiate_and_check_impossible_predicates<'tcx>(
tcx: TyCtxt<'tcx>,
key: (DefId, GenericArgsRef<'tcx>),
(def_id, args): (DefId, GenericArgsRef<'tcx>),
) -> bool {
debug!("instantiate_and_check_impossible_predicates(key={:?})", key);

let mut predicates = tcx.predicates_of(key.0).instantiate(tcx, key.1).predicates;
let mut predicates = tcx.predicates_of(def_id).instantiate(tcx, args).predicates;

// Specifically check trait fulfillment to avoid an error when trying to resolve
// associated items.
if let Some(trait_def_id) = tcx.trait_of_item(key.0) {
let trait_ref = ty::TraitRef::from_method(tcx, trait_def_id, key.1);
if let Some(trait_def_id) = tcx.trait_of_item(def_id) {
let trait_ref = ty::TraitRef::from_method(tcx, trait_def_id, args);
predicates.push(trait_ref.upcast(tcx));
}

predicates.retain(|predicate| !predicate.has_param());
let result = impossible_predicates(tcx, predicates);

debug!("instantiate_and_check_impossible_predicates(key={:?}) = {:?}", key, result);
result
impossible_predicates(tcx, predicates)
}

/// Checks whether a trait's associated item is impossible to reference on a given impl.
Expand Down
27 changes: 0 additions & 27 deletions tests/codegen/dont_codegen_private_const_fn_only_used_in_const_eval.rs
View file
Open in desktop

This file was deleted.

Loading