Merge pull request #1670 from fitzgen/check-for-use-after-move-in-methods

Check for use-after-move in JS glue when `--debug` is enabled again

Author: alexcrichton

crates/cli-support/src/js/binding.rs

@@ -138,15 +138,20 @@ impl<'a, 'b> Builder<'a, 'b> {
             // method, so the leading parameter is the this pointer stored on
             // the JS object, so synthesize that here.
             match self.method {
-                Some(true) => {
+                Some(consumes_self) => {
                     drop(webidl_params.next());
-                    self.args_prelude.push_str("const ptr = this.ptr;\n");
-                    self.args_prelude.push_str("this.ptr = 0;\n");
-                    arg_names.push("ptr".to_string());
-                }
-                Some(false) => {
-                    drop(webidl_params.next());
-                    arg_names.push("this.ptr".to_string());
+                    if self.cx.config.debug {
+                        self.args_prelude.push_str(
+                            "if (this.ptr == 0) throw new Error('Attempt to use a moved value');\n",
+                        );
+                    }
+                    if consumes_self {
+                        self.args_prelude.push_str("const ptr = this.ptr;\n");
+                        self.args_prelude.push_str("this.ptr = 0;\n");
+                        arg_names.push("ptr".to_string());
+                    } else {
+                        arg_names.push("this.ptr".to_string());
+                    }
                 }
                 None => {}
             }

tests/wasm/validate_prt.js

@@ -1,23 +1,36 @@
 const wasm = require('wasm-bindgen-test.js');
 const assert = require('assert');
 
+// NB: `wasm-pack` uses the presence of checks for moved values as a way to test
+// whether it is correctly enabling `--debug` when configured to do so, so don't
+// change this expected debug output without also updating `wasm-pack`'s tests.
+const assertMovedPtrThrows = process.env.WASM_BINDGEN_NO_DEBUG == null
+    ? f => assert.throws(f, /Attempt to use a moved value/)
+    : f => assert.throws(f, /null pointer passed to rust/);
+
 const useMoved = () => {
     const apple = new wasm.Fruit('apple');
     apple.name();
     wasm.eat(apple);
-    assert.throws(() => apple.name(),
-      /Attempt to use a moved value|null pointer passed to rust/);
+    assertMovedPtrThrows(() => apple.name());
 };
 
 const moveMoved = () => {
     const pear = new wasm.Fruit('pear');
     pear.name();
     wasm.eat(pear);
-    assert.throws(() => wasm.eat(pear),
-      /Attempt to use a moved value|null pointer passed to rust/);
+    assertMovedPtrThrows(() => wasm.eat(pear));
+};
+
+const methodMoved = () => {
+    const quince = new wasm.Fruit('quince');
+    quince.name();
+    quince.rot();
+    assertMovedPtrThrows(() => quince.rot());
 };
 
 exports.js_works = () => {
     useMoved();
     moveMoved();
+    methodMoved();
 };

tests/wasm/validate_prt.rs

@@ -21,6 +21,10 @@ impl Fruit {
     pub fn new(name: String) -> Self {
         Fruit { name }
     }
+
+    pub fn rot(self) {
+        drop(self);
+    }
 }
 
 #[wasm_bindgen]