-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Make ptr_cast_add_auto_to_object
lint into hard error
#136764
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
An auto trait cannot be added to the bounds of a `dyn Trait` type via | ||
a pointer cast. | ||
|
||
Erroneous code example: | ||
|
||
```rust,edition2021,compile_fail,E0804 | ||
let ptr: *const dyn core::any::Any = &(); | ||
_ = ptr as *const (dyn core::any::Any + Send); | ||
``` | ||
|
||
Adding an auto trait can make the vtable invalid, potentially causing | ||
UB in safe code afterwards. For example: | ||
|
||
```rust,edition2021,no_run | ||
use core::{mem::transmute, ptr::NonNull}; | ||
|
||
trait Trait { | ||
fn f(&self) | ||
where | ||
Self: Send; | ||
} | ||
|
||
impl Trait for NonNull<()> { | ||
fn f(&self) { | ||
unreachable!() | ||
} | ||
} | ||
|
||
fn main() { | ||
let unsend: &dyn Trait = &NonNull::dangling(); | ||
let bad: &(dyn Trait + Send) = unsafe { transmute(unsend) }; | ||
// This crashes, since the vtable for `NonNull as dyn Trait` does | ||
// not have an entry for `Trait::f`. | ||
bad.f(); | ||
} | ||
``` | ||
|
||
To fix this error, you can use `transmute` rather than pointer casts, | ||
but you must ensure that the vtable is valid for the pointer's type | ||
before calling a method on the trait object or allowing other code to | ||
do so. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -547,6 +547,7 @@ E0800: 0800, | |
E0801: 0801, | ||
E0802: 0802, | ||
E0803: 0803, | ||
E0804: 0804, | ||
); | ||
) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,20 @@ | ||
//@ check-pass | ||
|
||
trait Trait<'a> {} | ||
|
||
fn add_auto<'a>(x: *mut dyn Trait<'a>) -> *mut (dyn Trait<'a> + Send) { | ||
x as _ | ||
//~^ warning: adding an auto trait `Send` to a trait object in a pointer cast may cause UB later on | ||
//~| warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
//~^ ERROR cannot add auto trait `Send` to dyn bound via pointer cast | ||
//~| NOTE unsupported cast | ||
//~| NOTE this could allow UB elsewhere | ||
//~| HELP use `transmute` if you're sure this is sound | ||
} | ||
|
||
// (to test diagnostic list formatting) | ||
fn add_multiple_auto<'a>(x: *mut dyn Trait<'a>) -> *mut (dyn Trait<'a> + Send + Sync + Unpin) { | ||
x as _ | ||
//~^ warning: adding auto traits `Send`, `Sync`, and `Unpin` to a trait object in a pointer cast may cause UB later on | ||
//~| warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
//~^ ERROR cannot add auto traits `Send`, `Sync`, and `Unpin` to dyn bound via pointer cast | ||
//~| NOTE unsupported cast | ||
//~| NOTE this could allow UB elsewhere | ||
//~| HELP use `transmute` if you're sure this is sound | ||
} | ||
|
||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,43 +1,21 @@ | ||
warning: adding an auto trait `Send` to a trait object in a pointer cast may cause UB later on | ||
--> $DIR/ptr-to-trait-obj-add-auto.rs:6:5 | ||
error[E0804]: cannot add auto trait `Send` to dyn bound via pointer cast | ||
--> $DIR/ptr-to-trait-obj-add-auto.rs:4:5 | ||
| | ||
LL | x as _ | ||
| ^^^^^^ | ||
| ^^^^^^ unsupported cast | ||
| | ||
= 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 | ||
= note: this could allow UB elsewhere | ||
= help: use `transmute` if you're sure this is sound | ||
|
||
warning: adding auto traits `Send`, `Sync`, and `Unpin` to a trait object in a pointer cast may cause UB later on | ||
error[E0804]: cannot add auto traits `Send`, `Sync`, and `Unpin` to dyn bound via pointer cast | ||
--> $DIR/ptr-to-trait-obj-add-auto.rs:13:5 | ||
| | ||
LL | x as _ | ||
| ^^^^^^ | ||
| ^^^^^^ unsupported cast | ||
| | ||
= 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: this could allow UB elsewhere | ||
= help: use `transmute` if you're sure this is sound | ||
|
||
warning: 2 warnings emitted | ||
|
||
Future incompatibility report: Future breakage diagnostic: | ||
warning: adding an auto trait `Send` to a trait object in a pointer cast may cause UB later on | ||
--> $DIR/ptr-to-trait-obj-add-auto.rs:6:5 | ||
| | ||
LL | x as _ | ||
| ^^^^^^ | ||
| | ||
= 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 | ||
|
||
Future breakage diagnostic: | ||
warning: adding auto traits `Send`, `Sync`, and `Unpin` to a trait object in a pointer cast may cause UB later on | ||
--> $DIR/ptr-to-trait-obj-add-auto.rs:13:5 | ||
| | ||
LL | x as _ | ||
| ^^^^^^ | ||
| | ||
= 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 | ||
error: aborting due to 2 previous errors | ||
|
||
For more information about this error, try `rustc --explain E0804`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
# Removed lints | ||
|
||
This directory contains tests to confirm that lints that have been | ||
removed do not cause errors and produce the appropriate warnings. | ||
Comment on lines
+1
to
+4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would that ever prevent a problem? As-in, if you forgot to remove the lint properly surely you'd also forget to add a test? And it seems very unlikely that any compiler change would break removed lints... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I had the same thoughts, but... you know, these removed lints are actually part of our language. If there were a "Rust-compatibility test suite", these should be in there. So, anyway, that pushed me slightly over the line to adding this. Besides, I could see this catching a case where the author did remember, but then the Also, when reviewing quickly, like we do on the lang side during calls, we tend to focus on the tests, so it's helpful to see everything we believe should be true actually double-checked there. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we are to proceed with it, could you open an issue to back-fill removed-lints tests for all previously removed lints? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
//@ check-pass | ||
|
||
#![deny(ptr_cast_add_auto_to_object)] | ||
//~^ WARN lint `ptr_cast_add_auto_to_object` has been removed | ||
fn main() {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
warning: lint `ptr_cast_add_auto_to_object` has been removed: converted into hard error, see issue #127323 <https://github.com/rust-lang/rust/issues/127323> for more information | ||
--> $DIR/ptr_cast_add_auto_to_object.rs:3:9 | ||
| | ||
LL | #![deny(ptr_cast_add_auto_to_object)] | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= note: `#[warn(renamed_and_removed_lints)]` on by default | ||
|
||
warning: 1 warning emitted | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(curiosity) Any specific reason you are using
ERROR
instead oferror:
?I prefer the latter cause it's closer to the actual compiler output (
error[CODE]: message
orerror: message
for code-less errors) and because in my opinion it looks/reads nicer. (but there is no accepted style, so feel free to use whatever of course)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No particular reason.
ERROR
just seemed more common among tests when I picked up the habit. I am sympathetic to the reasons you give for the other.