Fix a possible memory corruption bug when using val::as

5 files changed, 53 insertions, 7 deletions

diff --git a/src/embind/emval.js b/src/embind/emval.js
index 0d075188..d4e4271d 100644
--- a/src/embind/emval.js
+++ b/src/embind/emval.js
@@ -3,7 +3,7 @@
 /*global new_*/
 /*global createNamedFunction*/
 /*global readLatin1String, writeStringToMemory*/
-/*global requireRegisteredType, throwBindingError*/
+/*global requireRegisteredType, throwBindingError, runDestructors*/
 /*jslint sub:true*/ /* The symbols 'fromWireType' and 'toWireType' must be accessed via array notation to be closure-safe since craftInvokerFunction crafts functions as strings that can't be closured. */
 
 var Module = Module || {};
@@ -199,11 +199,12 @@ function __emval_set_property(handle, key, value) {
     _emval_handle_array[handle].value[_emval_handle_array[key].value] = _emval_handle_array[value].value;
 }
 
-function __emval_as(handle, returnType) {
+function __emval_as(handle, returnType, runDestructorsRef) {
     requireHandle(handle);
     returnType = requireRegisteredType(returnType, 'emval::as');
     var destructors = [];
-    // caller owns destructing
+    var rd = __emval_register(runDestructors.bind(undefined, destructors));
+    HEAP32[runDestructorsRef >> 2] = rd;
     return returnType['toWireType'](destructors, _emval_handle_array[handle].value);
 }
 
diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h
index b712d164..5a04d30f 100644
--- a/system/include/emscripten/val.h
+++ b/system/include/emscripten/val.h
@@ -11,6 +11,11 @@ namespace emscripten {
             void _emval_register_symbol(const char*);
 
             typedef struct _EM_VAL* EM_VAL;
+
+            // TODO: functions returning this are reinterpret_cast
+            // into the correct return type.  this needs some thought
+            // for asm.js.
+            typedef void _POLYMORPHIC_RESULT;
         
             void _emval_incref(EM_VAL value);
             void _emval_decref(EM_VAL value);
@@ -32,7 +37,8 @@ namespace emscripten {
             EM_VAL _emval_get_module_property(const char* name);
             EM_VAL _emval_get_property(EM_VAL object, EM_VAL key);
             void _emval_set_property(EM_VAL object, EM_VAL key, EM_VAL value);
-            void _emval_as(EM_VAL value, TYPEID returnType);
+            _POLYMORPHIC_RESULT _emval_as(EM_VAL value, TYPEID returnType, EM_VAL* runDestructors);
+
             EM_VAL _emval_call(
                 EM_VAL value,
                 unsigned argCount,
@@ -94,6 +100,22 @@ namespace emscripten {
                     toWireType(std::forward<Args>(args))...);
             }
         };
+
+        struct DestructorsRunner {
+        public:
+            DestructorsRunner(EM_VAL v)
+                : dr(v)
+            {}
+            DestructorsRunner(const DestructorsRunner&) = delete;
+            void operator=(const DestructorsRunner&) = delete;
+            ~DestructorsRunner() {
+                EM_VAL rv = _emval_call(dr, 0, 0);
+                _emval_decref(rv); // TODO: if we had an _emval_call_void we wouldn't need this
+                _emval_decref(dr);
+            }
+        private:
+            EM_VAL dr;
+        };
     }
 
 #define EMSCRIPTEN_SYMBOL(name)                                         \
@@ -262,11 +284,13 @@ namespace emscripten {
 
             typedef typename BT::WireType (*TypedAs)(
                 EM_VAL value,
-                TYPEID returnType);
+                TYPEID returnType,
+                EM_VAL* runDestructors);
             TypedAs typedAs = reinterpret_cast<TypedAs>(&_emval_as);
 
-            typename BT::WireType wt = typedAs(handle, TypeID<T>::get());
-            WireDeleter<T> deleter(wt);
+            EM_VAL runDestructors;
+            typename BT::WireType wt = typedAs(handle, TypeID<T>::get(), &runDestructors);
+            DestructorsRunner dr(runDestructors);
             return BT::fromWireType(wt);
         }
 
diff --git a/system/include/emscripten/wire.h b/system/include/emscripten/wire.h
index a5892216..70deb2c7 100644
--- a/system/include/emscripten/wire.h
+++ b/system/include/emscripten/wire.h
@@ -7,6 +7,7 @@
 //
 // We'll call the on-the-wire type WireType.
 
+#include <stdio.h>
 #include <cstdlib>
 #include <memory>
 #include <string>
diff --git a/tests/embind/embind.test.js b/tests/embind/embind.test.js
index da81a81e..ee80da0c 100644
--- a/tests/embind/embind.test.js
+++ b/tests/embind/embind.test.js
@@ -1888,6 +1888,18 @@ module({
             sh.delete();
         });
     });
+
+    BaseFixture.extend("val::as from pointer to value", function() {
+        test("calling as on pointer with value makes a copy", function() {
+            var sh1 = new cm.StringHolder("Hello world");
+            var sh2 = cm.return_StringHolder_copy(sh1);
+            assert.equal("Hello world", sh1.get());
+            assert.equal("Hello world", sh2.get());
+            assert.false(sh1.isAliasOf(sh2));
+            sh2.delete();
+            sh1.delete();
+        });
+    });
 });
 
 /* global run_all_tests */
diff --git a/tests/embind/embind_test.cpp b/tests/embind/embind_test.cpp
index d6b27bce..f723f240 100644
--- a/tests/embind/embind_test.cpp
+++ b/tests/embind/embind_test.cpp
@@ -2243,3 +2243,11 @@ void clear_StringHolder(StringHolder& sh) {
 EMSCRIPTEN_BINDINGS(references) {
     function("clear_StringHolder", &clear_StringHolder);
 }
+
+StringHolder return_StringHolder_copy(val func) {
+    return func.as<StringHolder>();
+}
+
+EMSCRIPTEN_BINDINGS(return_values) {
+    function("return_StringHolder_copy", &return_StringHolder_copy);
+}