std::mem::transmute may mutate result

[jmeggitt]

Example Code

During some experimenting in the Rust playground, I noticed something strange with the assembly output from transmute.

pub unsafe fn foo(x: u8) -> bool {
    ::std::mem::transmute(x)
}

Playground

Expected Output

transmute should copy the input memory with no mutations. I expected this to put x in the return register and return:

playground::foo:
	movl	%edi, %eax
	retq

Actual Output

However the resulting value gets modified in the process producing the following output:

playground::foo:
	movl	%edi, %eax
	andb	$1, %al
	retq

Now, I don't have any issue with the compiler choosing any arbitrary approach for representing a bool within std::mem::size_of::<bool>() bytes. However, I was under the impression that std::mem::transmute never mutates the underlying data. To quote the documentation, "It’s equivalent to C’s memcpy under the hood, just like transmute_copy." At the moment it feels like this contract is not being upheld.

Structs

However, the bigger issue is transmute may or may not apply this adjustment to bool fields in structs. This could probably hide some nasty bugs that would be extremely hard to debug. Take for example the code below:

impl Bar {
    pub fn from_buffer(buffer: [u8; BAR_SIZE]) -> Self {
        unsafe { ::std::mem::transmute(buffer) }
    }

    pub fn to_buffer(self) -> [u8; BAR_SIZE] {
        unsafe { ::std::mem::transmute(self) }
    }
}

pub fn main() {
    let buffer: [u8; BAR_SIZE] = [0xFF; BAR_SIZE];
    let identity = Bar::from_buffer(buffer).to_buffer();
    assert_eq!(buffer, identity);
}

(Playground with panic (small Bar)) (Playground no panic (large Bar))
It is not possible to tell if this code will panic without seeing the contents of Bar. The primary factors that determine if a panic occurs seems to be the size of Bar and at when in the compilation process from_buffer/to_buffer are inlined. Small structs with less fields are more likely to have their bools adjusted. Depending on which function the transmute is in, the compiler may determine that the transmutes will cancel out and will not produce an error.

Meta

rustc --version --verbose:

jaspermeggitt@Jaspers-Laptop:~/tmp$ rustc --verbose --version
rustc 1.60.0 (7737e0b5c 2022-04-04)
binary: rustc
commit-hash: 7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c
commit-date: 2022-04-04
host: x86_64-unknown-linux-gnu
release: 1.60.0
LLVM version: 14.0.0
jaspermeggitt@Jaspers-Laptop:~/tmp$ rustc +nightly --verbose --version
rustc 1.62.0-nightly (8f36334ca 2022-04-06)
binary: rustc
commit-hash: 8f36334ca939a67cce3f37f24953ff6f2d3f3d33
commit-date: 2022-04-06
host: x86_64-unknown-linux-gnu
release: 1.62.0-nightly
LLVM version: 14.0.0

[tmiasko]

The transmute has to produce a valid value. For a bool type there are only two valid values: 0 and 1. Anything else would result in an undefined behaviour. For example, when running linked playground code under Miri (available in Tools), Miri identifies the following issue:

error: Undefined Behavior: type validation failed at .b: encountered 0xff, but expected a boolean
  --> src/main.rs:11:18
   |
11 |         unsafe { ::std::mem::transmute(buffer) }
   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed at .b: encountered 0xff, but expected a boolean

[jmeggitt]

@tmiasko transmute was never intended to be a safe operation and we should not pretend it is. It does not need to produce a valid value and there are countless ways it can produce undefined behavior. Miri is correct for identifying undefined behavior, but is is not transmute's job to fix it. This behavior is in direct conflict with the documentation and it does not attempt to fix bools on larger types so it seems unlikely that this was the desired outcome.

To be honest, I am doubtful that transmute is even to blame for the issue. It seems more likely that transmute simply enables an environment where this issue can occur. My guess is that this has to do with how the compiler handles function return values. A bool is seen as a 1 bit integer so the compiler ensures that upon returning a bool, the padding is zeroed in accordance with that contract. Types composed of 1 or 2 small values can be seen as an aggregate type for which this contract must be upheld for each member. However larger types probably get treated as an anonymous sized block of memory so it does not get the opportunity to apply the same checks to all of their fields. Or at least that is my speculation on what is actually happening. I don't know enough about how the compiler works to verify it though.

[oli-obk]

Maybe the problem is LLVM just getting a bit confused because a bool value is actually i1, and it always masking away the extra bits when coming from anything larger. So we lower the transmute correctly, but that transfer of the transmuted value to an actual i1 in llvm will add the masking

[Kixiron]

It's undefined behavior for a boolean to have any values other than 1 or 0, so this is valid codegen on LLVM's part. When transmuting a u8 into a bool you're asserting the validity of that u8 as a i1/bool and therefore LLVM is fully within its rights to treat the possibility of other values (non-zero & non-one) as UB. (Much like tmiasko pointed out)
However, this isn't LLVM "fixing" anything. This is simply LLVM trusting you that your value is a valid i1 and therefore acting as such. This shouldn't be regarded as a bug in either rustc or LLVM any more than dereferencing a null pointer causing a segfault is, it's 100% intended behavior based off of the invariants you've stated to the compiler.

it does not attempt to fix bools on larger types so it seems unlikely that this was the desired outcome.

Undefined behavior is undefined behavior. The compiler can do whatever it wants to with this code, for all you know it could decide to explode if values other than zero or one are passed to the function.

Yes, this can cause nasty bugs as UB is prone to do but that's a side effect of falling afoul of UB, not a compiler or backend bug of any sort.

[zirconium-n]

It's undefined behavior for a boolean to have any values other than 1 or 0, so this is valid codegen on LLVM's part.

While it's valid codegen, it's missing optimization. The andb $1, %al part should not be here.

I disagree the Struct part in the OP though.

[RalfJung]

It is not possible to tell if this code will panic without seeing the contents of Bar.

It is not possible to tell if this code has Undefined Behavior or not without seeing the contents of Bar. But I don't think that's a bug -- that is just inherent in how transmute works.

[workingjubilee]

While it's valid codegen, it's missing optimization. The andb $1, %al part should not be here.

Do you have an example of this use-case being a performance bottleneck in a sample program with practical effects that we can benchmark?

LLVM insists any boolean value, in most practical cases, to interact with most of LLVM's instruction set that is intended to model "compare then act" in assembly, become an i1. Rust takes the path of least resistance and says that bool is indeed an i1. So in reality, while passing around those bools later, LLVM would be within its rights to apply the andb truncation anyways before any other comparison that it happened to do. This is because it is not just LLVM's job to generate fast code, it is also its job to generate correct code, and one way it can do that is by forcing users to push boolean data into a type that can indeed only hold literally one bit.

In fact, in a fully "worked" optimization of practical code, by making sure it can pretend this is only talking about single bits, LLVM may even happily choose to turn all this into flag-only operations, causing the byte in this conversation to vanish in a puff of logic. And the Rust abstract machine definitely has no idea what a "flags register" even is, in spite of generated x86 code using such.

[RalfJung]

However, I was under the impression that std::mem::transmute never mutates the underlying data.

It doesn't -- for programs that do not have UB. For programs that have UB, no promises are made. Ever. That's how unsafe works.

Passing any value other than 0 or 1 to your foo causes UB, and for 0 and 1 the underlying data is preserved. So, I think we should close this issue as not-a-bug. The docs state:

Neither the original, nor the result, may be an invalid value.

[oli-obk]

Yea, the only actionable thing I can see here is to remove the extra asm instructions for perf reasons or to allow follow up optimizations to trigger.

I guess let's close this and if someone encounters a repro for more complex programs that have missing opts we can revisit