Tracking Issue for future-incompatibility warning unaligned_references

[RalfJung]

This is a tracking issue for the future-incompatibility warning unaligned_references.
This warning will fire for code like the following:

#[repr(packed)]
struct Foo1 {
    bar: u8,
    baz: usize
}

let foo = Foo1 { bar: 1, baz: 2 };
// Direct field accesses are fine.
let val = foo.baz;
// However, creating a reference is not.
let brw = &foo.baz; //~WARN reference to packed field is unaligned
// Format strings implicitly create references.
println!("{}", foo.baz); //~WARN reference to packed field is unaligned

The reason this pattern is being phased out is that Rust requires references to always be aligned; creating an unaligned reference falls under the "creating an invalid value" clause in the Rust definition of Undefined Behavior. Fields of packed structs are not necessarily properly aligned. Hence creating a reference to a field of a packed struct can cause UB, even if it is never used, and even inside an unsafe block. This is a soundness bug, which is fixed by deprecating and eventually disallowing this pattern.

Previously, a future-incompatibility warning was emitted when creating references to packed fields outside an unsafe block; however, that warning was incorrectly silenced inside unsafe blocks.

To fix this code, it needs to stop creating a reference to a packed field. The alternative is to either just copy the packed field by adding curly braces (the compiler knows how to do that despite lack of alignment), or to create a raw pointer:

let mut foo = Foo1 { bar: 1, baz: 2 };

let brw = std::ptr::addr_of!(foo.baz); // Create an immutable raw pointer to the packed field.
let val = unsafe { brw.read_unaligned() }; // Perform an unaligned read of that pointer.
let brw_mut = std::ptr::addr_of_mut!(foo.baz); // Create a mutable raw pointer to the packed field.
unsafe { brw_mut.write_unaligned(val+1); } // Perform an unaligned write to that pointer.

// For formatting, adding curly braces means that a copy of the field is made, stored
// in a (properly aligned) temporary, and a reference to that temporary is being formatted.
println!("{}", {foo.baz});
// This is equivalent to the more verbose
println!("{}", {let copy = foo.baz; copy});

For further background, see #46043, #27060.

[golddranks]

Should this be deny-by-default for the 2021 edition, or is it too early for this list (and too late for the edition)?

[RalfJung]

IIRC the consensus was that since this problem affects all editions, it would not make sense to make it deny-by-default in only in some editions.

[osa1]

Sorry if this is not the right place for this question, but I'm curious about how to do method calls with &self receivers now when the receiver is potentially unaligned. Examples in the original post do not show this. Example:

#[repr(packed)]
pub struct Obj {
    pub field: String,
}

pub unsafe fn test(concat: *const Obj) {
    println!("{}", (*concat).field.len());
    //             ^^^^^^^^^^^^^^^ "reference to packed field is unaligned"
}

Here a reference is created implicitly for the method call, causing the warning.

Since the method is defined in a library I can't make it a *const self method (with arbitrary_self_types). What is the suggested fix for cases like this?

When I have the control over called methods, do I make the methods take a *const/mut self receiver? Or is there a better way to fix this?

When compiling to platforms with no alignment restrictions (such as Wasm) is there a way to relax this check?

Finally, looking at https://doc.rust-lang.org/reference/behavior-considered-undefined.html (linked in the issue description), it doesn't mention unaligned references. Should it be updated? It also doesn't link to this issue.

[RalfJung]

I'm curious about how to do method calls with &self receivers now when the receiver is potentially unaligned

It is not possible (and has never been possible) to call such methods on packed fields. You have to first move the data to some well-aligned place, and then call the method.

Finally, looking at https://doc.rust-lang.org/reference/behavior-considered-undefined.html (linked in the issue description), it doesn't mention unaligned references.

It does, the following is listed as UB due to producing an invalid value: "A reference or Box that is dangling, unaligned, or points to an invalid value."

[osa1]

It is not possible (and has never been possible) to call such methods on packed fields.

I'm confused by this. My example above does exactly this, and it used to work without warnings a few versions ago. It works today, but generates a warning. Did I misunderstand what you mean? Do you mean it's never supposed to work, rather than it never worked?

[RalfJung]

It's been 7.5 years since #27060 was reported, but the problem is finally fixed for good. :)

[jrose-signal]

I ran into #110777 today, but now I'm wondering why you can't form a reference to a field x in a repr(packed) struct where the type of x is itself repr(packed). That would allow my use of Debug to continue working.

[mathstuf]

Because x may be misaligned within the parent struct for its own alignment rules (which are not affected by repr(packed) AFAIK). I think here you'd want something like #[allow_unaligned] on the type of x so that nothing can assume anything about the alignment of the type.

[jrose-signal]

Oh, I forgot that repr(packed) can take an argument. And proposing that it's allowed for structs that happen to have an alignment of 1 is too broad (since that may not be intended to be a part of the public interface of a struct). But it seems like if a struct explicitly has alignment 1 it could be allowed. (Again, I would not care about this so much except that we had derive(Debug) on a packed struct with nested packed structs, and they don't technically need to be Copy-ed to be printed because they all have alignment 1.)

[RalfJung]

I ran into #110777 today, but now I'm wondering why you can't form a reference to a field x in a repr(packed) struct where the type of x is itself repr(packed).

You can:

pub(crate) struct UInt16LE {
    bytes: [u8; 2],
}

#[repr(C, packed)] // struct has the same layout without the `packed`!
pub(crate) struct Header {
    version: u8,
    kind: UInt16LE,
}

const _: () = {
    assert!(std::mem::align_of::<Header>() == 1);
};

fn main() {
    let h = Header { version: 0, kind: UInt16LE { bytes: [0, 0] }};
    let r = &h.kind; // taking the reference
}

Your issue is that derive(Debug) has to guess whether it should make a copy or take a reference, without having any type information. (derive macros run before type names are resolved.) For packed structs it guesses making a copy, and that's why you see an error. The only code where this used to work and now errors is code where the packed does not change the layout in any way, which was deemed acceptable breakage. Sorry that it hit you!

[jrose-signal]

That makes sense. For some reason I was hoping there was some kind of autoref trick involved rather than the derive-macro simply making a choice, but it makes sense that it does not do this. Sorry for the noise, then! (And thanks everyone for the help in both places.)

[scottmcm]

The derive macro can't know whether the field types are 1-aligned or packed, so it does the common thing. If you need something else, you'll have to write out the impl yourself.