Add misaligned_transmute lint

[devonhollowood]

Fix #1384

[Manishearth]

Thanks!

[sellibitze]

I came from here and noticed that the test in transmute.rs includes

let _: u32 = unsafe { std::mem::transmute([0u8; 4]) }; // err

But I don't actually see an alignment problem here as _ would get its own properly aligned storage.

Alignment is usually an issue when pointers & references are involved, e.g. creating a &T from an &U where T has higher alignment requirements than U.

[oli-obk]

Indeed, this should probably be alright. I say "probably", because the semantics of transmute are severely underspecified. Feel free to constrain the lint to references or things containing references.

[devonhollowood]

@sellibitze @oli-obk Are you sure? Maybe I am misreading this assembly, but for example in this function the [u8; 4] doesn't look like it gets copied:

Rust:

pub fn foo(input: &[u8; 4]) -> bool {
    let y: u32 = unsafe {std::mem::transmute(*input)};
    y == 123456
}

x86 (compiled with -O):

example::foo:
  push rbp
  mov rbp, rsp
  cmp dword ptr [rdi], 123456
  sete al
  pop rbp
  ret

Compiler Explorer link

edit: You can remove the pointer entirely in that input and get the same issue:

Rust:

pub fn foo(input: [u8; 4]) -> bool {
    let y: u32 = unsafe {std::mem::transmute(input)};
    y == 123456
}

x86:

example::foo:
  push rbp
  mov rbp, rsp
  cmp edi, 123456
  sete al
  pop rbp
  ret

[oli-obk]

@devonhollowood the u32 still needs to be aligned. Irrelevant where its bits come from

[devonhollowood]

@oli-obk I think I was somewhat unclear. I agree that the u32 needs to be aligned--the point I am trying to make is that I think this lint is still relevant for situations that don't contain references. In the case that I posted, my understanding is that the u32 is not necessarily properly aligned, because it does not get copied to its own properly aligned storage (separate from the [u8; 4]).

[oli-obk]

I don't know enough assembler to make a qualified statement about that.

Is the edi register 4 byte aligned?

[sellibitze]

@devonhollowood: Very interesting! I guess LLVM optimizes the temporary away and directly accesses the [u8; 4]'s memory as a u32 in your first foo function under the assumption that the alignment is compatible. I'd say the implementation of mem::transmute is broken in that it allows LLVM to do this kind of optimization. It's time to summon the compiler gurus. :)

@oli-obk: registers don't have addresses themselves, just names. But edi could store an odd memory address that points to a [u8;4] in which case dword ptr [edi] wold be a misaligned memory access.

[oli-obk]

that means we're on the safe side assuming that small align -> large align transmutes are wrong?

[sellibitze]

I maintain that

pub fn foo(input: &[u8; 4]) -> bool {
    let y: u32 = unsafe {std::mem::transmute(*input)};
    y == 123456
}

is (or at least should be) perfectly OK and no error. Its observable behaviour is just implementation-defined (depending on the endianness). mem::transmute is defined in terms of copying raw bits. The function call expression is an rvalue. The source/target type's alignment doesn't matter. Where alignment matters are transmutes of the form &T -> &U, *const T -> *const U and their mut equivalents ... and it's the alignment of the pointee type.

The assembly code we've seen is what LLVM comes up with after optimization. It made the program access the possibly unaligned memory via dword ptr [edi]. But ... IIRC that's fine on x64 architectures. So, I don't think this assembly code proves much. The compiler is allowed to do all kinds of transformations. That's what optimizers do.

I think the question is whether the optimizer makes an assumption that it shouldn't make (in which case the implementation of mem::transmute would be broken). But I can't tell whether that's the case. The data seems inconclusive to me.

But I might be wrong. Someone more involved with rustc / LLVM should weigh in, IMHO.

[devonhollowood]

@sellibitze Okay, so I went back and re-read the relevant docs.

The transmute docs say

transmute is semantically equivalent to a bitwise move of one type into another. It copies the bits from the source value into the destination value, then forgets the original. It's equivalent to C's memcpy under the hood, just like transmute_copy

The nomicon section on transmute doesn't mention alignment.

The Rust Reference's list of UB says:

Rust code, including within unsafe blocks and unsafe functions is incorrect if it exhibits any of the behaviors in the following list. It is the programmer's responsibility when writing unsafe code that it is not possible to let safe code exhibit these behaviors.

• ...
• Unaligned pointer reading and writing outside of read_unaligned and write_unaligned.
• ...

Because this so specifically only refers to referencing unaligned pointers, and because transmute is defined to be equivalent to memcpy (instead of just reinterpreting the underlying bytes), I think you are right: my two examples really are safe, and this lint should be restricted to just pointer types.