From 0160f6af451bbf0f32ee0ba4d606990ff481bff4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 26 Mar 2019 17:29:53 -0700 Subject: [PATCH] Fix handling of `u32` between Rust and JS All numbers in WebAssembly are signed and then each operation on them may optionally have an unsigned version. This means that when we pass large signed numbers to JS they actually show up as large negative numbers even though JS numbers can faithfully represent the type. This is fixed by adding `>>>0` in a few locations in the generated bindings to coerce the JS value into an unsigned value. Closes #1388 --- crates/cli-support/src/descriptor.rs | 20 +++++++-- crates/cli-support/src/js/js2rust.rs | 10 +++-- crates/cli-support/src/js/rust2js.rs | 12 +++++- tests/wasm/math.js | 32 +++++++++++++++ tests/wasm/math.rs | 61 ++++++++++++++++++++++++++++ 5 files changed, 126 insertions(+), 9 deletions(-) diff --git a/crates/cli-support/src/descriptor.rs b/crates/cli-support/src/descriptor.rs index 3479a74d..09d7b332 100644 --- a/crates/cli-support/src/descriptor.rs +++ b/crates/cli-support/src/descriptor.rs @@ -99,6 +99,10 @@ pub enum VectorKind { Anyref, } +pub struct Number { + u32: bool, +} + impl Descriptor { pub fn decode(mut data: &[u32]) -> Descriptor { let descriptor = Descriptor::_decode(&mut data); @@ -149,18 +153,20 @@ impl Descriptor { } } - pub fn is_number(&self) -> bool { + /// Returns `Some` if this type is a number, and the returned `Number` type + /// can be accessed to learn more about what kind of number this is. + pub fn number(&self) -> Option { match *self { Descriptor::I8 | Descriptor::U8 | Descriptor::I16 | Descriptor::U16 | Descriptor::I32 - | Descriptor::U32 | Descriptor::F32 | Descriptor::F64 - | Descriptor::Enum { .. } => true, - _ => return false, + | Descriptor::Enum { .. } => Some(Number { u32: false }), + Descriptor::U32 => Some(Number { u32: true }), + _ => None, } } @@ -360,3 +366,9 @@ impl VectorKind { } } } + +impl Number { + pub fn is_u32(&self) -> bool { + self.u32 + } +} diff --git a/crates/cli-support/src/js/js2rust.rs b/crates/cli-support/src/js/js2rust.rs index 61a5020d..36f3fe08 100644 --- a/crates/cli-support/src/js/js2rust.rs +++ b/crates/cli-support/src/js/js2rust.rs @@ -384,7 +384,7 @@ impl<'a, 'b> Js2Rust<'a, 'b> { return Ok(self); } - if arg.is_number() { + if arg.number().is_some() { self.js_arguments.push((name.clone(), "number".to_string())); if self.cx.config.debug { @@ -681,9 +681,13 @@ impl<'a, 'b> Js2Rust<'a, 'b> { return Ok(self); } - if ty.is_number() { + if let Some(num) = ty.number() { self.ret_ty = "number".to_string(); - self.ret_expr = format!("return RET;"); + if num.is_u32() { + self.ret_expr = format!("return RET >>> 0;"); + } else { + self.ret_expr = format!("return RET;"); + } return Ok(self); } diff --git a/crates/cli-support/src/js/rust2js.rs b/crates/cli-support/src/js/rust2js.rs index b5d68ddf..8e706dec 100644 --- a/crates/cli-support/src/js/rust2js.rs +++ b/crates/cli-support/src/js/rust2js.rs @@ -309,8 +309,16 @@ impl<'a, 'b> Rust2Js<'a, 'b> { return Ok(()); } + if let Some(num) = arg.number() { + if num.is_u32() { + self.js_arguments.push(format!("{} >>> 0", abi)); + } else { + self.js_arguments.push(abi); + } + return Ok(()); + } + let invoc_arg = match *arg { - ref d if d.is_number() => abi, Descriptor::Boolean => format!("{} !== 0", abi), Descriptor::Char => format!("String.fromCodePoint({})", abi), _ => bail!( @@ -504,7 +512,7 @@ impl<'a, 'b> Rust2Js<'a, 'b> { return Ok(()); } - if ty.is_number() { + if ty.number().is_some() { self.ret_expr = "return JS;".to_string(); return Ok(()); } diff --git a/tests/wasm/math.js b/tests/wasm/math.js index c4865494..e3b51830 100644 --- a/tests/wasm/math.js +++ b/tests/wasm/math.js @@ -1,5 +1,37 @@ const wasm = require('wasm-bindgen-test.js'); +const assert = require('assert'); exports.js_auto_bind_math = () => { wasm.math(1.0, 2.0); }; + +exports.roundtrip = x => x; + +exports.test_js_roundtrip = () => { + assert.strictEqual(wasm.rust_roundtrip_i8(0), 0); + assert.strictEqual(wasm.rust_roundtrip_i8(0x80), -128); + assert.strictEqual(wasm.rust_roundtrip_i8(0x7f), 127); + + assert.strictEqual(wasm.rust_roundtrip_i16(0), 0); + assert.strictEqual(wasm.rust_roundtrip_i16(0x8000), -32768); + assert.strictEqual(wasm.rust_roundtrip_i16(0x7fff), 32767); + + assert.strictEqual(wasm.rust_roundtrip_i32(0), 0); + assert.strictEqual(wasm.rust_roundtrip_i32(0x80000000), -2147483648); + assert.strictEqual(wasm.rust_roundtrip_i32(0x7fffffff), 2147483647); + + assert.strictEqual(wasm.rust_roundtrip_u8(0), 0); + assert.strictEqual(wasm.rust_roundtrip_u8(0x80), 128); + assert.strictEqual(wasm.rust_roundtrip_u8(0x7f), 127); + assert.strictEqual(wasm.rust_roundtrip_u8(0xff), 255); + + assert.strictEqual(wasm.rust_roundtrip_u16(0), 0); + assert.strictEqual(wasm.rust_roundtrip_u16(0x8000), 32768); + assert.strictEqual(wasm.rust_roundtrip_u16(0x7fff), 32767); + assert.strictEqual(wasm.rust_roundtrip_u16(0xffff), 65535); + + assert.strictEqual(wasm.rust_roundtrip_u32(0), 0); + assert.strictEqual(wasm.rust_roundtrip_u32(0x80000000), 2147483648); + assert.strictEqual(wasm.rust_roundtrip_u32(0x7fffffff), 2147483647); + assert.strictEqual(wasm.rust_roundtrip_u32(0xffffffff), 4294967295); +}; diff --git a/tests/wasm/math.rs b/tests/wasm/math.rs index 90d3fa85..ba5b799f 100644 --- a/tests/wasm/math.rs +++ b/tests/wasm/math.rs @@ -4,6 +4,26 @@ use wasm_bindgen_test::*; #[wasm_bindgen(module = "tests/wasm/math.js")] extern "C" { fn js_auto_bind_math(); + + // There's an identity function called `roundtrip` in the module and we bind + // that one function with multiple different signatures here. Note that the + // return value is always `f64` to faithfully capture what was sent to JS + // (what we're interested in) because all JS numbers fit in `f64` anyway. + // This is testing what happens when we pass numbers to JS and what it sees. + #[wasm_bindgen(js_name = roundtrip)] + fn roundtrip_i8(a: i8) -> f64; + #[wasm_bindgen(js_name = roundtrip)] + fn roundtrip_i16(a: i16) -> f64; + #[wasm_bindgen(js_name = roundtrip)] + fn roundtrip_i32(a: i32) -> f64; + #[wasm_bindgen(js_name = roundtrip)] + fn roundtrip_u8(a: u8) -> f64; + #[wasm_bindgen(js_name = roundtrip)] + fn roundtrip_u16(a: u16) -> f64; + #[wasm_bindgen(js_name = roundtrip)] + fn roundtrip_u32(a: u32) -> f64; + + fn test_js_roundtrip(); } #[wasm_bindgen] @@ -65,3 +85,44 @@ pub fn math(a: f32, b: f64) -> f64 { fn auto_bind_math() { js_auto_bind_math(); } + +macro_rules! t_roundtrip { + ($f:ident($e:expr)) => (assert_eq!($f($e), $e as f64)) +} + +#[wasm_bindgen_test] +fn limits_correct() { + t_roundtrip!(roundtrip_i8(i8::min_value())); + t_roundtrip!(roundtrip_i8(0)); + t_roundtrip!(roundtrip_i8(i8::max_value())); + t_roundtrip!(roundtrip_i16(i16::min_value())); + t_roundtrip!(roundtrip_i16(0)); + t_roundtrip!(roundtrip_i16(i16::max_value())); + t_roundtrip!(roundtrip_i32(i32::min_value())); + t_roundtrip!(roundtrip_i32(0)); + t_roundtrip!(roundtrip_i32(i32::max_value())); + t_roundtrip!(roundtrip_u8(u8::min_value())); + t_roundtrip!(roundtrip_u8(0)); + t_roundtrip!(roundtrip_u8(u8::max_value())); + t_roundtrip!(roundtrip_u16(u16::min_value())); + t_roundtrip!(roundtrip_u16(0)); + t_roundtrip!(roundtrip_u16(u16::max_value())); + t_roundtrip!(roundtrip_u32(u32::min_value())); + t_roundtrip!(roundtrip_u32(0)); + t_roundtrip!(roundtrip_u32(u32::max_value())); + + test_js_roundtrip(); + + #[wasm_bindgen] + pub fn rust_roundtrip_i8(a: i8) -> i8 { a } + #[wasm_bindgen] + pub fn rust_roundtrip_i16(a: i16) -> i16 { a } + #[wasm_bindgen] + pub fn rust_roundtrip_i32(a: i32) -> i32 { a } + #[wasm_bindgen] + pub fn rust_roundtrip_u8(a: u8) -> u8 { a } + #[wasm_bindgen] + pub fn rust_roundtrip_u16(a: u16) -> u16 { a } + #[wasm_bindgen] + pub fn rust_roundtrip_u32(a: u32) -> u32 { a } +}