Some mirunsafeck and thirunsafeck tests failing with weird assert failure

[antoyo]

---- [ui] ui/union/union-overwrite.rs#mirunsafeck stdout ----

error in revision `mirunsafeck`: test run failed!
status: signal: 6 (core dumped)
command: "/Projets/rustc_codegen_gccjit/rust/build/x86_64-unknown-linux-gnu/test/ui/union/union-overwrite.mirunsafeck/a"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `57022`,
 right: `57022`', /Projets/rustc_codegen_gccjit/rust/src/test/ui/union/union-overwrite.rs:34:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

That happens only when compiled in release mode, not in debug mode.

Example:

union U {
    a: (u8, u8),
    b: u16,
}

union W {
    a: u32,
    b: f32,
}

fn main() {
    unsafe {
        let mut u = U { a: (1, 1) };
        assert_eq!(u.b, (1 << 8) + 1);
        u.b = (2 << 8) + 2;
        assert_eq!(u.a, (2, 2));

        let mut w = W { a: 0b0_11111111_00000000000000000000000 };
        assert_eq!(w.b, f32::INFINITY);
        w.b = f32::NEG_INFINITY;
        assert_eq!(w.a, 0b1_11111111_00000000000000000000000);
    }
}

gives:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `(2, 2)`,
 right: `(2, 2)`', src/main.rs:16:9

[sapir]

I ran this with CG_GCCJIT_DUMP_EVERYTHING=1 CG_GCCJIT_KEEP_INTERMEDIATES=1, it seems one of the optimization passes is deleting the assert condition.

before (full file with more debug info here: fake.c.036t.ealias.txt)

  <bb 2> :
start:
  MEM[(unsigned short *)&stack_var_1] = 57054;
  MEM[(unsigned char *)&stack_var_1] = 190;
  stack_var_2_7 = &stack_var_1;
  # DEBUG stack_var_2 => stack_var_2_7
  _6 = (unsigned long) &global_0_mytest_8f3c4634_cgu_2;
  stack_var_2$field1_TODO_9 = &global_0_mytest_8f3c4634_cgu_2;
  # DEBUG stack_var_2$field1_TODO => stack_var_2$field1_TODO_9
  loadedValue1_116 = stack_var_2_7;
  # DEBUG loadedValue1 => loadedValue1_116
  # DEBUG stack_var_1 => loadedValue1_116
  loadedValue2_118 = stack_var_2$field1_TODO_9;
  # DEBUG loadedValue2 => loadedValue2_118
  # DEBUG stack_var_2 => loadedValue2_118
  loadedValue3_120 = *loadedValue1_116;
  # DEBUG loadedValue3 => loadedValue3_120
  if (loadedValue3_120 != 57022)
    goto <bb 4>; [INV]
  else
    goto <bb 3>; [INV]

and after: (full file here: fake.c.037t.fre1.txt)

  <bb 2> :
start:
  MEM[(unsigned short *)&stack_var_1] = 57054;
  MEM[(unsigned char *)&stack_var_1] = 190;
  # DEBUG stack_var_2 => &stack_var_1
  _6 = (unsigned long) &global_0_mytest_8f3c4634_cgu_2;
  # DEBUG stack_var_2$field1_TODO => &global_0_mytest_8f3c4634_cgu_2
  # DEBUG loadedValue1 => &stack_var_1
  # DEBUG stack_var_1 => &stack_var_1
  # DEBUG loadedValue2 => &global_0_mytest_8f3c4634_cgu_2
  # DEBUG stack_var_2 => &global_0_mytest_8f3c4634_cgu_2
  # DEBUG loadedValue3 => 57054
  # DEBUG bb1 => NULL
  # DEBUG stack_var_3 => 0
  MEM[(struct struct * *)&stack_var_4] = 0B;
  _31 = (unsigned long) &global_2_mytest_8f3c4634_cgu_2;
  _RINvNtCs2P6WAD9d4YR_4core9panicking13assert_failedttECs1k3PfII5Xk_6mytest (0, &stack_var_1, &global_0_mytest_8f3c4634_cgu_2, &stack_var_4, &global_2_mytest_8f3c4634_cgu_2);
  # DEBUG dummyValueThatShouldNeverBeUsed => 0
  __builtin_unreachable ();

[antoyo]

I can't remember all the details of my tests, but it seems something was needed in order to reproduce the issue outside the UI tests (maybe it was to compile with(out?) optimizations?).

[sapir]

The issue did happen for me, the assert condition gets deleted and then the assert panic gets called even though the two values are equal.

[sapir]

The issue happens even without the union; the following code reproduces the issue on little-endian targets. (assert! didn't trigger the bug, so I expanded assert_eq! to try to figure out what's going on.)

fn main() {
    unsafe {
        let mut u = 0xDE_DE_u16;
        *(&mut u as *mut _ as *mut u8) = 0xBE;
        match (&u, 0xDE_BE_u16) {
            (a, b) => {
                if !(*a == b) {
                    println!("how did this happen, a={}, b={}", *a, b);
                }
            }
        }
    }
}

(edit: removed the reference from the right-hand side of the match)

[sapir]

Maybe minimized it a bit more:

fn main() {
    unsafe {
        let mut u = 0xDE_DE_u16;
        *(&mut u as *mut _ as *mut u8) = 0xBE;
        let a = &u;
        let b = *a;
        println!("a={:?}, *a={:?}, b={:?}", a, *a, b);
    }
}

prints:

a=57022, *a=57022, b=57054

[antoyo]

I'd be curious to see the MIR and the GIMPLE for that.
Do you have to compile with optimizations enabled for this issue to happen?

[sapir]

I'm actually not so sure the last example is as useful for minimization because println! is a lot noisier than the if, but at least it's an example that's a bit different.

How do I get the MIR?

Do you have to compile with optimizations enabled for this issue to happen?

Last I checked, yes.

[antoyo]

With -Z dump-mir.

[bjorn3]

Does -fno-strict-aliasing fix this?

[antoyo]

@sapir Could you try compiling with -C llvm-args=-fno-strict-aliasing to check bjorn3's suggestion, please?

[sapir]

I'd be curious to see the MIR and the GIMPLE for that.

mytest.main.005-024.PreCodegen.after.mir.txt

fake.c.006t.gimple.txt

@sapir Could you try compiling with -C llvm-args=-fno-strict-aliasing to check bjorn3's suggestion, please?

yes it works!

[antoyo]

Seems like we should cast pointers using a union.

[sapir]

Wouldn't that have the same effect as -fno-strict-aliasing?

[antoyo]

No, that flag disable the optimization, while using a union would make it understand that those pointer aliases (at least, that's my understanding; I haven't had the time to read that article).

[bjorn3]

As far as I understand casting through unions is not enough. If a function takes two pointers of different types, strict aliasing will assume that they don't alias AFAIK, but rust allows this just fine. -fno-strict-aliasing will allow the aliasing, by telling all relevant optimizations that strict aliasing shouldn't be used.

[antoyo]

You're right. Rust relies on LLVM not doing TBAA.

[antoyo]

Thanks @sapir for your investigation and @bjorn3 for your help!