Fix vtable ICE on unsatisfied supertrait bounds

When a trait impl doesn't satisfy its supertrait bounds (for example,
`impl<T: Send> Bar for T` where `trait Bar: Droppable` but `T` has no
`Droppable` bound), the compiler emits E0277 and then then continues
compilation.

If a static initialiser then upcasts through this broken impl, const
evaluation builds the vtable and tries to resolve methods for the
unsatisfied supertrait (for example, `<bool as Droppable>::drop`).

One might think that the `impossible_predicates` guard in
`vtable_entries` should catch this, but it doesn't, because it calls
`predicates_of(method).instantiate_own()`, which only includes the
method's own where clauses, and the parent trait's `Self: Trait`
predicate lives on the trait definition, not the method, so the guard
passes and we fall through to `expect_resolve_for_vtable`, which hits a
`span_bug!` when resolution returns `Ok(None)`.

There was a previous attempt to fix this in issue 152287, which worked
by switching to `instantiate_and_check_impossible_predicates`, which
includes parent predicates and the trait ref. But unfortunately this is
unsound and was reverted because `impossible_predicates` has a
pre-existing unsoundness with coroutine witness types and opaque types
(see issue 153596).

Let's take a different approach that avoids `impossible_predicates`
entirely. Let's extract the core of `expect_resolve_for_vtable` into a
fallible `try_resolve_for_vtable` that returns `Result<Instance,
ErrorGuaranteed>`, and on resolution failure emit a `delayed_bug`.
`vtable_entries` then can use this and return `VtblEntry::Vacant` on the
error path.

Author: cdown

compiler/rustc_middle/src/ty/instance.rs

@@ -625,25 +625,36 @@ impl<'tcx> Instance<'tcx> {
         })
     }
 
-    pub fn expect_resolve_for_vtable(
+    /// Fallible version of [`Instance::expect_resolve_for_vtable`].
+    ///
+    /// Returns `Err` if instance resolution fails, e.g. due to a broken impl with unsatisfied
+    /// supertrait bounds.
+    // FIXME(#153596): Once `impossible_predicates` is sound again, use that to reject impossible
+    // vtable entries up front and remove this function.
+    pub fn try_resolve_for_vtable(
         tcx: TyCtxt<'tcx>,
         typing_env: ty::TypingEnv<'tcx>,
         def_id: DefId,
         args: GenericArgsRef<'tcx>,
-        span: Span,
-    ) -> Instance<'tcx> {
-        debug!("resolve_for_vtable(def_id={:?}, args={:?})", def_id, args);
+    ) -> Result<Instance<'tcx>, ErrorGuaranteed> {
+        debug!("try_resolve_for_vtable(def_id={:?}, args={:?})", def_id, args);
         let fn_sig = tcx.fn_sig(def_id).instantiate_identity();
         let is_vtable_shim = !fn_sig.inputs().skip_binder().is_empty()
             && fn_sig.input(0).skip_binder().is_param(0)
             && tcx.generics_of(def_id).has_self;
 
         if is_vtable_shim {
             debug!(" => associated item with unsizeable self: Self");
-            return Instance { def: InstanceKind::VTableShim(def_id), args };
+            return Ok(Instance { def: InstanceKind::VTableShim(def_id), args });
         }
 
-        let mut resolved = Instance::expect_resolve(tcx, typing_env, def_id, args, span);
+        let mut resolved =
+            Instance::try_resolve(tcx, typing_env, def_id, args)?.ok_or_else(|| {
+                tcx.dcx().delayed_bug(format!(
+                    "failed to resolve instance for vtable: {}",
+                    tcx.def_path_str_with_args(def_id, args)
+                ))
+            })?;
 
         let reason = tcx.sess.is_sanitizer_kcfi_enabled().then_some(ReifyReason::Vtable);
         match resolved.def {
@@ -700,7 +711,25 @@ impl<'tcx> Instance<'tcx> {
             _ => {}
         }
 
-        resolved
+        Ok(resolved)
+    }
+
+    pub fn expect_resolve_for_vtable(
+        tcx: TyCtxt<'tcx>,
+        typing_env: ty::TypingEnv<'tcx>,
+        def_id: DefId,
+        args: GenericArgsRef<'tcx>,
+        span: Span,
+    ) -> Instance<'tcx> {
+        Instance::try_resolve_for_vtable(tcx, typing_env, def_id, args).unwrap_or_else(|e| {
+            let span =
+                if span.is_dummy() && def_id.is_local() { tcx.def_span(def_id) } else { span };
+            span_bug!(
+                span,
+                "failed to resolve instance for {}: {e:#?}",
+                tcx.def_path_str_with_args(def_id, args)
+            )
+        })
     }
 
     pub fn resolve_closure(

compiler/rustc_trait_selection/src/traits/vtable.rs

@@ -8,7 +8,6 @@ use rustc_middle::query::Providers;
 use rustc_middle::ty::{
     self, GenericArgs, GenericParamDefKind, Ty, TyCtxt, TypeVisitableExt, Upcast, VtblEntry,
 };
-use rustc_span::DUMMY_SP;
 use smallvec::{SmallVec, smallvec};
 use tracing::debug;
 
@@ -285,15 +284,22 @@ fn vtable_entries<'tcx>(
                         return VtblEntry::Vacant;
                     }
 
-                    let instance = ty::Instance::expect_resolve_for_vtable(
+                    match ty::Instance::try_resolve_for_vtable(
                         tcx,
                         ty::TypingEnv::fully_monomorphized(),
                         def_id,
                         args,
-                        DUMMY_SP,
-                    );
-
-                    VtblEntry::Method(instance)
+                    ) {
+                        Ok(instance) => VtblEntry::Method(instance),
+                        Err(_guar) => {
+                            // This can happen when building a vtable for a type
+                            // whose impl has unsatisfied supertrait bounds (an
+                            // error for which has already been emitted). In that
+                            // case the supertrait's methods can't be resolved,
+                            // so we mark the entry as vacant.
+                            VtblEntry::Vacant
+                        }
+                    }
                 });
 
                 entries.extend(own_entries);

tests/crashes/137190-2.rs

@@ -1,4 +1,4 @@
-//~ ERROR reached the type-length limit
+//~? ERROR reached the type-length limit
 
 //! Checks the enforcement of the type-length limit
 //! and its configurability via `#![type_length_limit]`.

tests/crashes/137190-3.rs

@@ -8,7 +8,8 @@ LL |     drop::<Option<A>>(None);
    = note: the full name for the type has been written to '$TEST_BUILD_DIR/type-length-limit-enforcement.long-type-$LONG_TYPE_HASH.txt'
    = note: consider using `--verbose` to print the full type name to the console
 
-error: reached the type-length limit while instantiating `<{closure@...} as FnMut<()>>::call_mut`
+error: reached the type-length limit while instantiating `<{closure@...} as FnOnce<()>>::call_once`
+  --> $SRC_DIR/core/src/ops/function.rs:LL:COL
    |
    = help: consider adding a `#![type_length_limit="10"]` attribute to your crate
    = note: the full name for the type has been written to '$TEST_BUILD_DIR/type-length-limit-enforcement.long-type-$LONG_TYPE_HASH.txt'

tests/ui/limits/type-length-limit-enforcement.rs

@@ -0,0 +1,14 @@
+// Regression test for #154073.
+// Verify that we don't ICE when building vtable entries
+// for a trait whose impl is missing a required method body.
+
+//@ compile-flags: --crate-type lib
+
+trait Bar: Sync {
+    fn method(&self);
+}
+impl<T: Sync> Bar for T {
+    //~^ ERROR not all trait items implemented
+}
+
+static BAR: &dyn Sync = &false as &dyn Bar;

tests/ui/limits/type-length-limit-enforcement.stderr

@@ -0,0 +1,12 @@
+error[E0046]: not all trait items implemented, missing: `method`
+  --> $DIR/vtable-missing-method-impl.rs:10:1
+   |
+LL |     fn method(&self);
+   |     ----------------- `method` from trait
+LL | }
+LL | impl<T: Sync> Bar for T {
+   | ^^^^^^^^^^^^^^^^^^^^^^^ missing `method` in implementation
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0046`.

tests/ui/traits/vtable/vtable-missing-method-impl.rs

@@ -0,0 +1,22 @@
+// Regression test for #154073.
+// Verify that we don't ICE when building vtable entries
+// for a trait whose supertrait is not implemented, during
+// const evaluation of a static initializer.
+
+//@ compile-flags: --crate-type lib
+
+trait Bar: Send + Sync + Droppable {}
+
+impl<T: Send> Bar for T {}
+//~^ ERROR the trait bound `T: Droppable` is not satisfied
+//~| ERROR `T` cannot be shared between threads safely
+
+trait Droppable {
+    fn drop(&self);
+}
+
+const fn upcast(x: &dyn Bar) -> &(dyn Send + Sync) {
+    x
+}
+
+static BAR: &(dyn Send + Sync) = upcast(&false);

tests/ui/traits/vtable/vtable-missing-method-impl.stderr

@@ -0,0 +1,35 @@
+error[E0277]: the trait bound `T: Droppable` is not satisfied
+  --> $DIR/vtable-unsatisfied-supertrait-const.rs:10:23
+   |
+LL | impl<T: Send> Bar for T {}
+   |                       ^ the trait `Droppable` is not implemented for `T`
+   |
+note: required by a bound in `Bar`
+  --> $DIR/vtable-unsatisfied-supertrait-const.rs:8:26
+   |
+LL | trait Bar: Send + Sync + Droppable {}
+   |                          ^^^^^^^^^ required by this bound in `Bar`
+help: consider further restricting type parameter `T` with trait `Droppable`
+   |
+LL | impl<T: Send + Droppable> Bar for T {}
+   |              +++++++++++
+
+error[E0277]: `T` cannot be shared between threads safely
+  --> $DIR/vtable-unsatisfied-supertrait-const.rs:10:23
+   |
+LL | impl<T: Send> Bar for T {}
+   |                       ^ `T` cannot be shared between threads safely
+   |
+note: required by a bound in `Bar`
+  --> $DIR/vtable-unsatisfied-supertrait-const.rs:8:19
+   |
+LL | trait Bar: Send + Sync + Droppable {}
+   |                   ^^^^ required by this bound in `Bar`
+help: consider further restricting type parameter `T` with trait `Sync`
+   |
+LL | impl<T: Send + std::marker::Sync> Bar for T {}
+   |              +++++++++++++++++++
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.