Skip to content

Implement FromZeroes for thin raw pointers #294

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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
37 changes: 37 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,40 @@ safety_comment! {
unsafe_impl!(T: ?Sized + Unaligned => Unaligned for ManuallyDrop<T>);
assert_unaligned!(ManuallyDrop<()>, ManuallyDrop<u8>);
}
safety_comment! {
/// SAFETY:
/// The all-zeroes const and mut raw pointers are valid, and it is sound to
/// materialize them from nothing. The existence of `ptr::null` [1] and
Comment on lines +950 to +951
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it clearly documented anywhere that 0 is a valid value for the null pointer? Infamously, the C standard does not require that null pointers have an all-zero bit-pattern, only that they compare as equal to 0.

/// `ptr::null_mut` [2], which are safe functions, guarantees this (if this
/// were not sound, these functions could not exist). Unlike non-null
/// pointers, it is always unsound to dereference null pointers, and so it's
/// not a footgun that converting from zeroes may not preserve pointer
/// provenance information.
///
/// Since the encoding of fat pointers is not currently defined by the
/// reference, it would not be sound to implement `FromZeroes` for fat
/// pointer types such `*const T` for `T: ?Sized` or even for more
/// constrained pointer types such as `*const [T]` for `T: Sized`.
///
/// Currently, though it would likely be sound, we choose not to implement
/// `FromBytes` or `AsBytes` for raw pointers because it would be easy for
/// code to mistakenly assume that converting from a raw pointer to a
/// different representation (such as a byte array) and back again via
/// `FromBytes` and `AsBytes` would result in a semantically identical
/// pointer. Thanks to provenance information, that may not actually be
/// true, so this would present a serious footgun. Note that this aspect of
/// Rust's memory model is still up in the air, so it's possible that these
/// conversions will one day be determined to be sound, at which point we
/// could choose to support these impls. See #170 for more information.
///
/// [1] https://doc.rust-lang.org/core/ptr/fn.null.html
/// [2] https://doc.rust-lang.org/core/ptr/fn.null_mut.html
// TODO(https://github.com/rust-lang/reference/pull/1392#issuecomment-1696768191):
// Once the validity of materializing null pointers is guaranteed in the
// reference, cite that instead of `null` and `null_mut`.
unsafe_impl!(T: Sized => FromZeroes for *const T);
unsafe_impl!(T: Sized => FromZeroes for *mut T);
Comment on lines +979 to +980
Copy link
Collaborator

Choose a reason for hiding this comment

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

This snippet from the internals of the standard library gives me pause:

pub const fn invalid<T>(addr: usize) -> *const T {
    // FIXME(strict_provenance_magic): I am magic and should be a compiler intrinsic.
    // We use transmute rather than a cast so tools like Miri can tell that this
    // is *not* the same as from_exposed_addr.
    // SAFETY: every valid integer is also a valid pointer (as long as you don't dereference that
    // pointer).
    unsafe { mem::transmute(addr) }
}

The use of mem::transmute suggests that these sorts of transmutes are fine now, but what about the comment "I am magic and should be a compiler intrinsic"?

}
safety_comment! {
/// SAFETY:
/// Per the reference [1]:
Expand Down Expand Up @@ -3983,6 +4017,9 @@ mod tests {
assert_impls!(Unalign<u8>: FromZeroes, FromBytes, AsBytes, Unaligned);
assert_impls!(Unalign<NotZerocopy>: Unaligned, !FromZeroes, !FromBytes, !AsBytes);

assert_impls!(*const NotZerocopy: FromZeroes, !FromBytes, !AsBytes, !Unaligned);
assert_impls!(*mut NotZerocopy: FromZeroes, !FromBytes, !AsBytes, !Unaligned);

assert_impls!([u8]: FromZeroes, FromBytes, AsBytes, Unaligned);
assert_impls!([NotZerocopy]: !FromZeroes, !FromBytes, !AsBytes, !Unaligned);
assert_impls!([u8; 0]: FromZeroes, FromBytes, AsBytes, Unaligned);
Expand Down
4 changes: 2 additions & 2 deletions zerocopy-derive/tests/ui-nightly/derive_transparent.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ error[E0277]: the trait bound `NotZerocopy: FromZeroes` is not satisfied
|
= help: the following other types implement trait `FromZeroes`:
()
*const T
*mut T
AU16
F32<O>
F64<O>
I128<O>
I16<O>
I32<O>
I64<O>
and $N others
note: required for `TransparentStruct<NotZerocopy>` to implement `FromZeroes`
--> tests/ui-nightly/derive_transparent.rs:23:19
Expand Down
8 changes: 4 additions & 4 deletions zerocopy-derive/tests/ui-nightly/late_compile_pass.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ error[E0277]: the trait bound `NotZerocopy: FromZeroes` is not satisfied
|
= help: the following other types implement trait `FromZeroes`:
()
*const T
*mut T
AU16
F32<O>
F64<O>
FromZeroes1
I128<O>
I16<O>
I32<O>
and $N others
= help: see issue #48214
= help: add `#![feature(trivial_bounds)]` to the crate attributes to enable
Expand Down Expand Up @@ -46,13 +46,13 @@ error[E0277]: the trait bound `FromBytes1: FromZeroes` is not satisfied
|
= help: the following other types implement trait `FromZeroes`:
()
*const T
*mut T
AU16
F32<O>
F64<O>
FromZeroes1
I128<O>
I16<O>
I32<O>
and $N others
note: required by a bound in `FromBytes`
--> $WORKSPACE/src/lib.rs
Expand Down
4 changes: 2 additions & 2 deletions zerocopy-derive/tests/ui-stable/derive_transparent.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ error[E0277]: the trait bound `NotZerocopy: FromZeroes` is not satisfied
|
= help: the following other types implement trait `FromZeroes`:
()
*const T
*mut T
AU16
F32<O>
F64<O>
I128<O>
I16<O>
I32<O>
I64<O>
and $N others
note: required for `TransparentStruct<NotZerocopy>` to implement `FromZeroes`
--> tests/ui-stable/derive_transparent.rs:23:19
Expand Down
8 changes: 4 additions & 4 deletions zerocopy-derive/tests/ui-stable/late_compile_pass.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ error[E0277]: the trait bound `NotZerocopy: FromZeroes` is not satisfied
|
= help: the following other types implement trait `FromZeroes`:
()
*const T
*mut T
AU16
F32<O>
F64<O>
FromZeroes1
I128<O>
I16<O>
I32<O>
and $N others
= help: see issue #48214
= note: this error originates in the derive macro `FromZeroes` (in Nightly builds, run with -Z macro-backtrace for more info)
Expand Down Expand Up @@ -44,13 +44,13 @@ error[E0277]: the trait bound `FromBytes1: FromZeroes` is not satisfied
|
= help: the following other types implement trait `FromZeroes`:
()
*const T
*mut T
AU16
F32<O>
F64<O>
FromZeroes1
I128<O>
I16<O>
I32<O>
and $N others
note: required by a bound in `FromBytes`
--> $WORKSPACE/src/lib.rs
Expand Down