wasm32-unknown-unknown: Incorrect codegen

[BonsaiDen]

I've tested this with current nightly, beta and stable as well as nightlies all the way back to october (haven't tried anything before that).

Expected Output: a: false / t.a: false / both check for: (2 & 1) == 1 (Correct)
Actual Output: a: false / t.a: true / both check for: (2 & 1) == 1 (Incorrect)

Note: You can plug the code below into the https://github.com/rustwasm/rust-webpack-template for running it.

extern crate wasm_bindgen;
use wasm_bindgen::prelude::*;

#[derive(Debug)]
struct Test {
    a: bool,
    b: bool
}

impl Test {
    fn foo(byte: u8) -> String {

        // 1. This must be infront of the struct initializer - It will evaluate correctly to "false"
        let a = (byte & 1) == 1;

        let t = Test {
            a: (byte & 1) == 1, // 2. This must come before "b" - It will incorrectly evaluate to "true"
            b: (byte & 2) == 2, // 3. This must be evaluated inside the initializer
        };

        // 4. This must be called inside the function
        format!("a: {} / t.a: {} / both check for: (2 & 1) == 1", a, t.a)

    }

}

#[wasm_bindgen]
extern "C" {
    #[wasm_bindgen(js_namespace = console)]
    fn log(s: &str);
}

#[wasm_bindgen]
pub fn run() {
    log(&Test::foo(2));
}

For some reason the two different (byte & 1) == 1 expression generate different results.

Note: Inlining everything by replacing byte with 2 produces the correct output.
Also Note: In my real program there are more strange things that happen after this like (for i in vector not actually iterating over the contents of the vector) but I was unable to reduce those to a test case :(

For comparison, here's the same code (just without the WASM stuff) that produces the expected output a: false / t.a: false / both check for: (2 & 1) == 1 when invoked via a simple cargo run:

#[derive(Debug)]
struct Test {
    a: bool,
    b: bool
}

impl Test {

    fn foo(byte: u8) -> String {

        // 1. This must be infront of the struct initializer - It will evaluate correctly to "false"
        let a = (byte & 1) == 1;

        let t = Test {
            a: (byte & 1) == 1, // 2. This must come before "b" - It will incorrectly evaluate to "true"
            b: (byte & 2) == 2, // 3. This must be evaluated inside the initializer
        };

        // 4. This must be called inside the function
        format!("a: {} / t.a: {} / both check for: (2 & 1) == 1", a, t.a)

    }

}

pub fn main() {
    println!("{}", Test::foo(2));
}

[nikic]

This looks like a FastISel bug.

[nikic]

Somewhat reduced test case:

define void @test(i8 %byte) {
  %t = alloca { i8, i8 }, align 1
  %x4 = and i8 %byte, 1
  %x5 = icmp eq i8 %x4, 1
  %x6 = and i8 %byte, 2
  %x7 = icmp eq i8 %x6, 2
  %x8 = bitcast { i8, i8 }* %t to i8*
  %x9 = zext i1 %x5 to i8
  store i8 %x9, i8* %x8, align 1
  %x10 = getelementptr inbounds { i8, i8 }, { i8, i8 }* %t, i32 0, i32 1
  %x11 = zext i1 %x7 to i8
  store i8 %x11, i8* %x10, align 1
  ret void
}

Results in (MachineInstr, because WebAssembly is unintelligible to me):

bb.0 (%ir-block.0):
  liveins: $arguments
  %0:i32 = ARGUMENT_i32 0, implicit $arguments
  %10:i32 = CONST_I32 2, implicit-def $arguments
  %5:i32 = COPY %0:i32
  %14:i32 = CONST_I32 2, implicit-def dead $arguments
  %7:i32 = AND_I32 %0:i32, killed %14:i32, implicit-def dead $arguments
  %8:i32 = CONST_I32 255, implicit-def $arguments
  %9:i32 = AND_I32 %7:i32, %8:i32, implicit-def $arguments
  %11:i32 = CONST_I32 255, implicit-def $arguments
  %12:i32 = AND_I32 %10:i32, %11:i32, implicit-def $arguments
  %13:i32 = EQ_I32 %9:i32, %12:i32, implicit-def $arguments
  %6:i32 = COPY %5:i32
  STORE8_I32 0, 0, %stack.0.t, %6:i32, implicit-def $arguments :: (store 1 into %ir.x8)
  %3:i32 = COPY %13:i32
  STORE8_I32 0, 1, %stack.0.t, %3:i32, implicit-def $arguments :: (store 1 into %ir.x10)
  RETURN_VOID implicit-def $arguments

The first store effectively stores a copy of %0, which is clearly wrong.

[BonsaiDen]

I was also able to reduce the rust test case further, having only 3 members does not cause the bug.

extern crate wasm_bindgen;
use wasm_bindgen::prelude::*;

#[derive(Debug)]
struct Test {
    a: bool,
    b: bool,
    c: bool,
    d: bool, // Removing "d" generates the correct code
}

impl Test {
    fn from_byte(byte: u8) -> Test {
        Test {
            a: (byte & 1) == 1,
            b: (byte & 2) == 2,
            c: (byte & 4) == 4,
            d: (byte & 8) == 8,
        }
    }
}

#[wasm_bindgen]
extern "C" {
    #[wasm_bindgen(js_namespace = console)]
    fn log(s: &str);
}

#[wasm_bindgen]
pub fn run() {
    let t = Test::from_byte(2);
    log(&format!("{:?}", t));
}

I'm also working on a test case showing how a for i in vec.into_iter() body gets skipped :)

[BonsaiDen]

Okay here's the test case for the iterator issue:

extern crate wasm_bindgen;
use wasm_bindgen::prelude::*;

#[derive(Debug)]
struct Test {
    a: bool,
    b: bool,
    c: bool,
    d: bool, // Removing "d" generates the correct code
}

impl Test {
    fn from_byte(byte: u8) -> Test {
        Test {
            a: (byte & 1) == 1,
            b: (byte & 2) == 2,
            c: (byte & 4) == 4,
            d: (byte & 8) == 8,
        }
    }
}


#[wasm_bindgen]
extern "C" {
    #[wasm_bindgen(js_namespace = console)]
    fn log(s: &str);
}

#[wasm_bindgen]
pub fn run() {

    let list = vec![Test::from_byte(2)];

    log("&Iterator Start");
    for item in &list {
        // This gets printed as "true"
        log(&format!("{}", item.a))
    }
    log("&Iterator End");

    log("Into Iterator Start");
    for item in list {
        // This gets skipped completely
        log(&format!("{}", item.a))
    }
    log("Into Iterator End");

}

This generates the following (incorrect) output, basically skipping the body of the for item in list completely.

&Iterator Start
true
&Iterator End
Into Iterator Start
Into Iterator End

Removing d from the Test struct generates the correct code / output:

&Iterator Start
false
&Iterator End
Into Iterator Start
false
Into Iterator End

[nikic]

I've reported https://bugs.llvm.org/show_bug.cgi?id=40172 upstream.

[tlively]

Hi! I just wanted to report back that a fix is in the works here: https://reviews.llvm.org/D56428.

[nikic]

Fix landed upstream (https://reviews.llvm.org/rL351127).

@cuviper Do you plan to rebase the monorepo fork another time? If not we can just cherry-pick this later.

[cuviper]

@nikic Yes, I plan to rebase once more when 8.x is branched tomorrow, then I'll open a rust PR.

[nikic]

The LLVM update has landed and includes this fix. Would be great if you can confirm that the original problem is fixed on current nightly.

[BonsaiDen]

@nikic Problem is indeed fixed on 1.34.0-nightly 👏

[nikic]

Great, thanks!