Skip to content
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

WIP fix unsoundness via adjusting overlap check for some Pin impls #116706

Closed
wants to merge 1 commit into from
Closed
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
5 changes: 5 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
rustc_coinductive, AttributeType::Normal, template!(Word), WarnFollowing, @only_local: true,
"#![rustc_coinductive] changes a trait to be coinductive, allowing cycles in the trait solver."
),
rustc_attr!(
rustc_no_implicit_negative_coherence, AttributeType::Normal, template!(Word), WarnFollowing, @only_local: false,
"#![rustc_no_implicit_negative_coherence] makes the implementation overlap \
with other implementations even if where clauses are known to never hold"
),
rustc_attr!(
rustc_allow_incoherent_impl, AttributeType::Normal, template!(Word), ErrorFollowing, @only_local: true,
"#[rustc_allow_incoherent_impl] has to be added to all impl items of an incoherent inherent impl."
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 @@ -1383,6 +1383,7 @@ symbols! {
rustc_mir,
rustc_must_implement_one_of,
rustc_never_returns_null_ptr,
rustc_no_implicit_negative_coherence,
rustc_nonnull_optimization_guaranteed,
rustc_nounwind,
rustc_object_lifetime_default,
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,10 @@ fn overlap<'tcx>(
),
);

if overlap_mode.use_implicit_negative() {
if overlap_mode.use_implicit_negative()
&& !tcx.has_attr(impl1_def_id, sym::rustc_no_implicit_negative_coherence)
&& !tcx.has_attr(impl2_def_id, sym::rustc_no_implicit_negative_coherence)
{
for mode in [TreatInductiveCycleAs::Ambig, TreatInductiveCycleAs::Recur] {
if let Some(failing_obligation) = selcx.with_treat_inductive_cycle_as(mode, |selcx| {
impl_intersection_has_impossible_obligation(selcx, &obligations)
Expand Down
12 changes: 11 additions & 1 deletion library/core/src/pin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ use crate::ops::{CoerceUnsized, Deref, DerefMut, DispatchFromDyn, Receiver};
#[lang = "pin"]
#[fundamental]
#[repr(transparent)]
#[derive(Copy, Clone)]
#[derive(Copy)]
pub struct Pin<P> {
// FIXME(#93176): this field is made `#[unstable] #[doc(hidden)] pub` to:
// - deter downstream users from accessing it (which would be unsound!),
Expand All @@ -418,6 +418,15 @@ pub struct Pin<P> {
pub pointer: P,
}

// N.B. manual implementation exists to add `rustc_no_implicit_negative_coherence`
#[stable(feature = "pin", since = "1.33.0")]
#[cfg_attr(not(bootstrap), rustc_no_implicit_negative_coherence)]
impl<P: Clone> Clone for Pin<P> {
fn clone(&self) -> Self {
Pin { pointer: self.pointer.clone() }
}
}

// The following implementations aren't derived in order to avoid soundness
// issues. `&self.pointer` should not be accessible to untrusted trait
// implementations.
Expand Down Expand Up @@ -967,6 +976,7 @@ impl<P: Deref> Deref for Pin<P> {
}

#[stable(feature = "pin", since = "1.33.0")]
#[cfg_attr(not(bootstrap), rustc_no_implicit_negative_coherence)]
impl<P: DerefMut<Target: Unpin>> DerefMut for Pin<P> {
fn deref_mut(&mut self) -> &mut P::Target {
Pin::get_mut(Pin::as_mut(self))
Expand Down
4 changes: 1 addition & 3 deletions tests/ui/typeck/pin-unsound-issue-85099-derefmut.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// check-pass
// known-bug: #85099

// Should fail. Can coerce `Pin<T>` into `Pin<U>` where
// `T: Deref<Target: Unpin>` and `U: Deref<Target: !Unpin>`, using the
// `CoerceUnsized` impl on `Pin` and an unorthodox `DerefMut` impl for
Expand Down Expand Up @@ -43,6 +40,7 @@ impl<'a, Fut: Future<Output = ()>> SomeTrait<'a, Fut> for Fut {
}

impl<'b, 'a, Fut> DerefMut for Pin<&'b dyn SomeTrait<'a, Fut>> {
//~^ ERROR conflicting implementations of trait `DerefMut` for type `Pin<&dyn SomeTrait<'_, _>>`
fn deref_mut<'c>(
self: &'c mut Pin<&'b dyn SomeTrait<'a, Fut>>,
) -> &'c mut (dyn SomeTrait<'a, Fut> + 'b) {
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/typeck/pin-unsound-issue-85099-derefmut.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error[E0119]: conflicting implementations of trait `DerefMut` for type `Pin<&dyn SomeTrait<'_, _>>`
--> $DIR/pin-unsound-issue-85099-derefmut.rs:42:1
|
LL | impl<'b, 'a, Fut> DerefMut for Pin<&'b dyn SomeTrait<'a, Fut>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: conflicting implementation in crate `core`:
- impl<P> DerefMut for Pin<P>
where P: DerefMut, <P as Deref>::Target: Unpin;

error: aborting due to previous error

For more information about this error, try `rustc --explain E0119`.
Loading