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

Tracking issue for future-incompatibility lint ptr_cast_add_auto_to_object #127323

Open
WaffleLapkin opened this issue Jul 4, 2024 · 8 comments
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-trait-objects Area: trait objects, vtable layout C-future-incompatibility Category: Future-incompatibility lints C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-arbitrary_self_types `#![feature(arbitrary_self_types)]` F-derive_coerce_pointee Feature: RFC 3621's oft-renamed implementation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@WaffleLapkin
Copy link
Member

This is a tracking issue for the ptr_cast_add_auto_to_object lint, which was added in #120248.

This lint detects casts of raw pointers to trait objects, which add auto traits. Adding auto traits to trait objects may cause UB when #![feature(arbitrary_self_types)] is used.

Example

#![feature(arbitrary_self_types)]
trait Trait {
    fn f(self: *const Self)
    where
        Self: Send;
}

impl Trait for *const () {
    fn f(self: *const Self) {
        unreachable!()
    }
}

fn main() {
    let unsend: *const () = &();
    let unsend: *const dyn Trait = &unsend;
    let send_bad: *const (dyn Trait + Send) = unsend as _;
    send_bad.f(); // this crashes, since vtable for `*const ()` does not have an entry for `f`
    //~^ warning: adding an auto trait `Send` to a trait object in a pointer cast may cause UB later on
}

In case your usage is sound (e.g. because the trait doesn't have auto trait bounds), you can replace cast with a transmute to suppress the warning:

trait Cat {}
impl Cat for *const () {}

fn main() {
    let unsend: *const () = &();
    let unsend: *const dyn Cat = &unsend;
    let _send: *const (dyn Cat + Send) = unsafe {
        // Safety:
        // - Both types are pointers, to the same trait object (and thus have the same vtable)
        // - `Cat` does not have methods with `Send` bounds
        std::mem::transmute::<*const dyn Cat, *const (dyn Cat + Send)>(unsend)
    };
    // meow
}
@WaffleLapkin WaffleLapkin added C-future-incompatibility Category: Future-incompatibility lints C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-trait-objects Area: trait objects, vtable layout labels Jul 4, 2024
@bjorn3
Copy link
Member

bjorn3 commented Jul 4, 2024

In case your usage is sound (e.g. because the trait doesn't have auto trait bounds), you can replace cast with a transmute to suppress the warning:

Future compatibility warnings are generally made hard errors after a while, right? I don't think we should suggest suppressing a future compatibility warning.

@WaffleLapkin
Copy link
Member Author

@bjorn3 we only will make cases with the warning into errors. transmuteing is, and will be, allowed (although you do actually need to make sure that your code is sound).

swlynch99 added a commit to swlynch99/anymap3 that referenced this issue Jul 28, 2024
There's a new future incompatibility warning that recently started
firing on the code in CloneToAny (see [1]). It's triggering on code that
does a pointer cast from *mut dyn Trait to *mut dyn Trait + Send (or
other auto trait) which is an exact match for what this crate is doing.

The lint doesn't actually apply here though so it is safe to suppress
it. The recommended way to suppress it is to replace the pointer cast
with a transmute and that's what I have done here.

See the new safety comment for a more detailed explanation of why it is
safe.

[1]: rust-lang/rust#127323
@fmease fmease changed the title Tracking Issue for ptr_cast_add_auto_to_object future compatibility warning Tracking Issue for ptr_cast_add_auto_to_object future-incompatibility warning Sep 14, 2024
@fmease fmease added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 14, 2024
@fmease fmease changed the title Tracking Issue for ptr_cast_add_auto_to_object future-incompatibility warning Tracking issue for future-incompatibility lint ptr_cast_add_auto_to_object Sep 14, 2024
@fmease fmease added the A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. label Sep 14, 2024
reivilibre pushed a commit to reivilibre/anymap3 that referenced this issue Nov 9, 2024
There's a new future incompatibility warning that recently started
firing on the code in CloneToAny (see [1]). It's triggering on code that
does a pointer cast from *mut dyn Trait to *mut dyn Trait + Send (or
other auto trait) which is an exact match for what this crate is doing.

The lint doesn't actually apply here though so it is safe to suppress
it. The recommended way to suppress it is to replace the pointer cast
with a transmute and that's what I have done here.

See the new safety comment for a more detailed explanation of why it is
safe.

[1]: rust-lang/rust#127323
@BoxyUwU
Copy link
Member

BoxyUwU commented Feb 7, 2025

The example in the description now needs arbitrary_self_types_pointers instead however we can recreate the same kind of setup with a combination of arbitrary_self_types and derive_coerce_pointee in order to get a raw pointer receiver with a DispatchFromDyn impl:

#![feature(arbitrary_self_types, derive_coerce_pointee)]

use std::marker::CoercePointee;

#[derive(CoercePointee)]
#[repr(transparent)]
struct MyPtr<T: ?Sized>(*const T);

use std::ops::Receiver;

impl<T: ?Sized> Receiver for MyPtr<T> {
    type Target = T;
}

trait Trait {
    fn foo(self: MyPtr<Self>)
    where
        Self: Send;
}

impl Trait for *mut () {
    fn foo(self: MyPtr<Self>) {
        unreachable!()
    }
}

fn main() {
    let a = 0x1 as *const *mut ();
    let a = a as *const dyn Trait as *const (dyn Trait + Send);
    MyPtr(a).foo();
}

This does currently ICE due to #135179 not having been merged yet but once it's in this segfaults for me locally

@RalfJung
Copy link
Member

RalfJung commented Feb 7, 2025

We already have a safety invariant of requiring a valid vtable for raw pointers; dyn upcasting relies on that. I guess the cast above is accepted because "marker traits don't change the vtable", or so?

@RalfJung
Copy link
Member

RalfJung commented Feb 7, 2025

@BoxyUwU that does not trigger the lint that this issue tracks... so it should probably get a separate issue?

@BoxyUwU
Copy link
Member

BoxyUwU commented Feb 7, 2025

The ICE gets hit before the lint is triggered but once the ICE is fixed the lint triggers + there's a segfault when the program is run

@BoxyUwU
Copy link
Member

BoxyUwU commented Feb 7, 2025

boxy@crossaints:/media/Nyoomies/Repos/playground_1$ cargo +stage1 run
warning: adding an auto trait `Send` to a trait object in a pointer cast may cause UB later on
  --> src/main.rs:29:13
   |
29 |     let a = a as *const dyn Trait as *const (dyn Trait + Send);
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
   = note: for more information, see issue #127323 <https://github.com/rust-lang/rust/issues/127323>
   = note: `#[warn(ptr_cast_add_auto_to_object)]` on by default

warning: `c` (bin "c") generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.00s
warning: the following packages contain code that will be rejected by a future version of Rust: c v0.1.0 (/media/Nyoomies/Repos/playground_1)
note: to see what the problems were, use the option `--future-incompat-report`, or run `cargo report future-incompatibilities --id 2`
     Running `target/debug/c`
Segmentation fault (core dumped)

If anyone is curious this is the output I get locally on that example when using a stage1 compiler from errs' branch

@RalfJung
Copy link
Member

RalfJung commented Feb 7, 2025

Ah okay. So this one "just" needs the lint to be turned into a hard error then.

I agree with your statement elsewhere that that should happen before this example works on stable.

bors added a commit to rust-lang-ci/rust that referenced this issue Feb 9, 2025
…o_to_object-hard-error, r=<try>

Make `ptr_cast_add_auto_to_object` lint into hard error

In Rust 1.81, we added a FCW lint (including linting in dependencies) against pointer casts that add an auto trait to dyn bounds.  This was part of work making casts of pointers involving trait objects stricter, and was part of the work needed to restabilize trait upcasting.

We considered just making this a hard error, but opted against it at that time due to breakage found by crater.  This breakage was mostly due to the `anymap` crate which has been a persistent problem for us.

It's now a year later, and the fact that this is not yet a hard error is giving us pause about stabilizing arbitrary self types and `derive(CoercePointee)`.  So let's see about making a hard error of this.

r? ghost

cc `@adetaylor` `@Darksonn` `@BoxyUwU` `@RalfJung` `@compiler-errors` `@oli-obk` `@WaffleLapkin`

Related:

- rust-lang#135881
- rust-lang#136702

Tracking:

- rust-lang#127323
- rust-lang#44874
- rust-lang#123430
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-trait-objects Area: trait objects, vtable layout C-future-incompatibility Category: Future-incompatibility lints C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-arbitrary_self_types `#![feature(arbitrary_self_types)]` F-derive_coerce_pointee Feature: RFC 3621's oft-renamed implementation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants