Is Rc’s and Arc’s data_offset correct?

[SimonSapin]

In the implementation of Arc, we find:

rust/src/liballoc/sync.rs

Lines 258 to 269 in 09ab31b

	struct ArcInner<T: ?Sized> {
	strong: atomic::AtomicUsize,
	// the value usize::MAX acts as a sentinel for temporarily "locking" the
	// ability to upgrade weak pointers or downgrade strong ones; this is used
	// to avoid races in `make_mut` and `get_mut`.
	weak: atomic::AtomicUsize,
	data: T,
	}

rust/src/liballoc/sync.rs

Lines 2288 to 2304 in 09ab31b

	/// Computes the offset of the data field within ArcInner.
	unsafe fn data_offset<T: ?Sized>(ptr: *const T) -> isize {
	// Align the unsized value to the end of the ArcInner.
	// Because it is ?Sized, it will always be the last field in memory.
	let align = align_of_val(&*ptr);
	let layout = Layout::new::<ArcInner<()>>();
	(layout.size() + layout.padding_needed_for(align)) as isize
	}
	/// Computes the offset of the data field within ArcInner.
	///
	/// Unlike [`data_offset`], this doesn't need the pointer, but it works only on `T: Sized`.
	fn data_offset_sized<T>() -> isize {
	let align = align_of::<T>();
	let layout = Layout::new::<ArcInner<()>>();
	(layout.size() + layout.padding_needed_for(align)) as isize
	}

(And similarly in rc.rs with struct RcBox.)

Note the comment: Because it is ?Sized, it will always be the last field in memory.

Is this actually true, even for instantiations where T happens to be Sized? Does the language guarantee that there exist no type Foo such that the fields of ArcInner<Foo> would be re-ordered?

Or do we need to add #[repr(C)] to ArcInner and RcBox?

CC @rust-lang/wg-unsafe-code-guidelines

[hanna-kruppe]

If this were user-facing code it would be a clear case of "no that's not specified, don't assume that". However, I believe rustc does currently honor this assumption and libstd can of course rely on rustc implementation details. @eddyb would know best.

Is this actually true, even for instantiations where T happens to be Sized?

Note that because unsizing happens with "reorganizing" the struct, the struct with a sized T has to be laid out "as if unsized" from the start. For example, Rc<u8> and Rc<u128> and many other Rc<T: Sized> can all be casted to Rc<dyn Debug> without touching the RcInner, the differences in layout between those are quite constrained (limited to different amounts of paddding to respect alignment, I think?).

This reasoning wouldn't apply if there was a type that couldn't ever be involved in unsizing, but I don't think the compiler can ever realistically rule that out locally, and I'm rather certain that it doesn't today.

[SimonSapin]

Ah that’s a good point. The constraint that layout cannot change while unsizing is probably enough indeed.

[Centril]

If this were user-facing code it would be a clear case of "no that's not specified, don't assume that". However, I believe rustc does currently honor this assumption and libstd can of course rely on rustc implementation details.

We should probably add a comment to that effect so "don't assume that" is clearer to everyone.

[RalfJung]

We should probably add a comment to that effect so "don't assume that" is clearer to everyone.

We should! See rust-lang/unsafe-code-guidelines#90.

[RalfJung]

FWIW these are repr(C) nowadays so this got properly fixed at some point.