Rc/Arc for DST compute layout incorrectly

[RalfJung]

#54922 introduced a critical bug where the layout of an allocation is computed incorrectly, leading (at least) to a deallocation with a different size than the allocation.

This is easily demonstrated by cherry-picking RalfJung@c2e5933 and running ./x.py --stage 1 test src/liballoc (left is the correct layout, right is what we compute):

---- rc::tests::test_clone_from_slice stdout ----
thread 'rc::tests::test_clone_from_slice' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 32, align_: 8 }`,
 right: `Layout { size_: 28, align_: 8 }`', liballoc/rc.rs:683:9

---- rc::tests::test_copy_from_slice stdout ----
thread 'rc::tests::test_copy_from_slice' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 32, align_: 8 }`,
 right: `Layout { size_: 28, align_: 8 }`', liballoc/rc.rs:683:9

---- rc::tests::test_from_box_slice stdout ----
thread 'rc::tests::test_from_box_slice' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 32, align_: 8 }`,
 right: `Layout { size_: 28, align_: 8 }`', liballoc/rc.rs:683:9

---- rc::tests::test_from_box stdout ----
thread 'rc::tests::test_from_box' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 24, align_: 8 }`,
 right: `Layout { size_: 20, align_: 8 }`', liballoc/rc.rs:683:9

---- rc::tests::test_from_box_str stdout ----
thread 'rc::tests::test_from_box_str' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 24, align_: 8 }`,
 right: `Layout { size_: 19, align_: 8 }`', liballoc/rc.rs:683:9

---- rc::tests::test_from_box_trait stdout ----
thread 'rc::tests::test_from_box_trait' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 24, align_: 8 }`,
 right: `Layout { size_: 20, align_: 8 }`', liballoc/rc.rs:683:9

---- rc::tests::test_from_vec stdout ----
thread 'rc::tests::test_from_vec' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 32, align_: 8 }`,
 right: `Layout { size_: 28, align_: 8 }`', liballoc/rc.rs:683:9

---- rc::tests::test_from_str stdout ----
thread 'rc::tests::test_from_str' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 24, align_: 8 }`,
 right: `Layout { size_: 19, align_: 8 }`', liballoc/rc.rs:683:9

---- rc::tests::test_into_from_raw_unsized stdout ----
thread 'rc::tests::test_into_from_raw_unsized' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 24, align_: 8 }`,
 right: `Layout { size_: 19, align_: 8 }`', liballoc/rc.rs:683:9

---- sync::tests::test_clone_from_slice stdout ----
thread 'sync::tests::test_clone_from_slice' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 32, align_: 8 }`,
 right: `Layout { size_: 28, align_: 8 }`', liballoc/sync.rs:586:9

---- sync::tests::test_copy_from_slice stdout ----
thread 'sync::tests::test_copy_from_slice' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 32, align_: 8 }`,
 right: `Layout { size_: 28, align_: 8 }`', liballoc/sync.rs:586:9

---- sync::tests::test_from_box stdout ----
thread 'sync::tests::test_from_box' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 24, align_: 8 }`,
 right: `Layout { size_: 20, align_: 8 }`', liballoc/sync.rs:586:9

---- sync::tests::test_from_box_slice stdout ----
thread 'sync::tests::test_from_box_slice' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 32, align_: 8 }`,
 right: `Layout { size_: 28, align_: 8 }`', liballoc/sync.rs:586:9

---- sync::tests::test_from_box_str stdout ----
thread 'sync::tests::test_from_box_str' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 24, align_: 8 }`,
 right: `Layout { size_: 19, align_: 8 }`', liballoc/sync.rs:586:9

---- sync::tests::test_from_box_trait stdout ----
thread 'sync::tests::test_from_box_trait' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 24, align_: 8 }`,
 right: `Layout { size_: 20, align_: 8 }`', liballoc/sync.rs:586:9

---- sync::tests::test_from_str stdout ----
thread 'sync::tests::test_from_str' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 24, align_: 8 }`,
 right: `Layout { size_: 19, align_: 8 }`', liballoc/sync.rs:586:9

---- sync::tests::test_from_vec stdout ----
thread 'sync::tests::test_from_vec' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 32, align_: 8 }`,
 right: `Layout { size_: 28, align_: 8 }`', liballoc/sync.rs:586:9

---- sync::tests::test_into_from_raw_unsized stdout ----
thread 'sync::tests::test_into_from_raw_unsized' panicked at 'assertion failed: `(left == right)`
  left: `Layout { size_: 24, align_: 8 }`,
 right: `Layout { size_: 19, align_: 8 }`', liballoc/sync.rs:586:9

Cc @murarth

[RalfJung]

Looking at the data, I think the step that is missing is to make the size a multiple of the alignment. extend should probably not do that (when adding several fields you would not want this intermediate step), but whoever calls extend likely wants to finish of by doing that.

To make the API safer it might even be useful to make extend return IncompleteLayout, and make Layout have the invariant that size is divisible by alignment? IncompleteLayout could have a method finish that returns Layout and rounds the size up to the next multiple of alignment.

Cc @eddyb

[RalfJung]

Hm, that would however change the behavior of a stable type. (extend() is unstable, but Layout::from_size_and_align is stable). Not sure if we can do that.

The default allocator could at least debug_assert! that size is divisible by alignment, though? (Or assert!, that check is cheap compared to allocating.)

[gnzlbg]

To make the API safer it might even be useful to make extend return IncompleteLayout, and make Layout have the invariant that size is divisible by alignment?

cc @Amanieu @SimonSapin

[Amanieu]

Maybe add a method called pad_to_align() which rounds the size of a Layout up to the next multiple of its alignment?

[SimonSapin]

Looking at the data, I think the step that is missing is to make the size a multiple of the alignment. extend should probably not do that (when adding several fields you would not want this intermediate step), but whoever calls extend likely wants to finish of by doing that.

extend is currently documented as returning a layout equivalent to that of a two-field struct with #[repr(C)], which would imply this rounding up. But yes, indeed we’d only want to round once if a single "struct" with more than two fields.

[RumataEstor]

What if Layout is added a field internal_size that is used for extending and the normal size is always a multiple of alignment?

[gnzlbg]

@RumataEstor it is probably better to raise that idea in the thread for the alloc trait: #32838