Pad BlobVec item size to multiple of alignment

[rj00a]

Objective

Some of the BlobVec code assumes the size of the contained item is a multiple of its alignment. However, std::alloc::Layout by itself does not enforce this, and there are no checks in BlobVec::new. (There is a debug assertion elsewhere, but it can be bypassed in release mode). Functions like BlobVec::get_unchecked will produce pointers that are not properly aligned.

This problem is potentially observable with World::init_component_with_descriptor.

Solution

• Round up the size of the layout to a multiple of its alignment in BlobVec::new.
• Add test to demonstrate the problem.

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[alice-i-cecile]

Is there a tracking issue for this?

[rj00a]

The utility methods for Layout are part of alloc_layout_extra

[alice-i-cecile]

Suggested change

	// TODO: replace with `Layout::pad_to_align` if/when it stabilizes
	// TODO: replace with `Layout::pad_to_align` if/when it stabilizes (https://github.com/rust-lang/rust/issues/55724)

[rj00a]

Ok so simply changing the size of item_layout doesn't work because initialize_unchecked will copy more data from the src pointer than it's supposed to, hence the Miri error. Might want to track "item size" and "item size + padding" as two separate things. That, or place some restrictions on item_layout to avoid the situation.

[robojeb]

Ok so simply changing the size of item_layout doesn't work because initialize_unchecked will copy more data from the src pointer than it's supposed to, hence the Miri error. Might want to track "item size" and "item size + padding" as two separate things. That, or place some restrictions on item_layout to avoid the situation.

It would make more sense to me to store cell_layout and item_size rather than item_layout + padding.

The common operations are going to be "Offset a pointer to find a cell in the array" and "Copy item_size bytes in/out of a cell". So it feels like less mental overhead to store those values.

[robojeb]

Does repeat_layout() need to recompute the padding anymore if self.item_layout is being modified in the constructor to be "Item Size + Padding" anyway?