Skip to content

Commit

Permalink
Rollup merge of #128580 - compiler-errors:cfi-param-env, r=lcnr
Browse files Browse the repository at this point in the history
Use `ParamEnv::reveal_all` in CFI

I left a huge comment for why this ICEs in the test I committed.

`typeid_for_instance` should only be called on monomorphic instances during codegen, and we should just be using `ParamEnv::reveal_all()` rather than the param-env of the instance itself. I added an assertion to ensure that we only do this for fully substituted instances (this may break with polymorphization, but I kinda don't care lol).

Fixes #114160
cc `@rcvalle`
  • Loading branch information
matthiaskrgr authored Aug 5, 2024
2 parents 8cbf1c1 + b0beb64 commit 9cb3688
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use rustc_data_structures::fx::FxHashMap;
use rustc_middle::bug;
use rustc_middle::ty::{self, Instance, Ty, TyCtxt, TypeFoldable};
use rustc_middle::ty::{self, Instance, Ty, TyCtxt, TypeFoldable, TypeVisitableExt};
use rustc_target::abi::call::{Conv, FnAbi, PassMode};
use tracing::instrument;

Expand Down Expand Up @@ -112,11 +112,12 @@ pub fn typeid_for_instance<'tcx>(
instance: Instance<'tcx>,
options: TypeIdOptions,
) -> String {
assert!(!instance.has_non_region_param(), "{instance:#?} must be fully monomorphic");
let transform_ty_options = TransformTyOptions::from_bits(options.bits())
.unwrap_or_else(|| bug!("typeid_for_instance: invalid option(s) `{:?}`", options.bits()));
let instance = transform_instance(tcx, instance, transform_ty_options);
let fn_abi = tcx
.fn_abi_of_instance(tcx.param_env(instance.def_id()).and((instance, ty::List::empty())))
.fn_abi_of_instance(ty::ParamEnv::reveal_all().and((instance, ty::List::empty())))
.unwrap_or_else(|error| {
bug!("typeid_for_instance: couldn't get fn_abi of instance {instance:?}: {error:?}")
});
Expand Down
44 changes: 44 additions & 0 deletions tests/ui/sanitizer/cfi-can-reveal-opaques.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//@ needs-sanitizer-cfi
//@ compile-flags: -Ccodegen-units=1 -Clto -Ctarget-feature=-crt-static -Zsanitizer=cfi
//@ no-prefer-dynamic
//@ only-x86_64-unknown-linux-gnu
//@ build-pass

// See comment below for why this test exists.

trait Tr<U> {
type Projection;
}

impl<F, U> Tr<U> for F
where
F: Fn() -> U
{
type Projection = U;
}

fn test<B: Tr<U>, U>(b: B) -> B::Projection
{
todo!()
}

fn main() {
fn rpit_fn() -> impl Sized {}

// When CFI runs, it tries to compute the signature of the call. This
// ends up giving us a signature of:
// `fn test::<rpit_fn, ()>() -> <rpit_fn as Tr<()>>::Projection`,
// where `rpit_fn` is the ZST FnDef for the function. However, we were
// previously using a Reveal::UserFacing param-env. This means that the
// `<rpit_fn as Tr<()>>::Projection` return type is impossible to normalize,
// since it would require proving `rpit_fn: Fn() -> ()`, but we cannot
// prove that the `impl Sized` opaque is `()` with a user-facing param-env.
// This leads to a normalization error, and then an ICE.
//
// Side-note:
// So why is the second generic of `test` "`()`", and not the
// `impl Sized` since we inferred it from the return type of `rpit_fn`
// during typeck? Well, that's because we're using the generics from the
// terminator of the MIR, which has had the RevealAll pass performed on it.
let _ = test(rpit_fn);
}

0 comments on commit 9cb3688

Please sign in to comment.