moonbitlang / core

MoonBit's Core library
https://moonbitlang.com/
Apache License 2.0
636 stars 79 forks source link

Possible compiler bug - unreachable code for wasm and wasm-gc #1219

Closed gmlewis closed 1 day ago

gmlewis commented 1 week ago

With this release of this repo: https://github.com/gmlewis/moonbit-io/releases/tag/v0.6.0 running the following works:

$ moon version --all
moon 0.1.20241111 (e6d64e0 2024-11-11) ~/.moon/bin/moon
moonc v0.1.20241111+dc2407357 ~/.moon/bin/moonc
moonrun 0.1.20241111 (e6d64e0 2024-11-11) ~/.moon/bin/moonrun
$ moon test --target js
Total tests: 38, passed: 38, failed: 0.

However, all of the other targets fail with "unreachable" or "panic":

peter-jerry-ye commented 1 week ago

Minimal code to reproduce:

test "panic" {
  let empty : Array[Byte] = []
  let array : Array[Byte] = [1, 2, 3]
  empty.unsafe_blit(0, array, 0, 3) // wasm panic but not js
}

test "size" {
  let empty : Array[Byte] = Array::new(capacity=3)
  let array : Array[Byte] = [1, 2, 3]
  empty.unsafe_blit(0, array, 0, 3)
  inspect!(empty, content="[b'\\x01', b'\\x02', b'\\x03']") // js has content but wasm is empty
}

@hackwaly @Yu-zh

peter-jerry-ye commented 1 week ago

So the panic is expected: unsafe_blit is expected to panic when the length is not enough. As the name suggest, it is unsafe so no extra check would be done.

The problem here is that the compiler should not use intrinsic instructions during debug/test mode, which we will fix it.

gmlewis commented 1 week ago

So if I'm understanding correctly, then I have a bug in my code since I should not be blitting into an empty array.

Thank you, @peter-jerry-ye - I'll investigate.

gmlewis commented 1 week ago

Ah! So you are saying that the following is not correct:

pub fn write(self : Buffer, buf : Slice[Byte]) -> (Int, IOError?) {
  let cap = buf.length() + self.b.length()
  let b = Array::new(capacity=cap)
  Array::unsafe_blit(b, 0, self.b, 0, self.b.length())
  Array::unsafe_blit(b, self.b.length(), buf.buf, buf.start, buf.len)
  self.b = b
  (buf.length(), None)
}

I'll try using "push" instead of "unsafe_blit" and see if that fixes the problem.

gmlewis commented 1 week ago

Thank you for your help, @peter-jerry-ye ! I've now released version v0.8.0 which fixes the unsafe_blit usage: https://github.com/gmlewis/moonbit-io/releases/tag/v0.8.0

$ moon version --all
moon 0.1.20241111 (e6d64e0 2024-11-11) ~/.moon/bin/moon
moonc v0.1.20241111+dc2407357 ~/.moon/bin/moonc
moonrun 0.1.20241111 (e6d64e0 2024-11-11) ~/.moon/bin/moonrun
$ moon test --target all
Total tests: 39, passed: 39, failed: 0. [wasm-gc]
Total tests: 39, passed: 39, failed: 0. [wasm]
Total tests: 39, passed: 39, failed: 0. [js]
$ moon test --target native
Total tests: 26, passed: 26, failed: 0.
gmlewis commented 1 week ago

I'm not sure if this is related, but target wasm is failing for this repo: https://github.com/gmlewis/moonbit-gzip/releases/tag/v0.16.0

but all other targets pass:

$ moon version --all
moon 0.1.20241115 (67c2b06 2024-11-15) ~/.moon/bin/moon
moonc v0.1.20241115+351cfb074 ~/.moon/bin/moonc
moonrun 0.1.20241115 (67c2b06 2024-11-15) ~/.moon/bin/moonrun
$ moon test --target wasm
test gmlewis/gzip/gunzip_test.mbt::decompressor by resetting the reader failed: RangeError: Maximum call stack size exceeded
    at @moonbitlang/core/builtin.Array::length|Byte| (wasm://wasm/00193d2e:wasm-function[134]:0x209a)
    at @gmlewis/io.Buffer::read_byte (wasm://wasm/00193d2e:wasm-function[346]:0x7c02)
    at @gmlewis/flate.Reader::read_byte|@gmlewis/io.Buffer|@gmlewis/flate.Reader (wasm://wasm/00193d2e:wasm-function[525]:0x16d30)
    at @gmlewis/flate.Reader::read_byte|@gmlewis/io.Buffer|@gmlewis/flate.Reader.dyncall (wasm://wasm/00193d2e:wasm-function[57]:0x134b)
    at @gmlewis/flate.Decompressor::huff_sym (wasm://wasm/00193d2e:wasm-function[420]:0xe3e5)
    at @gmlewis/flate.Decompressor::read_literal (wasm://wasm/00193d2e:wasm-function[434]:0xf937)
    at @gmlewis/flate.Decompressor::copy_history (wasm://wasm/00193d2e:wasm-function[433]:0xf8f7)
    at @gmlewis/flate.Decompressor::read_literal (wasm://wasm/00193d2e:wasm-function[434]:0x10227)
    at @gmlewis/flate.Decompressor::read_literal (wasm://wasm/00193d2e:wasm-function[434]:0xfab1)
    at @gmlewis/flate.Decompressor::read_literal (wasm://wasm/00193d2e:wasm-function[434]:0xfab1)
Total tests: 7, passed: 6, failed: 1.
gmlewis commented 1 week ago

Here's one other unusual issue I found for target native: https://github.com/gmlewis/moonbit-image/releases/tag/v0.5.0

$ moon version --all
moon 0.1.20241115 (67c2b06 2024-11-15) ~/.moon/bin/moon
moonc v0.1.20241115+351cfb074 ~/.moon/bin/moonc
moonrun 0.1.20241115 (67c2b06 2024-11-15) ~/.moon/bin/moonrun
$ moon test --target native
[======================================--] 29/30 done, 1/1 running
link-core: gmlewis/image/png_blackbox_test
failed to execute '/Users/glenn/src/github.com/gmlewis/moonbit-image/target/native/debug/test/png/png.blackbox_test.exe /Users/glenn/src/github.com/gmlewis/moonbit-image/target/native/debug/test/png/png.blackbox_test.exe'

Caused by:
    No such file or directory (os error 2)

Total tests: 26, passed: 16, failed: 10.
hackwaly commented 1 week ago

/cc @Guest0x0 @lijunchen

peter-jerry-ye commented 1 week ago

I'm not sure if this is related, but target wasm is failing for this repo: https://github.com/gmlewis/moonbit-gzip/releases/tag/v0.16.0

but all other targets pass:

The loopify optimization is off for debug mode. The test will pass if you test it under release mode: moon test --target wasm -p gmlewis/gzip -f gunzip_test.mbt -i 0 --release

@Guest0x0 Consider using tail-call proposal for wasm?

peter-jerry-ye commented 1 week ago

Here's one other unusual issue I found for target native: https://github.com/gmlewis/moonbit-image/releases/tag/v0.5.0

I found a bug with the code:

struct T {}

fn T::new() -> T! {
  if false {
    fail!("Error")
  }
  T::{  }
}

pub(open) trait Any {}

struct Test {
  f : () -> Any!
}

fn main {
  let _ : Test = { f: fn() { T::new!() } } // To bypass, write `T::new!() as Image` explicitly

}

@Yu-zh

But I didn't reproduce the one you described.

peter-jerry-ye commented 1 day ago

All the reproducible bugs are fixed. Feel free to reopen if any one still persists.