diff --git a/src/embind/embind.js b/src/embind/embind.js index 44b1131ac179..a1c01cbcd2b5 100644 --- a/src/embind/embind.js +++ b/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 diff --git a/src/embind/embind_gen.js b/src/embind/embind_gen.js index d5446e6f18e4..98735cec8fa4 100644 --- a/src/embind/embind_gen.js +++ b/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}},` ); } }, diff --git a/src/embind/embind_shared.js b/src/embind/embind_shared.js index e45fb2add9ac..3c61ce07804e 100644 --- a/src/embind/embind_shared.js +++ b/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,11 +218,11 @@ 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) { @@ -205,7 +230,7 @@ var LibraryEmbindShared = { } 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]; } diff --git a/test/other/embind_jsgen_method_pointer_stability.cpp b/test/other/embind_jsgen_method_pointer_stability.cpp new file mode 100644 index 000000000000..8031b86fd648 --- /dev/null +++ b/test/other/embind_jsgen_method_pointer_stability.cpp @@ -0,0 +1,32 @@ +#include +#include +#include + +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") + .function("go", &Foo::go); +} diff --git a/test/test_other.py b/test/test_other.py index 79098a8b0306..9f935f2decfd 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -3130,6 +3130,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)