GetFirefly / firefly

An alternative BEAM implementation, designed for WebAssembly
Apache License 2.0
3.61k stars 104 forks source link

Dividend and divisor corruption when using `//2` with integers #650

Closed KronicDeth closed 4 years ago

KronicDeth commented 4 years ago

Using using //2, which is the divide that converts all arguments to floats, the dividend contains the divisor's value and the divisor is None.

init.erl

-module(init).
-export([start/0]).

start() ->
  test(1, 2).

test(Dividend, Divisor) ->
  test:caught(fun () ->
    Dividend / Divisor
  end).

Output

With debugging

---- lib::erlang::divide_2::with_number_dividend_without_zero_number_divisor_returns_float stdout ----
thread 'lib::erlang::divide_2::with_number_dividend_without_zero_number_divisor_returns_float' panicked at 'assertion failed: `(left == right)`
  left: `"dividend = 2; divisor = NoneValue { backtrace: <disabled> }\n"`,
 right: `"4"`: 
Commands:
cd /Users/luke.imhoff/github/lumen/lumen/native_implemented/otp
"tests/internal/lib/erlang/divide_2/with_number_dividend_without_zero_number_divisor_returns_float/bin/with_number_dividend_without_zero_number_divisor_returns_float"
stdout: dividend = 2; divisor = NoneValue { backtrace: <disabled> }

Patches

Index: native_implemented/otp/src/erlang/divide_2.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- native_implemented/otp/src/erlang/divide_2.rs   (revision 9ff743ea709e912175911b248e654b525e7108b7)
+++ native_implemented/otp/src/erlang/divide_2.rs   (date 1601485955469)
@@ -1,6 +1,3 @@
-#[cfg(all(not(target_arch = "wasm32"), test))]
-mod test;
-
 use std::convert::TryInto;

 use anyhow::*;
@@ -14,6 +11,7 @@
 /// `float`.
 #[native_implemented::function(erlang:/ /2)]
 pub fn result(process: &Process, dividend: Term, divisor: Term) -> exception::Result<Term> {
+    println!("dividend = {}; divisor = {}", dividend, divisor);
     let dividend_f64: f64 = dividend.try_into().map_err(|_| {
         badarith(
             Trace::capture(),
Index: runtimes/core/src/builtins.rs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- runtimes/core/src/builtins.rs   (revision 9ff743ea709e912175911b248e654b525e7108b7)
+++ runtimes/core/src/builtins.rs   (date 1601482885538)
@@ -23,6 +23,8 @@
     fn erlang_bxor_2(left: Term, right: Term) -> Term;
     #[link_name = "erlang:div/2"]
     fn erlang_div_2(dividend: Term, divisor: Term) -> Term;
+    #[link_name = "erlang://2"]
+    fn erlang_divide_2(dividend: Term, divisor: Term) -> Term;
 }

 #[export_name = "__lumen_builtin_bigint_from_cstr"]
@@ -224,7 +226,11 @@

 math_builtin!("__lumen_builtin_math.sub", builtin_math_sub, Sub, sub);
 math_builtin!("__lumen_builtin_math.mul", builtin_math_mul, Mul, mul);
-math_builtin!("__lumen_builtin_math.fdiv", builtin_math_fdiv, Div, div);
+
+#[export_name = "__lumen_builtin_math.fdiv"]
+pub extern "C" fn builtin_math_fdiv(dividend: Term, divisor: Term) -> Term {
+    unsafe { erlang_divide_2(dividend, divisor) }
+}

 #[export_name = "__lumen_builtin_math.div"]
 pub extern "C" fn builtin_math_div(dividend: Term, divisor: Term) -> Term {
KronicDeth commented 4 years ago

//2 works fine with floats, so it is something to do with using integers with it.

bitwalker commented 4 years ago

Fixed in #582