Skip to content

Avoid regression in consteval #2286

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 29, 2022
Merged

Avoid regression in consteval #2286

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion utils/zerovec/src/zerovec/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,9 @@ where
/// `bytes` need to be an output from [`ZeroSlice::as_bytes()`].
pub const unsafe fn from_bytes_unchecked(bytes: &'a [u8]) -> Self {
// &[u8] and &[T::ULE] are the same slice with different length metadata.
let (data, mut metadata): (usize, usize) = core::mem::transmute(bytes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW the old code most likely has UB. Quoting from the transmute docs:

Transmuting pointers to integers in a const context is undefined behavior. Any attempt to use the resulting value for integer operations will abort const-evaluation.

So, if from_bytes_unchecked is ever called with a non-zero-sized slice (or a zero-sized slice that actually uses a pointer to some allocated memory), then this is definitely UB. This PR is a UB-fix, not a rustc-regression-workaround.

Copy link
Member Author

@Manishearth Manishearth Jul 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm aware it's UB.

As I've stated in rust-lang/rust#99923, I still consider things like this a regression as long as the docs on undefined behavior are incomplete, scattered, and in flux, and avoiding UB is not practically possible for unsafe Rust programmers.

// n.b. be careful here, this might hit https://github.com/rust-lang/rust/issues/99923
let data = bytes.as_ptr();
let mut metadata = bytes.len();
metadata /= core::mem::size_of::<T::ULE>();
Self::Borrowed(core::mem::transmute((data, metadata)))
Comment on lines -260 to 264
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: the proper way to do this will be stable in 1.64:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!

}
Expand Down