asm2wasm frem miscompilation leads to wasm validation error

[pepyakin]

Sources:

• frem float miscompilation emscripten-core/emscripten#5399
• f32::rem fails on wasm32-unknown-emscripten rust-lang/rust#42629

invoking asm2wasm on following input:

function () {
  "almost asm";

  var STACKTOP = 0, STACK_MAX = 0;
  var Math_fround=global.Math.fround;

  function _foo($0,$1) {
    $0 = Math_fround($0);
    $1 = Math_fround($1);
    var $2 = Math_fround(0), label = 0, sp = 0;
    sp = STACKTOP;
    $2 = Math_fround($0 % $1);
    return (Math_fround($2));
  }
  return { _foo: _foo };
}

will lead to wasm validation error:

[wasm-validator error in function $_foo] 3 != 4: call param types must match, on
[f64] (call $f64-rem
 [f32] (get_local $0)
 [f32] (get_local $1)
)
(on argument 0)
[wasm-validator error in function $_foo] 3 != 4: call param types must match, on
[f64] (call $f64-rem
 [f32] (get_local $0)
 [f32] (get_local $1)
)
(on argument 1)
(module
 (type $FUNCSIG$ddd (func (param f64 f64) (result f64)))
 (import "asm2wasm" "f64-rem" (func $f64-rem (param f64 f64) (result f64)))
 (import "env" "memory" (memory $0 256 256))
 (import "env" "table" (table 0 0 anyfunc))
 (import "env" "memoryBase" (global $memoryBase i32))
 (import "env" "tableBase" (global $tableBase i32))
 (global $STACKTOP (mut i32) (i32.const 0))
 (global $STACK_MAX (mut i32) (i32.const 0))
 (export "_foo" (func $legalstub$_foo))
 (func $_foo (param $$0 f32) (param $$1 f32) (result f32)
  (local $$2 f32)
  (local $label i32)
  (local $sp i32)
  (set_local $sp
   (get_global $STACKTOP)
  )
  (set_local $$2
   (f32.demote/f64
    (call $f64-rem
     (get_local $$0)
     (get_local $$1)
    )
   )
  )
  (return
   (get_local $$2)
  )
 )
 (func $legalstub$_foo (param $0 f64) (param $1 f64) (result f64)
  (f64.promote/f32
   (call $_foo
    (f32.demote/f64
     (get_local $0)
    )
    (f32.demote/f64
     (get_local $1)
    )
   )
  )
 )
)

[CryZe]

Looks like neither f32.rem nor f64.rem are WebAssembly instructions, which means binaryen will try to replace it with a f64-rem function that it provides itself. Seems like there's no f32-rem. This seems to be the code that does the replacement:

binaryen/src/asm2wasm.h

Lines 1822 to 1838 in e865f2f

	// WebAssembly does not have floating-point remainder, we have to emit a call to a special import of ours
	CallImport *call = allocator.alloc<CallImport>();
	call->target = F64_REM;
	call->operands.push_back(ret->left);
	call->operands.push_back(ret->right);
	call->type = f64;
	static bool addedImport = false;
	if (!addedImport) {
	addedImport = true;
	auto import = new Import; // f64-rem = asm2wasm.f64-rem;
	import->name = F64_REM;
	import->module = ASM2WASM;
	import->base = F64_REM;
	import->functionType = ensureFunctionType("ddd", &wasm)->name;
	import->kind = ExternalKind::Function;
	wasm.addImport(import);
	}

Seems like it just needs a check that promotes the f32 arguments to f64, which seems to be done with:

curr->operands[i] = parent->builder.makeUnary(PromoteFloat32, curr->operands[i]);

Interestingly, this is part of visitCallImport which seems to be the function that fixes calls to imports. So I feel like that should've been fixing the arguments, but it maybe didn't get called?!

https://github.com/WebAssembly/binaryen/blob/master/src/asm2wasm.h#L1350

[kripken]

Yeah, that's it, thanks. I think this was missed since clang doesn't end up emitting it, but looks like rust can. Fix is in #1106.

[kripken]

Fixed by that PR.