embind: Fix some embind issues with memory64. (#21061)

brendandahl · web-flow · commit 1059b203fdc4 · 2024-01-12T17:37:19.000-08:00
* embind: Fix some embind issues with memory64.

 - Fixes string encoding/decoding by using the correct offsets.
 - Allows numbers to be automatically converted to BigInts.
 - Only validates the range assertion on BigInts when ASSERTIONS are
   enabled. This more closely matches the behavior of other number
   validation.
 - Fixes function signatures for longs in memory64 mode.
 - Enables test_embind in memory64 mode.
 - Updates test_embind_i64_binding to match the new behavior of allowing
   auto conversion of numbers to BigInts.

* Only test throwing behavior when assertions are enabled.
diff --git a/src/embind/embind.js b/src/embind/embind.js
@@ -432,9 +432,14 @@ var LibraryEmbind = {
         if (typeof value != "bigint" && typeof value != "number") {
           throw new TypeError(`Cannot convert "${embindRepr(value)}" to ${this.name}`);
         }
+        if (typeof value == "number") {
+          value = BigInt(value);
+        }
+#if ASSERTIONS
         if (value < minRange || value > maxRange) {
           throw new TypeError(`Passing a number "${embindRepr(value)}" from JS side to C/C++ side to an argument of type "${name}", which is outside the valid range [${minRange}, ${maxRange}]!`);
         }
+#endif
         return value;
       },
       'argPackAdvance': GenericWireTypeSize,
@@ -551,7 +556,7 @@ var LibraryEmbind = {
           length = value.length;
         }
 
-        // assumes 4-byte alignment
+        // assumes POINTER_SIZE alignment
         var base = _malloc({{{ POINTER_SIZE }}} + length + 1);
         var ptr = base + {{{ POINTER_SIZE }}};
         {{{ makeSetValue('base', '0', 'length', SIZE_TYPE) }}};
@@ -616,10 +621,10 @@ var LibraryEmbind = {
         var HEAP = getHeap();
         var str;
 
-        var decodeStartPtr = value + 4;
+        var decodeStartPtr = value + {{{ POINTER_SIZE }}};
         // Looping here to support possible embedded '0' bytes
         for (var i = 0; i <= length; ++i) {
-          var currentBytePtr = value + 4 + i * charSize;
+          var currentBytePtr = value + {{{ POINTER_SIZE }}} + i * charSize;
           if (i == length || HEAP[currentBytePtr >> shift] == 0) {
             var maxReadBytes = currentBytePtr - decodeStartPtr;
             var stringSegment = decodeString(decodeStartPtr, maxReadBytes);
@@ -642,12 +647,12 @@ var LibraryEmbind = {
           throwBindingError(`Cannot pass non-string to C++ string type ${name}`);
         }
 
-        // assumes 4-byte alignment
+        // assumes POINTER_SIZE alignment
         var length = lengthBytesUTF(value);
-        var ptr = _malloc(4 + length + charSize);
-        HEAPU32[ptr >> 2] = length >> shift;
+        var ptr = _malloc({{{ POINTER_SIZE }}} + length + charSize);
+        {{{ makeSetValue('ptr', '0', 'length >> shift', SIZE_TYPE) }}};
 
-        encodeString(value, ptr + 4, length + charSize);
+        encodeString(value, ptr + {{{ POINTER_SIZE }}}, length + charSize);
 
         if (destructors !== null) {
           destructors.push(_free, ptr);
diff --git a/system/include/emscripten/bind.h b/system/include/emscripten/bind.h
@@ -519,6 +519,12 @@ struct SignatureCode<size_t> {
         return 'p';
     }
 };
+template<>
+struct SignatureCode<long> {
+    static constexpr char get() {
+        return 'j';
+    }
+};
 #endif
 
 template<typename... Args>
@@ -532,6 +538,7 @@ template<> struct SignatureTranslator<void> { using type = void; };
 template<> struct SignatureTranslator<float> { using type = float; };
 template<> struct SignatureTranslator<double> { using type = double; };
 #ifdef __wasm64__
+template<> struct SignatureTranslator<long> { using type = long; };
 template<> struct SignatureTranslator<size_t> { using type = size_t; };
 template<typename PtrType>
 struct SignatureTranslator<PtrType*> { using type = void*; };
diff --git a/system/include/emscripten/val.h b/system/include/emscripten/val.h
@@ -540,6 +540,28 @@ class val {
     return fromGenericWireType<T>(result);
   }
 
+#ifdef __wasm64__
+  template<>
+  long as<long>() const {
+    using namespace internal;
+
+    typedef BindingType<long> BT;
+    typename WithPolicies<>::template ArgTypeList<long> targetType;
+
+    return _emval_as_int64(as_handle(), targetType.getTypes()[0]);
+  }
+
+  template<>
+  unsigned long as<unsigned long>() const {
+    using namespace internal;
+
+    typedef BindingType<unsigned long> BT;
+    typename WithPolicies<>::template ArgTypeList<unsigned long> targetType;
+
+    return _emval_as_uint64(as_handle(), targetType.getTypes()[0]);
+  }
+#endif
+
   template<>
   int64_t as<int64_t>() const {
     using namespace internal;
diff --git a/test/embind/embind.test.js b/test/embind/embind.test.js
@@ -856,10 +856,17 @@ module({
                 assert.throws(TypeError, function() { cm.int_to_string(2147483648); });
                 assert.throws(TypeError, function() { cm.unsigned_int_to_string(-1); });
                 assert.throws(TypeError, function() { cm.unsigned_int_to_string(4294967296); });
-                assert.throws(TypeError, function() { cm.long_to_string(-2147483649); });
-                assert.throws(TypeError, function() { cm.long_to_string(2147483648); });
-                assert.throws(TypeError, function() { cm.unsigned_long_to_string(-1); });
-                assert.throws(TypeError, function() { cm.unsigned_long_to_string(4294967296); });
+                if (cm.getCompilerSetting('MEMORY64')) {
+                    assert.throws(TypeError, function() { cm.long_to_string(-18446744073709551616n); });
+                    assert.throws(TypeError, function() { cm.long_to_string(18446744073709551616n); });
+                    assert.throws(TypeError, function() { cm.unsigned_long_to_string(-1n); });
+                    assert.throws(TypeError, function() { cm.unsigned_long_to_string(18446744073709551616n); });
+                } else {
+                    assert.throws(TypeError, function() { cm.long_to_string(-2147483649); });
+                    assert.throws(TypeError, function() { cm.long_to_string(2147483648); });
+                    assert.throws(TypeError, function() { cm.unsigned_long_to_string(-1); });
+                    assert.throws(TypeError, function() { cm.unsigned_long_to_string(4294967296); });
+                }
             } else {
                 // test that an out of range value doesn't throw without assertions.
                 assert.equal("-129", cm.char_to_string(-129));
@@ -2730,7 +2737,11 @@ module({
             assert.equal(127,   cm.val_as_char(127));
             assert.equal(32767, cm.val_as_short(32767));
             assert.equal(65536, cm.val_as_int(65536));
-            assert.equal(65536, cm.val_as_long(65536));
+            if (cm.getCompilerSetting('MEMORY64')) {
+                assert.equal(65536n, cm.val_as_long(65536));
+            } else {
+                assert.equal(65536, cm.val_as_long(65536));
+            }
             assert.equal(10.5,  cm.val_as_float(10.5));
             assert.equal(10.5,  cm.val_as_double(10.5));
 
diff --git a/test/embind/test_i64_binding.cpp b/test/embind/test_i64_binding.cpp
@@ -41,30 +41,13 @@ int run_js(string js_code)
   }, js_code_pointer);
 }
 
-void ensure_js_throws(string js_code, string error_type)
-{
-  js_code.append(";");
-  const char* js_code_pointer = js_code.c_str();
-  const char* error_type_pointer = error_type.c_str();
-  assert(EM_ASM_INT({
-    var js_code = UTF8ToString($0);
-    var error_type = UTF8ToString($1);
-    try {
-      eval(js_code);
-    }
-    catch(error_thrown)
-    {
-      return error_thrown.name === error_type;
-    }
-    return false;
-  }, js_code_pointer, error_type_pointer));
-}
-
 EMSCRIPTEN_BINDINGS(tests) {
   register_vector<int64_t>("Int64Vector");
   register_vector<uint64_t>("UInt64Vector");
 }
 
+extern "C" void ensure_js_throws_with_assertions_enabled(const char* js_code, const char* error_type);
+
 int main()
 {
   const int64_t max_int64_t = numeric_limits<int64_t>::max();
@@ -85,11 +68,8 @@ int main()
   assert_js("v64.size() === 5");
   assert_js("v64.get(4) === 1234n");
 
-  test("vector<int64_t> Cannot convert number to int64_t");
-  ensure_js_throws("v64.push_back(1234)", "TypeError");
-
   test("vector<int64_t> Cannot convert bigint that is too big");
-  ensure_js_throws("v64.push_back(12345678901234567890123456n)", "TypeError");
+  ensure_js_throws_with_assertions_enabled("v64.push_back(12345678901234567890123456n)", "TypeError");
 
   test("vector<uint64_t>");
   val myval2(vector<uint64_t>{1, 2, 3, 4});
@@ -103,14 +83,15 @@ int main()
   assert_js("vU64.size() === 5");
   assert_js("vU64.get(4) === 1234n");
 
-  test("vector<uint64_t> Cannot convert number to uint64_t");
-  ensure_js_throws("vU64.push_back(1234)", "TypeError");
+  execute_js("vU64.push_back(1234)");
+  assert_js("vU64.size() === 6");
+  assert_js("vU64.get(5) === 1234n");
 
   test("vector<uint64_t> Cannot convert bigint that is too big");
-  ensure_js_throws("vU64.push_back(12345678901234567890123456n)", "TypeError");
+  ensure_js_throws_with_assertions_enabled("vU64.push_back(12345678901234567890123456n)", "TypeError");
 
   test("vector<uint64_t> Cannot convert bigint that is negative");
-  ensure_js_throws("vU64.push_back(-1n)", "TypeError");
+  ensure_js_throws_with_assertions_enabled("vU64.push_back(-1n)", "TypeError");
 
   myval.call<void>("delete");
   myval2.call<void>("delete");
diff --git a/test/embind/test_i64_binding.js b/test/embind/test_i64_binding.js
@@ -0,0 +1,21 @@
+addToLibrary({
+  ensure_js_throws_with_assertions_enabled__deps: ['$bigintToI53Checked'],
+  ensure_js_throws_with_assertions_enabled: function(js_code, error_type) {
+    js_code = UTF8ToString(bigintToI53Checked(js_code));
+    error_type = UTF8ToString(bigintToI53Checked(error_type));
+    js_code += ";";
+    try {
+      eval(js_code);
+    } catch(error_thrown) {
+#if ASSERTIONS
+      assert(error_thrown.name === error_type);
+#else
+      assert(false);
+#endif
+      return;
+    }
+#if ASSERTIONS
+    assert(false);
+#endif
+  }
+});
diff --git a/test/embind/test_i64_binding.out b/test/embind/test_i64_binding.out
@@ -2,14 +2,10 @@ start
 test:
 vector<int64_t>
 test:
-vector<int64_t> Cannot convert number to int64_t
-test:
 vector<int64_t> Cannot convert bigint that is too big
 test:
 vector<uint64_t>
 test:
-vector<uint64_t> Cannot convert number to uint64_t
-test:
 vector<uint64_t> Cannot convert bigint that is too big
 test:
 vector<uint64_t> Cannot convert bigint that is negative
diff --git a/test/test_core.py b/test/test_core.py
@@ -7559,7 +7559,7 @@ def test_embind_i64_val(self):
   @no_wasm2js('wasm_bigint')
   def test_embind_i64_binding(self):
     self.set_setting('WASM_BIGINT')
-    self.emcc_args += ['-lembind']
+    self.emcc_args += ['-lembind', '--js-library', test_file('embind/test_i64_binding.js')]
     self.node_args += shared.node_bigint_flags(self.get_nodejs())
     self.do_run_in_out_file_test('embind/test_i64_binding.cpp', assert_identical=True)
 
diff --git a/test/test_other.py b/test/test_other.py
@@ -2968,6 +2968,7 @@ def test_embind_closure_no_dynamic_execution(self):
     'no_utf8': ['-sEMBIND_STD_STRING_IS_UTF8=0'],
     'no_dynamic': ['-sDYNAMIC_EXECUTION=0'],
     'aot_js': ['-sDYNAMIC_EXECUTION=0', '-sEMBIND_AOT', '-DSKIP_UNBOUND_TYPES'],
+    'wasm64': ['-sMEMORY64', '-Wno-experimental'],
   })
   @parameterized({
     '': [],
@@ -2980,6 +2981,8 @@ def test_embind_closure_no_dynamic_execution(self):
     'strict_js': ['-sSTRICT_JS']
   })
   def test_embind(self, *extra_args):
+    if '-sMEMORY64' in extra_args:
+      self.require_wasm64()
     self.emcc_args += [
       '--no-entry',
       # This test explicitly creates std::string from unsigned char pointers
