binaryen icon indicating copy to clipboard operation
binaryen copied to clipboard

Verification error after function inlining

Open igoriakovlev opened this issue 2 years ago • 5 comments

Hi, It seems binaryen (7d5d24f) creates invalid blocks after function body inlining.

Steps to reproduce:

  1. Download wasm file from here and unpack index.wasm file
  2. Run wasm-opt --enable-gc --enable-reference-types --enable-exception-handling --enable-bulk-memory --enable-nontrapping-float-to-int -O4 index.wasm -o indexOpt.wasm
  3. Get error: [wasm-validator error in function kotlin.wasm.internal.jsToKotlinAnyAdapter] unexpected false: non-final block elements returning a value must be drop()ed (binaryen's autodrop option might help you), on (block $__inlined_func$kotlin.wasm.internal.externRefToAny$53 (result (ref null $kotlin.Any)) (block $label$1 (result anyref) (br $__inlined_func$kotlin.wasm.internal.externRefToAny$53 (br_on_cast_fail $label$1 $kotlin.Any (extern.internalize (local.get $0) ) ) ) ) (call $kotlin.wasm.internal.jsToKotlinAnyAdapter (call $kotlin.wasm.internal.jsCheckIsNullOrUndefinedAdapter (call $fimport$8 (local.get $0) (extern.externalize (struct.new $kotlin.wasm.internal.JsExternalBox (global.get $kotlin.wasm.internal.JsExternalBox.vtable) (ref.null none) (i32.const 888) (i32.const 0) (local.get $0) ) ) ) ) ) ) (on index 0: 0x13081ce08 ), type: anyref Fatal: error after opts

It seems br_on_cast_fail return value is not dropped (the original wasm file contains this drop)

With -O3 everything works fine. V8 and JsShell works fine.

igoriakovlev avatar May 12 '23 16:05 igoriakovlev

This is a limitation of "flat" mode, which is used in -O4. In that mode we assume that blocks do not have return values, which is much flatter and simpler, and we need to be in that form to do some control flow optimizations that we do in -O4. The problem that is hit in this wasm module is that it uses br_on_cast_fail, and that instruction always sends a value to the target block - it cannot be removed, unlike br and switch.

We could add logic to lower br_on_cast, br_on_cast_fail to simpler sequences (br_on_cast => if cast then br else return value). However, that would make them less efficient, so it's not clear to me if it's worth it.

As a workaround for now, you can use -O3 or avoid using those branch instructions.

How much of a benefit do you get from -O4 vs -O3? If it's large then that could motivate lowering br_on_cast*.

kripken avatar May 12 '23 17:05 kripken