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 d4914ea
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 10 deletions.
13 changes: 8 additions & 5 deletions crates/cli-support/src/descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,18 +149,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<bool> {
match *self {
Descriptor::I8
| Descriptor::U8
| Descriptor::I16
| Descriptor::U16
| Descriptor::I32
| Descriptor::U32
| Descriptor::F32
| Descriptor::F64
| Descriptor::Enum { .. } => true,
_ => return false,
| Descriptor::F64 => Some(false),
Descriptor::U32 => Some(true),
| Descriptor::Enum { .. } => Some(false),
_ => None,
}
}

Expand Down
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(is_u32) = ty.number() {
self.ret_ty = "number".to_string();
self.ret_expr = format!("return RET;");
if 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(is_u32) = arg.number() {
if 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);
};
55 changes: 55 additions & 0 deletions tests/wasm/math.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,20 @@ use wasm_bindgen_test::*;
#[wasm_bindgen(module = "tests/wasm/math.js")]
extern "C" {
fn js_auto_bind_math();

#[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 +79,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 d4914ea

Please sign in to comment.