Struct tail ordered differently when unsizing

[y21]

I tried to make the MRE shorter, but this code is very sensitive to changes.

I tried this code under Miri (playground):

#![forbid(unsafe_code)]

use std::ptr::NonNull;

trait Trace {}
impl Trace for Box<i32> {}

struct GcNode<T: ?Sized> {
  refcount: u64,
  next: Option<NonNull<GcNode<dyn Trace>>>,
  value: T,
}

fn main() {
  let node: Box<GcNode<dyn Trace>> = Box::new(GcNode {
      refcount: 0,
      next: None,
      value: Box::new(0),
  });
  dbg!(node.refcount);
}

I expected to see this happen: no error.

Instead, this happened:

error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
  --> src/main.rs:20:3
   |
20 |   dbg!(node.refcount);
   |   ^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE:
   = note: inside `main` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/macros.rs:352:13: 352:16
   = note: this error originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info)

Meta

rustc --version --verbose:

❯ rustc -Vv
rustc 1.71.0-nightly (cca7ee581 2023-05-27)
binary: rustc
commit-hash: cca7ee58110726983951a19d5fb7316d9243925d
commit-date: 2023-05-27
host: x86_64-unknown-linux-gnu
release: 1.71.0-nightly
LLVM version: 16.0.4

[lukas-code]

Looks like the fields in GcNode<Box<i32> are ordered differently than GcNode<dyn Trace>, even though they absolutely shouldn't be:

#![feature(offset_of)]
#![allow(dead_code)]
#![forbid(unsafe_code)]

use std::ptr::NonNull;

trait Trace {}
impl Trace for Box<i32> {}

struct GcNode<T: ?Sized> {
    refcount: u64,
    next: (usize, usize),
    value: T,
}

fn main() {
    println!("offset in GcNode<Box<i32>> = {}", core::mem::offset_of!(GcNode<Box<i32>>, refcount));
    println!("offset in GcNode<dyn Trace> = {}", core::mem::offset_of!(GcNode<dyn Trace>, refcount));

    let node: Box<GcNode<dyn Trace>> = Box::new(GcNode {
        refcount: 1,
        next: (2, 3),
        value: Box::new(0),
    });
    
    dbg!(node.refcount, node.next);
}

prints:

offset in GcNode<Box<i32>> = 0
offset in GcNode<dyn Trace> = 16
[src/main.rs:26] node.refcount = 3
[src/main.rs:26] node.next = (
    1,
    2,
)

regressed in #108106

@rustbot label A-layout I-unsound

[Jules-Bertholet]

@rustbot label regression-from-stable-to-stable

[lukas-code]

This is a regression to the next beta (1.71), not the one that will be stable this week.

@rustbot label -regression-from-stable-to-stable +regression-from-stable-to-beta

Also, the struct tail is in the correct place, it's the other two fields that get reordered.

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

[lcnr]

this is not yet on beta, slightly minimized example which segfaults:

use std::fmt::Debug;

#[derive(Debug)]
struct GcNode<T: ?Sized> {
  gets_swapped_with_next: usize,
  next: Option<&'static GcNode<dyn Debug>>,
  tail: T,
}

fn main() {
  let node: Box<GcNode<dyn Debug>> = Box::new(GcNode {
      gets_swapped_with_next: 1,
      next: None,
      tail: Box::new(1),
  });
  
  println!("{:?}", node.next);
}

upgrading to P-critical, imo this must not get to stable.

[lcnr]

issue is not recursion afaict you need 1) a widepointer and 2) the tail needs a better - idk what better means here - niche than the widepointer which then gets yeeted via unsizing:

struct WideptrField<T: ?Sized> {
  first: usize,
  second: usize,
  option_of_wideptr: Option<&'static str>,
  tail: T,
}

fn main() {
  let unsized_value: Box<WideptrField<dyn Send>> = Box::new(WideptrField {
      first: 0xdead,
      second: 0xbeef,
      option_of_wideptr: None,
      tail: Some(1),
  });
  
  let wideptr = unsized_value.option_of_wideptr.unwrap();
  println!("{:p} {:x}", wideptr, wideptr.len());
  println!("{:?}", wideptr); // pointer deref -> segfault.
}

[the8472]

Oh, thanks for catching that. I totally didn't have unsized types in mind when doing those optimizations.

I'm now rereading the UCG on struct layout and it looks like I also invalidated its section on current behavior. But that should only require updating the UCG, not further fixes.

[lukas-code]

The fix has been backported to beta in #113000, so I think this can be closed now.

[the8472]

the assert from #112062 (comment) would still be nice to have.