embind - Fix reading various bindings types from val on wasm64.

 - The function simpleReadValueFromPointer was always reading pointers as
i32 numbers which was incorrect on wasm64.
- Decoding of std::wstring was using a right shift to read pointers. Right
  shift was truncating numbers larger than i32 and causing misreads.
- Added tests to execute readValueFromPointer on all the binding types
  to ensure these don't break on wasm64.

Author: brendandahl

src/embind/embind.js

@@ -15,7 +15,7 @@
 // -- jshint doesn't understand library syntax, so we need to specifically tell it about the symbols we define
 /*global typeDependencies, flushPendingDeletes, getTypeName, getBasestPointer, throwBindingError, UnboundTypeError, embindRepr, registeredInstances, registeredTypes*/
 /*global ensureOverloadTable, embind__requireFunction, awaitingDependencies, makeLegalFunctionName, embind_charCodes:true, registerType, createNamedFunction, RegisteredPointer, throwInternalError*/
-/*global simpleReadValueFromPointer, floatReadValueFromPointer, integerReadValueFromPointer, enumReadValueFromPointer, replacePublicSymbol, craftInvokerFunction, tupleRegistrations*/
+/*global floatReadValueFromPointer, integerReadValueFromPointer, enumReadValueFromPointer, replacePublicSymbol, craftInvokerFunction, tupleRegistrations*/
 /*global finalizationRegistry, attachFinalizer, detachFinalizer, releaseClassHandle, runDestructor*/
 /*global ClassHandle, makeClassHandle, structRegistrations, whenDependentTypesAreResolved, BindingError, deletionQueue, delayFunction:true, upcastPointer*/
 /*global exposePublicSymbol, heap32VectorToArray, newFunc, char_0, char_9*/
@@ -41,7 +41,7 @@ var LibraryEmbind = {
   // If register_type is used, emval will be registered multiple times for
   // different type id's, but only a single type object is needed on the JS side
   // for all of them. Store the type for reuse.
-  $EmValType__deps: ['_emval_decref', '$Emval', '$simpleReadValueFromPointer', '$GenericWireTypeSize'],
+  $EmValType__deps: ['_emval_decref', '$Emval', '$readPointer', '$GenericWireTypeSize'],
   $EmValType: `{
     name: 'emscripten::val',
     'fromWireType': (handle) => {
@@ -51,7 +51,7 @@ var LibraryEmbind = {
     },
     'toWireType': (destructors, value) => Emval.toHandle(value),
     'argPackAdvance': GenericWireTypeSize,
-    'readValueFromPointer': simpleReadValueFromPointer,
+    'readValueFromPointer': readPointer,
     destructorFunction: null, // This type does not need a destructor
 
     // TODO: do we need a deleteObject here?  write a test where
@@ -494,12 +494,6 @@ var LibraryEmbind = {
     });
   },
 
-  $simpleReadValueFromPointer__docs: '/** @suppress {globalThis} */',
-  // For types whose wire types are 32-bit pointers.
-  $simpleReadValueFromPointer: function(pointer) {
-    return this['fromWireType']({{{ makeGetValue('pointer', '0', 'i32') }}});
-  },
-
   $readPointer__docs: '/** @suppress {globalThis} */',
   $readPointer: function(pointer) {
     return this['fromWireType']({{{ makeGetValue('pointer', '0', '*') }}});
@@ -617,33 +611,33 @@ var LibraryEmbind = {
     ],
   _embind_register_std_wstring: (rawType, charSize, name) => {
     name = readLatin1String(name);
-    var decodeString, encodeString, getHeap, lengthBytesUTF, shift;
+    var decodeString, encodeString, getValue, lengthBytesUTF;
     if (charSize === 2) {
       decodeString = UTF16ToString;
       encodeString = stringToUTF16;
       lengthBytesUTF = lengthBytesUTF16;
+      getValue = (pointer) => {{{ makeGetValue('pointer', 0, 'u16') }}};
       getHeap = () => HEAPU16;
-      shift = 1;
     } else if (charSize === 4) {
       decodeString = UTF32ToString;
       encodeString = stringToUTF32;
       lengthBytesUTF = lengthBytesUTF32;
-      getHeap = () => HEAPU32;
-      shift = 2;
+      getValue = (pointer) => {{{ makeGetValue('pointer', 0, 'u32') }}};
     }
     registerType(rawType, {
       name,
       'fromWireType': (value) => {
         // Code mostly taken from _embind_register_std_string fromWireType
         var length = {{{ makeGetValue('value', 0, '*') }}};
-        var HEAP = getHeap();
         var str;
 
         var decodeStartPtr = value + {{{ POINTER_SIZE }}};
         // Looping here to support possible embedded '0' bytes
         for (var i = 0; i <= length; ++i) {
           var currentBytePtr = value + {{{ POINTER_SIZE }}} + i * charSize;
-          if (i == length || HEAP[currentBytePtr >> shift] == 0) {
+          // Use division instead of shift since the pointer may be greater than
+          // the i32 range.
+          if (i == length || getValue(currentBytePtr) == 0) {
             var maxReadBytes = currentBytePtr - decodeStartPtr;
             var stringSegment = decodeString(decodeStartPtr, maxReadBytes);
             if (str === undefined) {
@@ -668,7 +662,7 @@ var LibraryEmbind = {
         // assumes POINTER_SIZE alignment
         var length = lengthBytesUTF(value);
         var ptr = _malloc({{{ POINTER_SIZE }}} + length + charSize);
-        {{{ makeSetValue('ptr', '0', 'length >> shift', SIZE_TYPE) }}};
+        {{{ makeSetValue('ptr', '0', 'length / charSize', SIZE_TYPE) }}};
 
         encodeString(value, ptr + {{{ POINTER_SIZE }}}, length + charSize);
 
@@ -678,7 +672,7 @@ var LibraryEmbind = {
         return ptr;
       },
       'argPackAdvance': GenericWireTypeSize,
-      'readValueFromPointer': simpleReadValueFromPointer,
+      'readValueFromPointer': readPointer,
       destructorFunction(ptr) {
         _free(ptr);
       }
@@ -1012,7 +1006,7 @@ var LibraryEmbind = {
 
   _embind_finalize_value_array__deps: [
     '$tupleRegistrations', '$runDestructors',
-    '$simpleReadValueFromPointer', '$whenDependentTypesAreResolved'],
+    '$readPointer', '$whenDependentTypesAreResolved'],
   _embind_finalize_value_array: (rawTupleType) => {
     var reg = tupleRegistrations[rawTupleType];
     delete tupleRegistrations[rawTupleType];
@@ -1064,7 +1058,7 @@ var LibraryEmbind = {
           return ptr;
         },
         'argPackAdvance': GenericWireTypeSize,
-        'readValueFromPointer': simpleReadValueFromPointer,
+        'readValueFromPointer': readPointer,
         destructorFunction: rawDestructor,
       }];
     });
@@ -1115,7 +1109,7 @@ var LibraryEmbind = {
 
   _embind_finalize_value_object__deps: [
     '$structRegistrations', '$runDestructors',
-    '$simpleReadValueFromPointer', '$whenDependentTypesAreResolved'],
+    '$readPointer', '$whenDependentTypesAreResolved'],
   _embind_finalize_value_object: (structType) => {
     var reg = structRegistrations[structType];
     delete structRegistrations[structType];
@@ -1173,7 +1167,7 @@ var LibraryEmbind = {
           return ptr;
         },
         'argPackAdvance': GenericWireTypeSize,
-        'readValueFromPointer': simpleReadValueFromPointer,
+        'readValueFromPointer': readPointer,
         destructorFunction: rawDestructor,
       }];
     });

test/code_size/embind_val_wasm.json

@@ -1,10 +1,10 @@
 {
   "a.html": 673,
   "a.html.gz": 431,
-  "a.js": 7387,
-  "a.js.gz": 3112,
+  "a.js": 7325,
+  "a.js.gz": 3088,
   "a.wasm": 11433,
   "a.wasm.gz": 5725,
-  "total": 19493,
-  "total_gz": 9268
+  "total": 19431,
+  "total_gz": 9244
 }

test/embind/test_val_read_pointer.cpp

@@ -0,0 +1,81 @@
+#include <emscripten/bind.h>
+#include <emscripten.h>
+
+using namespace emscripten;
+
+struct ValueObject {
+  ValueObject(): value(43) {};
+  int value;
+};
+
+struct ValueArray {
+  ValueArray(): x(44) {};
+  int x;
+};
+
+struct Foo {
+  Foo(): value(45) {};
+  int value;
+};
+
+enum Enum { ONE, TWO };
+
+EMSCRIPTEN_BINDINGS(xxx) {
+  value_object<ValueObject>("ValueObject")
+    .field("value", &ValueObject::value);
+  
+  value_array<ValueArray>("ValueArray")
+    .element(&ValueArray::x);
+  
+  class_<Foo>("Foo")
+    .property("value", &Foo::value);
+
+  enum_<Enum>("Enum")
+    .value("ONE", ONE)
+    .value("TWO", TWO);
+}
+
+
+int main() {
+  EM_ASM(
+    globalThis["passthrough"] = (arg) => {
+      return arg;
+    };
+    globalThis["passthroughValueObject"] = (arg) => {
+      return arg.value;
+    };
+    globalThis["passthroughValueArray"] = (arg) => {
+      return arg[0];
+    };
+    globalThis["passthroughClass"] = (arg) => {
+      const value = arg.value;
+      arg.delete();
+      return value;
+    };
+    globalThis["passthroughMemoryView"] = (arg) => {
+      return arg[2];
+    };
+  );
+
+  // These tests execute all the various readValueFromPointer functions for each
+  // of the different binding types.
+  assert(val::global().call<bool>("passthrough", true));
+  assert(val::global().call<int>("passthrough", 42) == 42);
+  assert(val::global().call<double>("passthrough", 42.2) == 42.2);
+  ValueObject vo;
+  assert(val::global().call<int>("passthroughValueObject", vo) == 43);
+  ValueArray va;
+  assert(val::global().call<int>("passthroughValueArray", va) == 44);
+  Foo foo;
+  assert(val::global().call<int>("passthroughClass", foo) == 45);
+  assert(val::global().call<std::string>("passthrough", val("abc")) == "abc");
+  std::string str = "hello";
+  assert(val::global().call<std::string>("passthrough", str) == "hello");
+  std::wstring wstr = L"abc";
+  assert(val::global().call<std::wstring>("passthrough", wstr) == L"abc");
+  static const int data[] = {0, 1, 2};
+  assert(val::global().call<int>("passthroughMemoryView", typed_memory_view(3, data)) == 2);
+  assert(val::global().call<Enum>("passthrough", ONE) == ONE);
+
+  return 0;
+}

test/test_core.py

@@ -7471,6 +7471,10 @@ def test_embind_val(self):
     self.emcc_args += ['-lembind']
     self.do_run_in_out_file_test('embind/test_val.cpp')
 
+  def test_embind_val_read_pointer(self):
+    self.emcc_args += ['-lembind']
+    self.do_runf('embind/test_val_read_pointer.cpp')
+
   def test_embind_val_assignment(self):
     err = self.expect_fail([EMCC, test_file('embind/test_val_assignment.cpp'), '-lembind', '-c'])
     self.assertContained('candidate function not viable: expects an lvalue for object argument', err)