-
Notifications
You must be signed in to change notification settings - Fork 13k
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
const-eval: error when encountering references to functions / vtables #121199
Conversation
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri |
Failed to set assignee to
|
Eh, sorry. |
const-eval: error when encountering references to functions / vtables Such references sound like an exceedingly bad idea... just use raw pointers if you must do things like this. But we should crater this, to be on the safe side.
☀️ Try build successful - checks-actions |
@craterbot check |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🎉 Experiment
|
Okay two of these are legit regressions -- some people are actually creating references to vtables. oO
I will have to check more closely if what they are doing makes any sense. |
rattish seems to be basically this code: #![feature(ptr_metadata)]
use std::ptr;
pub type Metadata<U> = <U as ptr::Pointee>::Metadata;
type T = i32;
type U = dyn PartialEq<T>;
const ADDRESS: *mut () = 0xdeadbeef_usize as _;
const METADATA: Metadata<U> = ptr::metadata::<U>(ptr::null::<T>()); I guess that is this reference: pub struct DynMetadata<Dyn: ?Sized> {
vtable_ptr: &'static VTable,
phantom: crate::marker::PhantomData<Dyn>,
} So... are we sure we want this to be a reference? Can't it be The other one also involves |
I'm gonna assume no one actually found it important to have it be a reference. It was just natural to assume that since we know the pointer is always valid, we can also just use a reference to statically signal that. Then again... Why not permit references to vtables as long as the pointee type is an extern type? That seems like the one use case for it that is not going to be biting us |
I was just thinking since there's no actual memory there (vtables are magic objects that exist outside of addressable memory, there's just a zero-sized "handle" representing them being put at some address so that we can point to them), we shouldn't allow such references. But maybe it's not worth it... |
Clearly there is no UB here, since we allow such references to point even do bare integer addresses and soon to go out-of-bounds or even after-free.
This was meant more as a lint, an early warning system. If someone creates a reference to a vtable then maybe they think they can read/write it and that would be UB. But that would probably need a more targeted message. Not "this code has UB" but "you are doing something which is likely leading to UB elsewhere and here is why".
That would be a lot more work than this PR so yeah let's close this.
|
add const test for ptr::metadata rust-lang#121199 uncovered this as a gap in our test suite. r? `@oli-obk`
Rollup merge of rust-lang#121806 - RalfJung:const-metadata, r=oli-obk add const test for ptr::metadata rust-lang#121199 uncovered this as a gap in our test suite. r? `@oli-obk`
Such references sound like an exceedingly bad idea... just use raw pointers if you must do things like this.
But we should crater this, to be on the safe side.