embind: Fix method pointers for AOT JS generation.

src/embind/embind.js

@@ -781,6 +781,7 @@ var LibraryEmbind = {
 #endif
 #if EMBIND_AOT
     '$InvokerFunctions',
+    '$createJsInvokerSignature',
 #endif
 #if ASYNCIFY
     '$Asyncify',
@@ -884,7 +885,7 @@ var LibraryEmbind = {
 #else
   // Builld the arguments that will be passed into the closure around the invoker
   // function.
-  var closureArgs = [throwBindingError, cppInvokerFunc, cppTargetFunc, runDestructors, argTypes[0], argTypes[1]];
+  var closureArgs = [humanName, throwBindingError, cppInvokerFunc, cppTargetFunc, runDestructors, argTypes[0], argTypes[1]];
 #if EMSCRIPTEN_TRACING
   closureArgs.push(Module);
 #endif
@@ -903,9 +904,10 @@ var LibraryEmbind = {
   }
 
 #if EMBIND_AOT
-  var invokerFn = InvokerFunctions[cppTargetFunc].apply(null, closureArgs);
+  var signature = createJsInvokerSignature(argTypes, isClassMethodFunc, returns, isAsync);
+  var invokerFn = InvokerFunctions[signature].apply(null, closureArgs);
 #else
-  let [args, invokerFnBody] = createJsInvoker(humanName, argTypes, isClassMethodFunc, returns, isAsync);
+  let [args, invokerFnBody] = createJsInvoker(argTypes, isClassMethodFunc, returns, isAsync);
   args.push(invokerFnBody);
   var invokerFn = newFunc(Function, args).apply(null, closureArgs);
 #endif

src/embind/embind_gen.js

@@ -7,6 +7,8 @@
 var LibraryEmbind = {
 
   $moduleDefinitions: [],
+  // Function signatures that have already been generated for JS generation.
+  $emittedFunctions: 'new Set()',
 
   $PrimitiveType: class {
     constructor(typeId, name, destructorType) {
@@ -40,7 +42,7 @@ var LibraryEmbind = {
       this.destructorType = 'none'; // Same as emval.
     }
   },
-  $FunctionDefinition__deps: ['$createJsInvoker'],
+  $FunctionDefinition__deps: ['$createJsInvoker', '$createJsInvokerSignature', '$emittedFunctions'],
   $FunctionDefinition: class {
     constructor(name, returnType, argumentTypes, functionIndex, thisType = null, isAsync = false) {
       this.name = name;
@@ -105,10 +107,15 @@ var LibraryEmbind = {
       for (const argType of this.argumentTypes) {
         argTypes.push(this.convertToEmbindType(argType.type));
       }
-      let [args, body] = createJsInvoker(this.name, argTypes, !!this.thisType, this.returnType.name !== 'void', this.isAsync);
+      const signature = createJsInvokerSignature(argTypes, !!this.thisType, this.returnType.name !== 'void', this.isAsync)
+      if (emittedFunctions.has(signature)) {
+        return;
+      }
+      emittedFunctions.add(signature);
+      let [args, body] = createJsInvoker(argTypes, !!this.thisType, this.returnType.name !== 'void', this.isAsync);
       out.push(
         // The ${""} is hack to workaround the preprocessor replacing "function".
-        `'${this.functionIndex}': f${""}unction(${args.join(',')}) {\n${body}},`
+        `'${signature}': f${""}unction(${args.join(',')}) {\n${body}},`
       );
     }
   },

src/embind/embind_shared.js

@@ -179,8 +179,33 @@ var LibraryEmbindShared = {
     return false;
   },
 
+  // Many of the JS invoker functions are generic and can be reused for multiple
+  // function bindings. This function needs to match createJsInvoker and create
+  // a unique signature for any inputs that will create different invoker
+  // function outputs.
+  $createJsInvokerSignature(argTypes, isClassMethodFunc, returns, isAsync) {
+    const signature = [
+      isClassMethodFunc ? 't' : 'f',
+      returns ? 't' : 'f',
+      isAsync ? 't' : 'f'
+    ];
+    for (let i = isClassMethodFunc ? 1 : 2; i < argTypes.length; ++i) {
+      const arg = argTypes[i];
+      let destructorSig = '';
+      if (arg.destructorFunction === undefined) {
+        destructorSig = 'u';
+      } else if (arg.destructorFunction === null) {
+        destructorSig = 'n';
+      } else {
+        destructorSig = 't';
+      }
+      signature.push(destructorSig);
+    }
+    return signature.join('');
+  },
+
   $createJsInvoker__deps: ['$usesDestructorStack'],
-  $createJsInvoker(humanName, argTypes, isClassMethodFunc, returns, isAsync) {
+  $createJsInvoker(argTypes, isClassMethodFunc, returns, isAsync) {
     var needsDestructorStack = usesDestructorStack(argTypes);
     var argCount = argTypes.length;
     var argsList = "";
@@ -193,19 +218,19 @@ var LibraryEmbindShared = {
     var invokerFnBody = `
       return function (${argsList}) {
       if (arguments.length !== ${argCount - 2}) {
-        throwBindingError('function ${humanName} called with ' + arguments.length + ' arguments, expected ${argCount - 2}');
+        throwBindingError('function ' + humanName + ' called with ' + arguments.length + ' arguments, expected ${argCount - 2}');
       }`;
 
 #if EMSCRIPTEN_TRACING
-    invokerFnBody += `Module.emscripten_trace_enter_context('embind::${humanName}');\n`;
+    invokerFnBody += `Module.emscripten_trace_enter_context('embind::' + humanName );\n`;
 #endif
 
     if (needsDestructorStack) {
       invokerFnBody += "var destructors = [];\n";
     }
 
     var dtorStack = needsDestructorStack ? "destructors" : "null";
-    var args1 = ["throwBindingError", "invoker", "fn", "runDestructors", "retType", "classParam"];
+    var args1 = ["humanName", "throwBindingError", "invoker", "fn", "runDestructors", "retType", "classParam"];
 
 #if EMSCRIPTEN_TRACING
     args1.push("Module");
@@ -216,7 +241,7 @@ var LibraryEmbindShared = {
     }
 
     for (var i = 0; i < argCount - 2; ++i) {
-      invokerFnBody += "var arg"+i+"Wired = argType"+i+"['toWireType']("+dtorStack+", arg"+i+"); // "+argTypes[i+2].name+"\n";
+      invokerFnBody += "var arg"+i+"Wired = argType"+i+"['toWireType']("+dtorStack+", arg"+i+");\n";
       args1.push("argType"+i);
     }
 
@@ -240,7 +265,7 @@ var LibraryEmbindShared = {
       for (var i = isClassMethodFunc?1:2; i < argTypes.length; ++i) { // Skip return value at index 0 - it's not deleted here. Also skip class type if not a method.
         var paramName = (i === 1 ? "thisWired" : ("arg"+(i - 2)+"Wired"));
         if (argTypes[i].destructorFunction !== null) {
-          invokerFnBody += paramName+"_dtor("+paramName+"); // "+argTypes[i].name+"\n";
+          invokerFnBody += paramName+"_dtor("+paramName+");\n";
           args1.push(paramName+"_dtor");
         }
       }
@@ -269,7 +294,7 @@ var LibraryEmbindShared = {
     invokerFnBody += "}\n";
 
 #if ASSERTIONS
-    invokerFnBody = `if (arguments.length !== ${args1.length}){ throw new Error("${humanName} Expected ${args1.length} closure arguments " + arguments.length + " given."); }\n${invokerFnBody}`;
+    invokerFnBody = `if (arguments.length !== ${args1.length}){ throw new Error(humanName + "Expected ${args1.length} closure arguments " + arguments.length + " given."); }\n${invokerFnBody}`;
 #endif
     return [args1, invokerFnBody];
   }

test/other/embind_jsgen_method_pointer_stability.cpp

@@ -0,0 +1,32 @@
+#include <stdio.h>
+#include <emscripten.h>
+#include <emscripten/bind.h>
+
+using namespace emscripten;
+
+__attribute__((constructor))
+void malloc_in_static_constructor(void) {
+  // Malloc different sizes in AOT generation mode and normal mode to test
+  // that method pointers still work correctly.
+  EM_ASM(
+    if (typeof InvokerFunctions == 'undefined') {
+      _malloc(10);
+    } else {
+      _malloc(100);
+    }
+  );
+}
+
+class Foo {
+public:
+  void go() {}
+};
+
+int main() {
+  printf("done\n");
+}
+
+EMSCRIPTEN_BINDINGS(xxx) {
+    class_<Foo>("Foo")
+      .function("go", &Foo::go);
+}

test/test_other.py

@@ -3129,6 +3129,12 @@ def test_embind_tsgen_exceptions(self):
                       '-lembind', '--embind-emit-tsd', 'embind_tsgen.d.ts', '-fwasm-exceptions', '-sASSERTIONS'])
     self.assertFileContents(test_file('other/embind_tsgen.d.ts'), read_file('embind_tsgen.d.ts'))
 
+  def test_embind_jsgen_method_pointer_stability(self):
+    self.emcc_args += ['-lembind', '-sEMBIND_AOT']
+    # Test that when method pointers are allocated at different addresses that
+    # AOT JS generation still works correctly.
+    self.do_runf('other/embind_jsgen_method_pointer_stability.cpp', 'done')
+
   def test_emconfig(self):
     output = self.run_process([emconfig, 'LLVM_ROOT'], stdout=PIPE).stdout.strip()
     self.assertEqual(output, config.LLVM_ROOT)