Rollup merge of #95973 - oli-obk:tait_ub3, r=compiler-errors

prevent opaque types from appearing in impl headers cc `@lqd` opaque types are not distinguishable from their hidden type at the codegen stage. So we could either end up with cases where the hidden type doesn't implement the trait (which will thus ICE) or where the hidden type does implement the trait (so we'd be using its impl instead of the one written for the opaque type). This can even lead to unsound behaviour without unsafe code. Fixes #86411. Fixes #84660. rebase of #87382 plus some diagnostic tweaks
rust-lang · Apr 12, 2022 · e96304b · e96304b
2 parents d63a46a + 6d0349d
commit e96304b
Show file tree

Hide file tree

Showing 18 changed files with 204 additions and 48 deletions.
diff --git a/compiler/rustc_typeck/src/coherence/orphan.rs b/compiler/rustc_typeck/src/coherence/orphan.rs
@@ -7,6 +7,7 @@ use rustc_errors::ErrorGuaranteed;
 use rustc_hir as hir;
 use rustc_index::bit_set::GrowableBitSet;
 use rustc_infer::infer::TyCtxtInferExt;
+use rustc_middle::ty::subst::GenericArgKind;
 use rustc_middle::ty::subst::{GenericArg, InternalSubsts};
 use rustc_middle::ty::{self, ImplPolarity, Ty, TyCtxt, TypeFoldable, TypeVisitor};
 use rustc_session::lint;
@@ -141,13 +142,56 @@ fn orphan_check_impl(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(), ErrorGua
         }
     }
 
-    if let ty::Opaque(def_id, _) = *trait_ref.self_ty().kind() {
-        let reported = tcx
-            .sess
-            .struct_span_err(sp, "cannot implement trait on type alias impl trait")
-            .span_note(tcx.def_span(def_id), "type alias impl trait defined here")
-            .emit();
-        return Err(reported);
+    // Ensure no opaque types are present in this impl header. See issues #76202 and #86411 for examples,
+    // and #84660 where it would otherwise allow unsoundness.
+    if trait_ref.has_opaque_types() {
+        trace!("{:#?}", item);
+        // First we find the opaque type in question.
+        for ty in trait_ref.substs {
+            for ty in ty.walk() {
+                let ty::subst::GenericArgKind::Type(ty) = ty.unpack() else { continue };
+                let ty::Opaque(def_id, _) = *ty.kind() else { continue };
+                trace!(?def_id);
+
+                // Then we search for mentions of the opaque type's type alias in the HIR
+                struct SpanFinder<'tcx> {
+                    sp: Span,
+                    def_id: DefId,
+                    tcx: TyCtxt<'tcx>,
+                }
+                impl<'v, 'tcx> hir::intravisit::Visitor<'v> for SpanFinder<'tcx> {
+                    #[instrument(level = "trace", skip(self, _id))]
+                    fn visit_path(&mut self, path: &'v hir::Path<'v>, _id: hir::HirId) {
+                        // You can't mention an opaque type directly, so we look for type aliases
+                        if let hir::def::Res::Def(hir::def::DefKind::TyAlias, def_id) = path.res {
+                            // And check if that type alias's type contains the opaque type we're looking for
+                            for arg in self.tcx.type_of(def_id).walk() {
+                                if let GenericArgKind::Type(ty) = arg.unpack() {
+                                    if let ty::Opaque(def_id, _) = *ty.kind() {
+                                        if def_id == self.def_id {
+                                            // Finally we update the span to the mention of the type alias
+                                            self.sp = path.span;
+                                            return;
+                                        }
+                                    }
+                                }
+                            }
+                        }
+                        hir::intravisit::walk_path(self, path)
+                    }
+                }
+
+                let mut visitor = SpanFinder { sp, def_id, tcx };
+                hir::intravisit::walk_item(&mut visitor, item);
+                let reported = tcx
+                    .sess
+                    .struct_span_err(visitor.sp, "cannot implement trait on type alias impl trait")
+                    .span_note(tcx.def_span(def_id), "type alias impl trait defined here")
+                    .emit();
+                return Err(reported);
+            }
+        }
+        span_bug!(sp, "opaque type not found, but `has_opaque_types` is set")
     }
 
     Ok(())

diff --git a/src/test/ui/impl-trait/auto-trait.rs b/src/test/ui/impl-trait/auto-trait.rs
@@ -20,6 +20,7 @@ impl<T: Send> AnotherTrait for T {}
 // in the future.)
 impl AnotherTrait for D<OpaqueType> {
     //~^ ERROR conflicting implementations of trait `AnotherTrait` for type `D<OpaqueType>`
+    //~| ERROR cannot implement trait on type alias impl trait
 }
 
 fn main() {}
diff --git a/src/test/ui/impl-trait/auto-trait.stderr b/src/test/ui/impl-trait/auto-trait.stderr
@@ -1,3 +1,15 @@
+error: cannot implement trait on type alias impl trait
+  --> $DIR/auto-trait.rs:21:25
+   |
+LL | impl AnotherTrait for D<OpaqueType> {
+   |                         ^^^^^^^^^^
+   |
+note: type alias impl trait defined here
+  --> $DIR/auto-trait.rs:7:19
+   |
+LL | type OpaqueType = impl OpaqueTrait;
+   |                   ^^^^^^^^^^^^^^^^
+
 error[E0119]: conflicting implementations of trait `AnotherTrait` for type `D<OpaqueType>`
   --> $DIR/auto-trait.rs:21:1
    |
@@ -7,6 +19,6 @@ LL | impl<T: Send> AnotherTrait for T {}
 LL | impl AnotherTrait for D<OpaqueType> {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `D<OpaqueType>`
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0119`.
diff --git a/src/test/ui/impl-trait/negative-reasoning.rs b/src/test/ui/impl-trait/negative-reasoning.rs
@@ -18,6 +18,7 @@ impl<T: std::fmt::Debug> AnotherTrait for T {}
 // This is in error, because we cannot assume that `OpaqueType: !Debug`
 impl AnotherTrait for D<OpaqueType> {
     //~^ ERROR conflicting implementations of trait `AnotherTrait` for type `D<OpaqueType>`
+    //~| ERROR cannot implement trait on type alias impl trait
 }
 
 fn main() {}
diff --git a/src/test/ui/impl-trait/negative-reasoning.stderr b/src/test/ui/impl-trait/negative-reasoning.stderr
@@ -1,3 +1,15 @@
+error: cannot implement trait on type alias impl trait
+  --> $DIR/negative-reasoning.rs:19:25
+   |
+LL | impl AnotherTrait for D<OpaqueType> {
+   |                         ^^^^^^^^^^
+   |
+note: type alias impl trait defined here
+  --> $DIR/negative-reasoning.rs:7:19
+   |
+LL | type OpaqueType = impl OpaqueTrait;
+   |                   ^^^^^^^^^^^^^^^^
+
 error[E0119]: conflicting implementations of trait `AnotherTrait` for type `D<OpaqueType>`
   --> $DIR/negative-reasoning.rs:19:1
    |
@@ -9,6 +21,6 @@ LL | impl AnotherTrait for D<OpaqueType> {
    |
    = note: upstream crates may add a new impl of trait `std::fmt::Debug` for type `OpaqueType` in future versions
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0119`.
diff --git a/src/test/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle-2.rs b/src/test/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle-2.rs
@@ -5,13 +5,14 @@ type Foo = impl PartialEq<(Foo, i32)>;
 struct Bar;
 
 impl PartialEq<(Foo, i32)> for Bar {
+//~^ ERROR cannot implement trait on type alias impl trait
     fn eq(&self, _other: &(Foo, i32)) -> bool {
         true
     }
 }
 
 fn foo() -> Foo {
-    Bar //~ ERROR can't compare `Bar` with `(Bar, i32)`
+    Bar
 }
 
 fn main() {}
diff --git a/src/test/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle-2.stderr b/src/test/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle-2.stderr
@@ -1,12 +1,14 @@
-error[E0277]: can't compare `Bar` with `(Bar, i32)`
-  --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle-2.rs:14:5
+error: cannot implement trait on type alias impl trait
+  --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle-2.rs:7:17
    |
-LL |     Bar
-   |     ^^^ no implementation for `Bar == (Bar, i32)`
+LL | impl PartialEq<(Foo, i32)> for Bar {
+   |                 ^^^
    |
-   = help: the trait `PartialEq<(Bar, i32)>` is not implemented for `Bar`
-   = help: the trait `PartialEq<(Foo, i32)>` is implemented for `Bar`
+note: type alias impl trait defined here
+  --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle-2.rs:3:12
+   |
+LL | type Foo = impl PartialEq<(Foo, i32)>;
+   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
-For more information about this error, try `rustc --explain E0277`.
diff --git a/src/test/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.rs b/src/test/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.rs
@@ -2,7 +2,6 @@
 
 mod a {
     type Foo = impl PartialEq<(Foo, i32)>;
-    //~^ ERROR unconstrained opaque type
 
     struct Bar;
 
@@ -15,13 +14,12 @@ mod a {
 
 mod b {
     type Foo = impl PartialEq<(Foo, i32)>;
-    //~^ ERROR unconstrained opaque type
 
     struct Bar;
 
     impl PartialEq<(Foo, i32)> for Bar {
+        //~^ ERROR cannot implement trait on type alias impl trait
         fn eq(&self, _other: &(Bar, i32)) -> bool {
-            //~^ ERROR impl has stricter requirements than trait
             true
         }
     }

diff --git a/src/test/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.stderr b/src/test/ui/impl-trait/recursive-type-alias-impl-trait-declaration-too-subtle.stderr
@@ -1,25 +1,14 @@
-error: unconstrained opaque type
-  --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:4:16
+error: cannot implement trait on type alias impl trait
+  --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:20:21
    |
-LL |     type Foo = impl PartialEq<(Foo, i32)>;
-   |                ^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     impl PartialEq<(Foo, i32)> for Bar {
+   |                     ^^^
    |
-   = note: `Foo` must be used in combination with a concrete type within the same module
-
-error: unconstrained opaque type
-  --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:17:16
+note: type alias impl trait defined here
+  --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:16:16
    |
 LL |     type Foo = impl PartialEq<(Foo, i32)>;
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = note: `Foo` must be used in combination with a concrete type within the same module
-
-error[E0276]: impl has stricter requirements than trait
-  --> $DIR/recursive-type-alias-impl-trait-declaration-too-subtle.rs:23:9
-   |
-LL |         fn eq(&self, _other: &(Bar, i32)) -> bool {
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ impl has extra requirement `b::Bar: PartialEq<(b::Bar, i32)>`
 
-error: aborting due to 3 previous errors
+error: aborting due to previous error
 
-For more information about this error, try `rustc --explain E0276`.
diff --git a/src/test/ui/traits/alias/issue-83613.stderr b/src/test/ui/traits/alias/issue-83613.stderr
@@ -1,8 +1,8 @@
 error: cannot implement trait on type alias impl trait
-  --> $DIR/issue-83613.rs:10:1
+  --> $DIR/issue-83613.rs:10:23
    |
 LL | impl AnotherTrait for OpaqueType {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                       ^^^^^^^^^^
    |
 note: type alias impl trait defined here
   --> $DIR/issue-83613.rs:4:19

diff --git a/src/test/ui/type-alias-impl-trait/issue-65384.stderr b/src/test/ui/type-alias-impl-trait/issue-65384.stderr
@@ -1,8 +1,8 @@
 error: cannot implement trait on type alias impl trait
-  --> $DIR/issue-65384.rs:10:1
+  --> $DIR/issue-65384.rs:10:18
    |
 LL | impl MyTrait for Bar {}
-   | ^^^^^^^^^^^^^^^^^^^^
+   |                  ^^^
    |
 note: type alias impl trait defined here
   --> $DIR/issue-65384.rs:8:12

diff --git a/src/test/ui/type-alias-impl-trait/issue-76202-trait-impl-for-tait.stderr b/src/test/ui/type-alias-impl-trait/issue-76202-trait-impl-for-tait.stderr
@@ -1,8 +1,8 @@
 error: cannot implement trait on type alias impl trait
-  --> $DIR/issue-76202-trait-impl-for-tait.rs:16:1
+  --> $DIR/issue-76202-trait-impl-for-tait.rs:16:15
    |
 LL | impl Test for F {
-   | ^^^^^^^^^^^^^^^
+   |               ^
    |
 note: type alias impl trait defined here
   --> $DIR/issue-76202-trait-impl-for-tait.rs:9:10

diff --git a/src/test/ui/type-alias-impl-trait/issue-84660-trait-impl-for-tait.rs b/src/test/ui/type-alias-impl-trait/issue-84660-trait-impl-for-tait.rs
@@ -0,0 +1,23 @@
+// Regression test for issues #84660 and #86411: both are variations on #76202.
+// Tests that we don't ICE when we have an opaque type appearing anywhere in an impl header.
+
+#![feature(type_alias_impl_trait)]
+
+trait Foo {}
+impl Foo for () {}
+type Bar = impl Foo;
+fn _defining_use() -> Bar {}
+
+trait TraitArg<T> {
+    fn f();
+}
+
+impl TraitArg<Bar> for () { //~ ERROR cannot implement trait
+    fn f() {
+        println!("ho");
+    }
+}
+
+fn main() {
+    <() as TraitArg<Bar>>::f();
+}
diff --git a/src/test/ui/type-alias-impl-trait/issue-84660-trait-impl-for-tait.stderr b/src/test/ui/type-alias-impl-trait/issue-84660-trait-impl-for-tait.stderr
@@ -0,0 +1,14 @@
+error: cannot implement trait on type alias impl trait
+  --> $DIR/issue-84660-trait-impl-for-tait.rs:15:15
+   |
+LL | impl TraitArg<Bar> for () {
+   |               ^^^
+   |
+note: type alias impl trait defined here
+  --> $DIR/issue-84660-trait-impl-for-tait.rs:8:12
+   |
+LL | type Bar = impl Foo;
+   |            ^^^^^^^^
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/type-alias-impl-trait/issue-84660-unsoundness.rs b/src/test/ui/type-alias-impl-trait/issue-84660-unsoundness.rs
@@ -0,0 +1,41 @@
+// Another example from issue #84660, this time weaponized as a safe transmute: an opaque type in an
+// impl header being accepted was used to create unsoundness.
+
+#![feature(type_alias_impl_trait)]
+
+trait Foo {}
+impl Foo for () {}
+type Bar = impl Foo;
+fn _defining_use() -> Bar {}
+
+trait Trait<T, In> {
+    type Out;
+    fn convert(i: In) -> Self::Out;
+}
+
+impl<In, Out> Trait<Bar, In> for Out { //~ ERROR cannot implement trait
+    type Out = Out;
+    fn convert(_i: In) -> Self::Out {
+        unreachable!();
+    }
+}
+
+impl<In, Out> Trait<(), In> for Out {
+    type Out = In;
+    fn convert(i: In) -> Self::Out {
+        i
+    }
+}
+
+fn transmute<In, Out>(i: In) -> Out {
+    <Out as Trait<Bar, In>>::convert(i)
+}
+
+fn main() {
+    let d;
+    {
+        let x = "Hello World".to_string();
+        d = transmute::<&String, &String>(&x);
+    }
+    println!("{}", d);
+}
diff --git a/src/test/ui/type-alias-impl-trait/issue-84660-unsoundness.stderr b/src/test/ui/type-alias-impl-trait/issue-84660-unsoundness.stderr
@@ -0,0 +1,14 @@
+error: cannot implement trait on type alias impl trait
+  --> $DIR/issue-84660-unsoundness.rs:16:21
+   |
+LL | impl<In, Out> Trait<Bar, In> for Out {
+   |                     ^^^
+   |
+note: type alias impl trait defined here
+  --> $DIR/issue-84660-unsoundness.rs:8:12
+   |
+LL | type Bar = impl Foo;
+   |            ^^^^^^^^
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/type-alias-impl-trait/nested-tait-inference3.rs b/src/test/ui/type-alias-impl-trait/nested-tait-inference3.rs
@@ -4,11 +4,11 @@
 use std::fmt::Debug;
 
 type FooX = impl Debug;
-//~^ unconstrained opaque type
 
 trait Foo<A> { }
 
 impl Foo<FooX> for () { }
+//~^ cannot implement trait on type alias impl trait
 
 fn foo() -> impl Foo<FooX> {
     ()

diff --git a/src/test/ui/type-alias-impl-trait/nested-tait-inference3.stderr b/src/test/ui/type-alias-impl-trait/nested-tait-inference3.stderr
@@ -1,10 +1,14 @@
-error: unconstrained opaque type
+error: cannot implement trait on type alias impl trait
+  --> $DIR/nested-tait-inference3.rs:10:10
+   |
+LL | impl Foo<FooX> for () { }
+   |          ^^^^
+   |
+note: type alias impl trait defined here
   --> $DIR/nested-tait-inference3.rs:6:13
    |
 LL | type FooX = impl Debug;
    |             ^^^^^^^^^^
-   |
-   = note: `FooX` must be used in combination with a concrete type within the same module
 
 error: aborting due to previous error