`repr(transparent)` and requirements of `std::slice::from_raw_parts`

help
1

I wrote the following code to turn a slice of u8 with length divisible by 4 into a slice of Offset, where Offset is a repr(transparent) wrapper around [u8; 4]:

#[repr(transparent)]
pub struct Offset([u8; 4]);

// (Not strictly related to my question, but left in to help explain the motivation here.)
impl Into<u32> for Offset {
    fn into(self) -> u32 {
        u32::from_be_bytes(self.0)
    }
}

fn offsets_from_bytes<'a>(bytes: &'a [u8]) -> &'a [Offset] {
    assert!(bytes.len() % 4 == 0);
    let data = bytes.as_ptr() as *const Offset;
    let len = bytes.len() / 4;
    unsafe { std::slice::from_raw_parts(data, len) }
}

My thinking was that offsets_from_bytes is sound because of the repr(transparent), the length check, and the fact that [u8; 4] has the same alignment as u8 and there are no padding bytes anywhere. However, the std::slice::from_raw_parts documentation says, in part:

  • data must point to len consecutive properly initialized values of type T .

which is not true in this case. So, is the function sound as written? If not, how can I rewrite it soundly?

2

If you can be confident about the alignment being the same, I would also think that it's sound.

For what it's worth, that line was added in this PR, and the concern is initialization in particular. Earlier documentation comes from here; to my reading, anyway,

data must be valid for reads for len * mem::size_of::<T>() many bytes, and it must be properly aligned.

is the key part.

Perhaps the results of a PR to tweak the wording would give you a firmer answer :slight_smile:

Other references:

  • I think this is a specific instance of this UCG issue. The conclusion there is that a [[T; N]; M] is not necessarily a [T; N * M], because [T; N] may have an alignment greater than T. But no other objections were raised, and I agree that if the alignment and size match, it's implied by...
  • The UCG section on arrays and slices, particularly the guarantee about contents being contiguous in certain cases.

A lot of UCG output is not yet normative though, and I didn't dig far enough to know which parts are in this case.

2 Likes
3

I figure this is covered by the UCG's note about [T; N] and T having the same alignment when T is repr(C) the alignment of [T; N] when T is repr(C), which I interpret as including the scalar primitive types. (I actually didn't know that align_of::<[T; N]>() == align_of::<T>() was ever not guaranteed. TIL!)

[edit: the specific language is

If the element type is repr(C) the layout of the array is guaranteed to be the same as the layout of a C array with the same element type.

which I'm not really sure how to interpret. It's still hard for me to imagine how this wouldn't be sound.]

I might do this, yeah.

Thanks for the UCG pointers—I keep forgetting to look there if I can't find something on doc.rust-lang.org.

1 Like
4

You might consider doing this in two steps, as a way to simplify what you have to prove.

Notably, nightly has [T]::as_chunks. That can do the &[u8] → &[[u8; 4]] part (plus an assert on the remainder slice being empty), and then you just have the &[[u8; 4]] → &[Offset] part, which is easily argued because of the repr(transparent). You might even be able to delegate the proof to dtolnay via GitHub - dtolnay/ref-cast: Safely cast &T to &U where the struct U contains a single field of type T.

5

I could definitely simplify things with one of those approaches, but in this case I'm unwilling to require nightly or pull in a new dependency.

6

You could implement this instead:

impl From<Offset> for u32 {
    fn into(this: Offset) -> u32 {
        u32::from_be_bytes(this.0)
    }
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=ccb5fd8a78b6090284285d46d7947a5e
that will allow you to do u32::from(an_offset), and also let integer: u32 = an_offset.into();

7

Yes, I know. I prefer to just implement Into<u32> in this specific case; u32::from(offset) doesn't "feel right" to me and doesn't correspond to how I expect/want people to use the code.

8

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.