Skip to content
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

Add Align type #249

Open
joshlf opened this issue Aug 10, 2023 · 1 comment
Open

Add Align type #249

joshlf opened this issue Aug 10, 2023 · 1 comment
Labels
compatibility-nonbreaking Changes that are (likely to be) non-breaking

Comments

@joshlf
Copy link
Member

joshlf commented Aug 10, 2023

Any design for this issue will interact with other issues tracked by #885; make sure to read that issue when tackling this one.

Needed for: #497

Add a type like the following, which represents a T aligned to the alignment of A:

/// A `T` which is at least as aligned as `A`.
///
/// `Align<T, A>`'s alignment is the maximum of the alignments of `T` and `A`,
/// and its size is `T`'s size rounded up to the next multiple of `A`'s
/// alignment (note that 0 is a valid multiple; if `T` is zero-sized, then
/// `Align<T, A>` will be zero-sized as well).
///
/// The first `mem::size_of::<T>()` bytes of `Align<T, A>` have the same layout
/// as `T`, and any remaining bytes are initialized to unspecified values.
#[derive(Unaligned)]
#[repr(C)]
pub struct Align<T: ?Sized, A> {
    // The layout of `Align` is guaranteed to have `t` at byte offset 0 possibly
    // followed by some padding bytes.
    //
    // `Align<T, A>` is guaranteed to have the maximum alignment of `T` and `A`.
    // This means that `_a`'s alignment requirement (equal to `A`'s) is
    // satisfied by `_a` living at byte offset 0. Since `_a` is 0 bytes in
    // length, `t` is properly aligned even if there is no padding between `_a`
    // and `t` (namely, if `t` lives at byte offset 0). `repr(C)` guarantees
    // that each field is located at the minimum address that satisfies the
    // field's alignment, which means that there is guaranteed to be no padding
    // between `_a` and `t`. Finally, if `size_of::<T>() > 0` and
    // `size_of::<T>()` is not a multiple of `align_of::<A>()`, there will be
    // trailing padding to ensure that `Align<T, A>`'s size is a multiple of its
    // alignment.
    _a: [A; 0],
    t: T,
    // INVARIANT: Any padding bytes are initialized. This is required in order
    // to make it sound for `Align<T, A>` to implement `AsBytes`.
}

This type was originally proposed (though never merged) here, and a more up-to-date prototype is in e8148f6.

See also this proposal for eliding alignment checks using Align.

@ChayimFriedman2
Copy link

Just looked and noticed two soundness holes in the prototype (feel free to ignore if you aren't going to use it anyway):

  1. In Align::new(), you return the constructed Align. However, this creates a typed copy, and typed does not preserve padding bytes (it makes them uninit). So all the effort gone into initializing them is lost. This can be fixed by wrapping Align in MaybeUninit (I believe), or any other union if we consider unions to be just a bag of bytes (but I don't think this was FCP'ed).
  2. In from_bytes() and from_bytes_mut() that take &Align<ByteArray<T>, A> you return &Align<T, A>, but this is unsound: ByteArray has an alignment of 1, so the value is definitely aligned to A but not to T. What we can have, however, is a conversion from &Align<ByteArray<T>, T> to &T.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility-nonbreaking Changes that are (likely to be) non-breaking
Projects
None yet
Development

No branches or pull requests

2 participants