Is Rc’s and Arc’s data_offset correct?

In the implementation of Arc, we find:

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

@eddyb

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.

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

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.

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.

Rollup merge of rust-lang#67504 - Mark-Simulacrum:note-data-offset, r…

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

Navigation Menu

Repository navigation

Is Rc’s and Arc’s data_offset correct? #62522

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development


	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,
	}

	/// 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
	}

Is Rc’s and Arc’s data_offset correct? #62522

Description

Activity

hanna-kruppe commented on Jul 9, 2019

SimonSapin commented on Jul 9, 2019

Centril commented on Jul 9, 2019

RalfJung commented on Jul 10, 2019

RalfJung commented on Aug 13, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions