Skip to content

Commit

Permalink
Fix handling of u32 between Rust and JS
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alexcrichton committed Mar 27, 2019
1 parent e3aabcb commit f4f6061
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 9 deletions.
21 changes: 17 additions & 4 deletions crates/cli-support/src/descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -149,18 +153,21 @@ impl Descriptor {
}
}

pub fn is_number(&self) -> bool {
/// Returns `Some` if this type is a number, and the boolean returned
/// represents whether it's `u32` and needs the sign reset when it goes
/// into JS because JS is otherwise receiving a `i32`
pub fn number(&self) -> Option<Number> {
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,
}
}

Expand Down Expand Up @@ -360,3 +367,9 @@ impl VectorKind {
}
}
}

impl Number {
pub fn is_u32(&self) -> bool {
self.u32
}
}
10 changes: 7 additions & 3 deletions crates/cli-support/src/js/js2rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down
12 changes: 10 additions & 2 deletions crates/cli-support/src/js/rust2js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down Expand Up @@ -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(());
}
Expand Down
32 changes: 32 additions & 0 deletions tests/wasm/math.js
Original file line number Diff line number Diff line change
@@ -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);
};
61 changes: 61 additions & 0 deletions tests/wasm/math.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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 }
}

0 comments on commit f4f6061

Please sign in to comment.