From 49270e7ca558895b87664f3d331174b04fd93fbe Mon Sep 17 00:00:00 2001 From: Samuel Selleck Date: Fri, 29 Sep 2023 14:31:14 -0700 Subject: [PATCH] Add safety TODO(#61)s where missing. Add missing quotes to safety comments --- src/lib.rs | 49 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7a02c3a5239..cbd5a11a92f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -600,6 +600,7 @@ impl_known_layout!(const N: usize, T => [T; N]); safety_comment! { /// SAFETY: + // TODO(#61): Add reference to docs + quotes /// `str` and `ManuallyDrop<[T]>` have the same representations as `[u8]` /// and `[T]` repsectively. `str` has different bit validity than `[u8]`, /// but that doesn't affect the soundness of this impl. @@ -704,6 +705,7 @@ pub unsafe trait FromZeroes { let slf: *mut Self = self; let len = mem::size_of_val(self); // SAFETY: + // TODO(#61): Add reference to docs + quotes // - `self` is guaranteed by the type system to be valid for writes of // size `size_of_val(self)`. // - `u8`'s alignment is 1, and thus `self` is guaranteed to be aligned @@ -1091,6 +1093,7 @@ pub unsafe trait AsBytes { let slf: *const Self = self; // SAFETY: + // TODO(#61): Add reference to docs + quotes // - `slf.cast::()` is valid for reads for `len * // mem::size_of::()` many bytes because... // - `slf` is the same pointer as `self`, and `self` is a reference @@ -1231,6 +1234,7 @@ safety_comment! { safety_comment! { /// SAFETY: + /// TODO(#61): Add quotes /// - `FromZeroes`, `FromBytes`: all bit patterns are valid for integers [1] /// - `AsBytes`: integers have no padding bytes [1] /// - `Unaligned` (`u8` and `i8` only): The reference [2] specifies the size @@ -1266,7 +1270,8 @@ safety_comment! { /// - `AsBytes`: the `{f32,f64}::to_bits` methods' documentation [4,5] /// states that they are currently equivalent to `transmute`. [3] /// - /// TODO: Make these arguments more precisely in terms of the documentation. + /// TODO(#61): Make these arguments more precisely in terms of the documentation + /// TODO(#61): Add quotes /// /// [1] https://doc.rust-lang.org/nightly/std/primitive.f32.html#method.from_bits /// [2] https://doc.rust-lang.org/nightly/std/primitive.f64.html#method.from_bits @@ -1280,11 +1285,11 @@ safety_comment! { safety_comment! { /// SAFETY: - /// - `FromZeroes`: Per the reference [1], 0x00 is a valid bit pattern for - /// `bool`. - /// - `AsBytes`: Per the reference [1], `bool` always has a size of 1 with - /// valid bit patterns 0x01 and 0x00, so the only byte of the bool is - /// always initialized + /// - `FromZeroes`: Valid since "The value false has the bit pattern 0x00" [1]. + /// - `AsBytes`: Since "the boolean type has a size and + /// alignment of 1 each" and "The value false has the bit pattern 0x00 and + /// the value true has the bit pattern 0x01" [1]. Note that it is undefined + /// behavior for an object with the boolean type to have any other bit pattern. /// - `Unaligned`: Per the reference [1], "[a]n object with the boolean type /// has a size and alignment of 1 each." /// @@ -1294,10 +1299,13 @@ safety_comment! { } safety_comment! { /// SAFETY: - /// - `FromZeroes`: Per the reference [1], 0x0000 is a valid bit pattern for - /// `char`. - /// - `AsBytes`: `char` is represented as a 32-bit unsigned word (`u32`) - /// [1], which is `AsBytes`. Note that unlike `u32`, not all bit patterns + /// - `FromZeroes`: Per reference [1], "A value of type char is + /// a Unicode scalar value (i.e. a code point that is not a surrogate), + /// represented as a 32-bit unsigned word in the 0x0000 to 0xD7FF or 0xE000 + /// to 0x10FFFF range" which contains 0x0000. + /// - `AsBytes`: `char` is per reference [1] "represented as a 32-bit unsigned + /// word" (`u32`) + /// which is `AsBytes`. Note that unlike `u32`, not all bit patterns /// are valid for `char`. /// /// [1] https://doc.rust-lang.org/reference/types/textual.html @@ -1305,6 +1313,7 @@ safety_comment! { } safety_comment! { /// SAFETY: + /// TODO(#61): Add quotes /// - `FromZeroes`, `AsBytes`, `Unaligned`: Per the reference [1], `str` has /// the same layout as `[u8]`, and `[u8]` is `FromZeroes`, `AsBytes`, and /// `Unaligned`. @@ -1320,6 +1329,7 @@ safety_comment! { // `NonZeroXxx` is `AsBytes`, but not `FromZeroes` or `FromBytes`. // /// SAFETY: + /// TODO(#61): Add quotes /// - `AsBytes`: `NonZeroXxx` has the same layout as its associated /// primitive. Since it is the same size, this guarantees it has no /// padding - integers have no padding, and there's no room for padding @@ -1354,6 +1364,7 @@ safety_comment! { } safety_comment! { /// SAFETY: + /// TODO(#61): Add quotes /// - `FromZeroes`, `FromBytes`, `AsBytes`: The Rust compiler reuses `0` /// value to represent `None`, so `size_of::>() == /// size_of::()`; see `NonZeroXxx` documentation. @@ -1386,7 +1397,11 @@ safety_comment! { safety_comment! { /// SAFETY: - /// For all `T`, `PhantomData` has size 0 and alignment 1. [1] + /// Per reference [1]: + /// "For all T, the following are guaranteed: + /// size_of::>() == 0 + /// align_of::>() == 1". + /// This gives: /// - `FromZeroes`, `FromBytes`: There is only one possible sequence of 0 /// bytes, and `PhantomData` is inhabited. /// - `AsBytes`: Since `PhantomData` has size 0, it contains no padding @@ -1403,12 +1418,14 @@ safety_comment! { } safety_comment! { /// SAFETY: + /// TODO(#61): Add quotes /// `Wrapping` is guaranteed by its docs [1] to have the same layout as /// `T`. Also, `Wrapping` is `#[repr(transparent)]`, and has a single /// field, which is `pub`. Per the reference [2], this means that the /// `#[repr(transparent)]` attribute is "considered part of the public ABI". /// - /// [1] https://doc.rust-lang.org/nightly/core/num/struct.Wrapping.html#layout-1 + /// references nightly?: + /// [1] TODO(https://doc.rust-lang.org/nightly/core/num/struct.Wrapping.html#layout-1): /// [2] https://doc.rust-lang.org/nomicon/other-reprs.html#reprtransparent unsafe_impl!(T: FromZeroes => FromZeroes for Wrapping); unsafe_impl!(T: FromBytes => FromBytes for Wrapping); @@ -1427,11 +1444,10 @@ safety_comment! { /// Thus, we require `T: FromZeroes` and `T: FromBytes` in order to ensure /// that `T` - and thus `MaybeUninit` - contains to `UnsafeCell`s. /// Thus, requiring that `T` implement each of these traits is sufficient - /// - `Unaligned`: `MaybeUninit` is guaranteed by its documentation [1] - /// to have the same alignment as `T`. + /// - `Unaligned`: "MaybeUninit is guaranteed to have the same size, alignment, + /// and ABI as T" [1] /// - /// [1] - /// https://doc.rust-lang.org/nightly/core/mem/union.MaybeUninit.html#layout-1 + /// [1] https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#layout-1 /// /// TODO(https://github.com/google/zerocopy/issues/251): If we split /// `FromBytes` and `RefFromBytes`, or if we introduce a separate @@ -1444,6 +1460,7 @@ safety_comment! { } safety_comment! { /// SAFETY: + // TODO(#61): Add reference to docs + quotes /// `ManuallyDrop` has the same layout as `T`, and accessing the inner value /// is safe (meaning that it's unsound to leave the inner value /// uninitialized while exposing the `ManuallyDrop` to safe code).