Confusion with double negation and booleans

[PieterPenninckx]

In certain circumstances, true != !false is considered true.

Steps to reproduce

1. Create a new project with cargo new negation --bin
2. Change the file src/main.rs so that the contents is the following:

fn g() -> bool {
    false
}

fn main() {
    let a = !g();
    if a != !g() {
        println!("wrong");
    } else {
        println!("correct");
    }
}

3.Run with cargo run

Expected and observed behavior

When run with cargo run, the compiled program prints wrong.
I expect correct to be printed because after the assignment,
a should still equal the value it was assigned to (note that g() always
returns the same value).

Note that cargo run --release results in printing correct, so even if it's me who is being confused, the output should still be the same in debug mode and in release mode.

Meta

When I do a normal rustc main.rs and witch cargo run --release the resulting
program behaves as I expect.
The unexpected behaviour only occurs when using cargo run (or running target/debug/negation).

rustc --version --verbose:

rustc 1.12.0 (3191fbae9 2016-09-23)
binary: rustc
commit-hash: 3191fbae9da539442351f883bdabcad0d72efcb6
commit-date: 2016-09-23
host: x86_64-unknown-linux-gnu
release: 1.12.0

cargo --version --verbose

cargo 0.13.0-nightly (109cb7c 2016-08-19)

Edit: added short description.

[TimNN]

This is... interesting

I tested on OS X with various rust versions with various flags:

	no flags	-g	-O	-g -O
1.2.0	correct	correct	correct	correct
1.3.0	wrong	wrong	correct	correct
1.10	wrong	wrong	correct	correct
1.11	wrong	wrong	correct	correct
1.12	correct	wrong	correct	correct
beta	correct	wrong	correct	correct
nightly	correct	wrong	correct	correct

[TimNN]

The first column seems to have been "fixed" by the switch to mir.

[eddyb]

The 1.12 change might mean MIR fixed an old trans bug. Unoptimized -g being wrong leads me to believe this has to do with whether we put variables in allocas.

Before MIR trans, we didn't have the infrastructure to handle turning let y = 5; x + y into add %x, 5.
We always used an alloca (a local stack slot in LLVM) for variables. With MIR trans, we can do better.

What hasn't changed, though, is the behavior with -g (debuginfo) turned on: while LLVM supports annotating values with debug information (i.e. saying that a variable y has value 5 without that 5 needing to be kept in memory), we haven't fully transitioned to that model so we still need allocas.

-g -O working means LLVM optimizations unbreak it, thus it's one of:

• LLVM codegen for the variable case and/or with no optimizations is wrong
• we generate bad IR for the variable case that somehow optimizes to the correct result

[TimNN]

(I updated the table above): 1.2.0 was correct in all cases, so marking as a stable-to-stable regression.

Also, I checked with -Z orbit=on/off which toggles the behaviour of the first column, leading me to suspect that mir was the fix.

[eddyb]

Minimal version for inspecting LLVM IR or assembly.

Cleaned up assembly:

call g
xor al, -1
and al, 1
mov byte ptr [rbp - 10], al
call g
xor al, -1
mov cl, byte ptr [rbp - 10]
cmp cl, al
je correct

With g returning 0 in al and byte ptr [rbp - 10] being the location of a, that's equivalent to:

mov al, 255
mov cl, 1
cmp cl, al
je correct

So LLVM turns xor i1 %0, true into xor al, -1, but that can't be correct when other instructions operating on i1 values end up leaving the rest of the bits 0 (i.e. and al, 1).

[eddyb]

clang doesn't hit this (assembly) because each zext from i1 appears to be an and 1, and it does the compare on i32 (i8 would also work AFAICT).

So we could just make compares use i8 instead, but I don't know what else is broken in LLVM.

[eddyb]

To further explain what MIR "fixed": both following forms also result in "correct" with -g:

fn main() {
    if !g() != !g() {
        println!("wrong");
    } else {
        println!("correct");
    }
}

fn main() {
    let a = !g();
    let b = !g();
    if a != b {
        println!("wrong");
    } else {
        println!("correct");
    }
}

Variables end up holding 1 for true while !g() is 255.

[pnkfelix]

we could just make compares use i8 instead

we may want to consider doing this. It seems like llvm bugs for comparing i8 would simply be more likely to be encountered and subsequently fixed in the wild by clang or other projects

[eddyb]

In that case, these two arguments need to be wrapped in from_immediate calls
(i.e. from_immediate(bcx, lhs) and from_immediate(bcx, rhs), respectively).

[nagisa]

https://llvm.org/bugs/show_bug.cgi?id=30579

[nagisa]

Something to note: MIPS, ARM and PowerPC should not be affected on quick visual inspection of the assembly.

[nikomatsakis]

I am in favor of working around this by using i8 or some other means. I think blocking on LLVM for perf fixes is one thing, but we should try to resolve correctness errors locally if we can.

[nikomatsakis]

(Preparing a patch.)

[nikomatsakis]

See PR #36958

[nagisa]

For all its worth, it seems like LLVM Trunk does not reproduce, though I didn’t check myself, yet. Backporting the LLVM patch would still make rustc compiled against 3.9/3.8 fail so working around the issue in rustc seems good to me.

[brson]

Just waiting for backport