Change alignment of virtiofs structs to be based on the first field.

[joannejchen]

Alignment of the virtiofs struct was previously based on the alignment of the largest field in fuse_in_header/fuse_out_header. Switched to explicitly align on u32, the type of the first field in the header.

Fixes #691

[mkroening]

Oh, sorry, the issue was not very detailed. I edited the original issue to explain it better.

For C structs, the correct alignment is not the alignment of the first field, but the maximum alignment of all fields. For example, align = max(align_of::<fuse_in_header>(), align_of::<fuse_lookup_in>()) would be correct for the case from the issue (this omits the alignment of the byte slice, which is 1).

Furthermore, assertions comparing our calculated layout to the correct layout that the compiler calculates would be very useful. This would be something like assert_eq!(layout, Layout::from_value(&cmd)).

[joannejchen]

Thanks for the extra information and sorry for all of the updates I just pushed -- was trying to figure out some issues that I noticed were happening in the CI.

I updated the files to take the maximum alignment across all of the fields (except for cases like the example you gave where there was another value with a known constant alignment/type). I then added assertions to check whether the layout we created was the same as the layout generated.
One thing I noticed in the CI with these assertions is that the layout we generated using Layout::from_size_align did not adjust the size to be a multiple of the alignment whereas the compiler-generated layouts did. This initially resulted in assertions failing because our Layout would have a size of 93 and an alignment of 8 whereas the compiler generated layout would have a size of 96 and an alignment of 8. To circumvent this, I used pad_to_align() in the assertions to make the layout size a multiple of the alignment, but let me know if the intended behavior is to apply pad_to_align our generated layout.

[mkroening]

Thanks for working on this! :)
No worries about force pushing to test with CI, that's how I do things myself.

About the padding: in the end, our layout has to match the one computed by Rust. So adjusting our layout with pad_to_align would be the correct thing to do. This might uncover new issues but we'll tackle them when we get there and maybe everything will be fine.

Btw, it's awesome we found this. This might corrupt the memory allocator otherwise, because after creating the Box, Rust deallocates with its computed layout, which is currently larger than that of the manual allocation. This is explicitly forbidden by the allocator APIs.

Also, you can directly unwrap the Layout after creation. No need to carry around the result and unwrap multiple times. The layout in the Result is Copy, so that would make things easier.

[mkroening]

Looks good to me! 👍
Thanks a lot :)

bors r+

PS: I opened a follow-up issue for deduplicating the DST creation code: #713

[bors]

Build succeeded:

• Build
• Check
• Check docs
• ci/gitlab/git.rwth-aachen.de
• Clippy
• Format
• Run RustyHermit (aarch64)
• Run RustyHermit (x86_64)
• Test