deref_addrof can suggest adding UB

[CAD97]

Summary

Reading/writing a place requires that place to be based on an aligned pointer, but taking the reference of a place only requires the referenced place to be aligned. These facts together mean that {*&(*ptr).0} is a semantically weaker operation than {(*ptr).0}, but clippy::deref_addrof will suggest changing the former to the latter, which potentially introduces UB to a program.

This might be considered reasonable (i.e. not a false positive), as semantically relying on this is unreasonably subtle. However, the lint also still fires when using &raw, which should not be happening when the place is potentially based on a misaligned pointer.

Note: the lint is still useful and should be fired for immediately dereferencing a pointer (*&raw) when the place is not based on a (potentially misaligned) pointer, as it is still an operational no-op when the place is based on an aligned pointer (or reference), and removing such allows *&raw to semantically indicate lowering the implied alignment of a place.

Lint Name

clippy::deref_addrof

Reproducer

I tried this code:

unsafe fn foo(ptr: *mut (u32, u64)) {
    *&mut (*ptr).0 = 5;
    let v = *&(*ptr).0;
    assert_eq!(v, 5);
}

I saw this happen:

warning: immediately dereferencing a reference
 --> src/main.rs:4:5
  |
4 |     *&mut (*ptr).0 = 5;
  |     ^^^^^^^^^^^^^^ help: try: `(*ptr).0`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof
  = note: `#[warn(clippy::deref_addrof)]` on by default

warning: immediately dereferencing a reference
 --> src/main.rs:5:13
  |
5 |     let v = *&(*ptr).0;
  |             ^^^^^^^^^^ help: try: `(*ptr).0`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof

I expected to see this happen:

Applying the warnings' suggested fix should not introduce UB.

Version

rustc: 1.82.0-nightly (2024-08-15 2c93fabd98d2c183bcb3)
clippy; 0.1.82 (2024-08-15 2c93fab)
miri: 0.1.0 (2024-08-15 2c93fab)

Additional Labels

@rustbot label +I-suggestion-causes-ub

:cheeky:

[CAD97]

Note: &raw is currently unstable, but is in FCP for stabilization at rust-lang/rust#127679.

[Jarcho]

Unrelated to clippy being wrong here, what's the rational for accessing being UB, but borrowing being ok?

[CAD97]

• replace 'UB on raw ptr deref' with UB on place projection/access reference#1387

It not being UB at the pointer dereference is what allows addr_of!(*ptr) to be valid. In theory, taking reference of a place could've required the place to be derived from aligned, but there's no meaningful benefit to adding that rule, and all else being equal less UB is better. Direct place access requires the base place's alignment such that e.g. loads from the field of #[align(8)] Aligned([u8; 8]) can use the higher alignment which is provided by the context.

This difference is subtle, yes, which isn't great, but the rule before was that a raw pointer must be aligned to dereference it at all. That is still sufficient, and the real rule is a relaxation that accepts more code without losing optimization.

It's edges like this where we have to carefully balance the ability to write performant code with the need to not unreasonably footgun developers. I think that a clippy suggestion to use *&raw const (*ptr).0 when alignment is being lowered and (*ptr).0 when it isn't (when a deref_addrof was written originally) will help push people in the right direction here.

But also, on the other hand, there are good arguments to avoid direct place access behind pointers and always use the .read()/.write() methods instead, avoiding the prior value drop.