diff --git a/src/lib/libembind.js b/src/lib/libembind.js index 3542d45f8c4e9..d0adcbb92fb43 100644 --- a/src/lib/libembind.js +++ b/src/lib/libembind.js @@ -662,6 +662,7 @@ var LibraryEmbind = { // craftInvokerFunction generates the JS invoker function for each function exposed to JS through embind. $craftInvokerFunction__deps: [ '$createNamedFunction', '$runDestructors', '$throwBindingError', '$usesDestructorStack', + '$argsUseStackAlloc', '$stackSave', '$stackRestore', #if DYNAMIC_EXECUTION && !EMBIND_AOT '$createJsInvoker', #endif @@ -709,6 +710,18 @@ var LibraryEmbind = { // TODO: Remove this completely once all function invokers are being dynamically generated. var needsDestructorStack = usesDestructorStack(argTypes); + // Stack-allocating trivial value types get a stackSave/stackRestore + // bracket around the call; see createJsInvoker for the async carve-outs. + var argsNeedStack = argsUseStackAlloc(argTypes); +#if ASYNCIFY == 1 + var useStackFrame = false; +#else + var useStackFrame = argsNeedStack && !isAsync && !needsDestructorStack; +#endif + if (argsNeedStack && !useStackFrame) { + needsDestructorStack = true; + } + var returns = !argTypes[0].isVoid; var expectedArgCount = argCount - 2; @@ -727,19 +740,36 @@ var LibraryEmbind = { Module.emscripten_trace_enter_context(`embind::${humanName}`); #endif destructors.length = 0; - var thisWired; - invokerFuncArgs.length = isClassMethodFunc ? 2 : 1; - invokerFuncArgs[0] = cppTargetFunc; - if (isClassMethodFunc) { - thisWired = argTypes[1].toWireType(destructors, this); - invokerFuncArgs[1] = thisWired; - } - for (var i = 0; i < expectedArgCount; ++i) { - argsWired[i] = argTypes[i + 2].toWireType(destructors, args[i]); - invokerFuncArgs.push(argsWired[i]); + var sp; + if (useStackFrame) { + sp = stackSave(); } + var thisWired; + var rv; + // The frame must be released on every completion, including a throwing + // argument conversion or callee: a skipped stackRestore permanently + // leaks wasm stack. + try { + invokerFuncArgs.length = isClassMethodFunc ? 2 : 1; + invokerFuncArgs[0] = cppTargetFunc; + if (isClassMethodFunc) { + thisWired = argTypes[1].toWireType(destructors, this); + invokerFuncArgs[1] = thisWired; + } + for (var i = 0; i < expectedArgCount; ++i) { + var argType = argTypes[i + 2]; + // Stack-allocating types take the stack path only under a frame; a + // null destructors argument is that contract. + argsWired[i] = argType.toWireType(useStackFrame && argType.argStackAlloc ? null : destructors, args[i]); + invokerFuncArgs.push(argsWired[i]); + } - var rv = cppInvokerFunc(...invokerFuncArgs); + rv = cppInvokerFunc(...invokerFuncArgs); + } finally { + if (useStackFrame) { + stackRestore(sp); + } + } function onDone(rv) { if (needsDestructorStack) { @@ -780,6 +810,10 @@ var LibraryEmbind = { var retType = argTypes[0]; var instType = argTypes[1]; var closureArgs = [humanName, throwBindingError, cppInvokerFunc, cppTargetFunc, runDestructors, retType.fromWireType.bind(retType), instType?.toWireType.bind(instType)]; + if (useStackFrame) { + // Must mirror the `args1.push('stackSave', 'stackRestore')` in createJsInvoker. + closureArgs.push(stackSave, stackRestore); + } #if EMSCRIPTEN_TRACING closureArgs.push(Module); #endif @@ -887,12 +921,16 @@ var LibraryEmbind = { constructorSignature, rawConstructor, destructorSignature, - rawDestructor + rawDestructor, + valueSize, + isTrivial ) => { tupleRegistrations[rawType] = { name: AsciiToString(name), rawConstructor: embind__requireFunction(constructorSignature, rawConstructor), rawDestructor: embind__requireFunction(destructorSignature, rawDestructor), + valueSize, + isTrivial: !!isTrivial, elements: [], }; }, @@ -922,7 +960,7 @@ var LibraryEmbind = { _embind_finalize_value_array__deps: [ '$tupleRegistrations', '$runDestructors', - '$readPointer', '$whenDependentTypesAreResolved'], + '$readPointer', '$whenDependentTypesAreResolved', '$stackAlloc'], _embind_finalize_value_array: (rawTupleType) => { var reg = tupleRegistrations[rawTupleType]; delete tupleRegistrations[rawTupleType]; @@ -933,6 +971,8 @@ var LibraryEmbind = { var rawConstructor = reg.rawConstructor; var rawDestructor = reg.rawDestructor; + var valueSize = reg.valueSize; + var isTrivial = reg.isTrivial; whenDependentTypesAreResolved([rawTupleType], elementTypes, (elementTypes) => { for (const [i, elt] of elements.entries()) { @@ -943,11 +983,19 @@ var LibraryEmbind = { const setter = elt.setter; const setterContext = elt.setterContext; elt.read = (ptr) => getterReturnType.fromWireType(getter(getterContext, ptr)); - elt.write = (ptr, o) => { - var destructors = []; - setter(setterContext, ptr, setterArgumentType.toWireType(destructors, o)); - runDestructors(destructors); - }; + if (setterArgumentType.destructorFunction === null && !setterArgumentType.argStackAlloc) { + // The element type never registers a destructor, so skip the + // per-write destructors array. (Stack-allocating types still need + // the array here: a null destructors argument means an + // invoker-managed stack frame, which a nested write cannot assume.) + elt.write = (ptr, o) => setter(setterContext, ptr, setterArgumentType.toWireType(null, o)); + } else { + elt.write = (ptr, o) => { + var destructors = []; + setter(setterContext, ptr, setterArgumentType.toWireType(destructors, o)); + runDestructors(destructors); + }; + } } return [{ @@ -964,17 +1012,33 @@ var LibraryEmbind = { if (elementsLength !== o.length) { throw new TypeError(`Incorrect number of tuple elements for ${reg.name}: expected=${elementsLength}, actual=${o.length}`); } - var ptr = rawConstructor(); + var ptr; + if (isTrivial && destructors === null) { + // Trivially constructible and destructible, and the invoker + // manages a stack frame around this call: the temporary lives on + // the wasm stack. No allocation, nothing to destruct. Callers + // that defer destruction (emval returns, property setters) pass + // a destructors array instead and take the heap path below. + // Zero-fill so unregistered fields and padding match the + // value-initialization the heap path's `new T()` performs. + ptr = stackAlloc(valueSize); + HEAPU8.fill(0, ptr, ptr + valueSize); + } else { + ptr = rawConstructor(); + if (destructors !== null) { + destructors.push(rawDestructor, ptr); + } + } for (var i = 0; i < elementsLength; ++i) { elements[i].write(ptr, o[i]); } - if (destructors !== null) { - destructors.push(rawDestructor, ptr); - } return ptr; }, readValueFromPointer: readPointer, - destructorFunction: rawDestructor, + // Trivial types have nothing to run after the call: the stack frame + // (or the destructors array, on the deferred path) covers cleanup. + destructorFunction: isTrivial ? null : rawDestructor, + argStackAlloc: isTrivial, }]; }); }, @@ -987,12 +1051,16 @@ var LibraryEmbind = { constructorSignature, rawConstructor, destructorSignature, - rawDestructor + rawDestructor, + valueSize, + isTrivial ) => { structRegistrations[rawType] = { name: AsciiToString(name), rawConstructor: embind__requireFunction(constructorSignature, rawConstructor), rawDestructor: embind__requireFunction(destructorSignature, rawDestructor), + valueSize, + isTrivial: !!isTrivial, fields: [], }; }, @@ -1024,13 +1092,15 @@ var LibraryEmbind = { _embind_finalize_value_object__deps: [ '$structRegistrations', '$runDestructors', - '$readPointer', '$whenDependentTypesAreResolved'], + '$readPointer', '$whenDependentTypesAreResolved', '$stackAlloc'], _embind_finalize_value_object: (structType) => { var reg = structRegistrations[structType]; delete structRegistrations[structType]; var rawConstructor = reg.rawConstructor; var rawDestructor = reg.rawDestructor; + var valueSize = reg.valueSize; + var isTrivial = reg.isTrivial; var fieldRecords = reg.fields; var fieldTypes = fieldRecords.map((field) => field.getterReturnType). concat(fieldRecords.map((field) => field.setterArgumentType)); @@ -1045,11 +1115,14 @@ var LibraryEmbind = { const setterContext = field.setterContext; fields[field.fieldName] = { read: (ptr) => getterReturnType.fromWireType(getter(getterContext, ptr)), - write: (ptr, o) => { - var destructors = []; - setter(setterContext, ptr, setterArgumentType.toWireType(destructors, o)); - runDestructors(destructors); - }, + // See the matching element-write logic in _embind_finalize_value_array. + write: (setterArgumentType.destructorFunction === null && !setterArgumentType.argStackAlloc) + ? (ptr, o) => setter(setterContext, ptr, setterArgumentType.toWireType(null, o)) + : (ptr, o) => { + var destructors = []; + setter(setterContext, ptr, setterArgumentType.toWireType(destructors, o)); + runDestructors(destructors); + }, optional: getterReturnType.optional, }; } @@ -1072,17 +1145,28 @@ var LibraryEmbind = { throw new TypeError(`Missing field: "${fieldName}"`); } } - var ptr = rawConstructor(); + var ptr; + if (isTrivial && destructors === null) { + // See the matching branch in _embind_finalize_value_array: the + // invoker manages a stack frame, so the temporary lives on the + // wasm stack with no allocation and no destructor bookkeeping; + // zero-filled to match the heap path's value-initialization. + ptr = stackAlloc(valueSize); + HEAPU8.fill(0, ptr, ptr + valueSize); + } else { + ptr = rawConstructor(); + if (destructors !== null) { + destructors.push(rawDestructor, ptr); + } + } for (fieldName in fields) { fields[fieldName].write(ptr, o[fieldName]); } - if (destructors !== null) { - destructors.push(rawDestructor, ptr); - } return ptr; }, readValueFromPointer: readPointer, - destructorFunction: rawDestructor, + destructorFunction: isTrivial ? null : rawDestructor, + argStackAlloc: isTrivial, }]; }); }, diff --git a/src/lib/libembind_gen.js b/src/lib/libembind_gen.js index 132e8deeff214..d35b76c80cd1f 100644 --- a/src/lib/libembind_gen.js +++ b/src/lib/libembind_gen.js @@ -133,6 +133,12 @@ var LibraryEmbind = { default: throw new Error(`Bad destructor type '${type.destructorType}'`); } + if (type.argStackAlloc) { + // Trivial value types stack-allocate their argument temporaries; + // must mirror the runtime type object so the invoker signature and + // generated shape match (see createJsInvokerSignature). + ret.argStackAlloc = true; + } return ret; } @@ -356,12 +362,15 @@ var LibraryEmbind = { } }, $ValueArrayDefinition: class { - constructor(typeId, name) { + constructor(typeId, name, isTrivial) { this.typeId = typeId; this.name = name; this.elementTypeIds = []; this.elements = []; - this.destructorType = 'function'; + // Trivial types need no destructor call; their argument temporaries + // live in the invoker's stack frame. + this.destructorType = isTrivial ? 'none' : 'function'; + this.argStackAlloc = !!isTrivial; } print(nameMap, out) { @@ -375,13 +384,15 @@ var LibraryEmbind = { } }, $ValueObjectDefinition: class { - constructor(typeId, name) { + constructor(typeId, name, isTrivial) { this.typeId = typeId; this.name = name; this.fieldTypeIds = []; this.fieldNames = []; this.fields = []; - this.destructorType = 'function'; + // See ValueArrayDefinition: trivial types stack-allocate. + this.destructorType = isTrivial ? 'none' : 'function'; + this.argStackAlloc = !!isTrivial; } print(nameMap, out) { @@ -802,10 +813,12 @@ var LibraryEmbind = { constructorSignature, rawConstructor, destructorSignature, - rawDestructor + rawDestructor, + valueSize, + isTrivial ) { name = AsciiToString(name); - const valueArray = new ValueArrayDefinition(rawType, name); + const valueArray = new ValueArrayDefinition(rawType, name, isTrivial); tupleRegistrations[rawType] = valueArray; }, _embind_register_value_array_element__deps: ['$tupleRegistrations'], @@ -844,10 +857,12 @@ var LibraryEmbind = { constructorSignature, rawConstructor, destructorSignature, - rawDestructor + rawDestructor, + valueSize, + isTrivial ) { name = AsciiToString(name); - const valueObject = new ValueObjectDefinition(rawType, name); + const valueObject = new ValueObjectDefinition(rawType, name, isTrivial); structRegistrations[rawType] = valueObject; }, _embind_register_value_object_field__deps: [ diff --git a/src/lib/libembind_shared.js b/src/lib/libembind_shared.js index 7bda2c575430d..3a30a765123f0 100644 --- a/src/lib/libembind_shared.js +++ b/src/lib/libembind_shared.js @@ -161,6 +161,20 @@ var LibraryEmbindShared = { return false; }, + // Trivially constructible/destructible value types (argStackAlloc) place + // their argument temporaries on the wasm stack when the invoker brackets + // the call in stackSave/stackRestore, skipping the per-call heap temp and + // destructor bookkeeping entirely. + $argsUseStackAlloc(argTypes) { + // Skip return value at index 0 - only arguments stack-allocate. + for (var i = 1; i < argTypes.length; ++i) { + if (argTypes[i] !== null && argTypes[i].argStackAlloc) { + return true; + } + } + 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 @@ -174,7 +188,11 @@ var LibraryEmbindShared = { for (let i = isClassMethodFunc ? 1 : 2; i < argTypes.length; ++i) { const arg = argTypes[i]; let destructorSig = ''; - if (arg.destructorFunction === undefined) { + if (arg.argStackAlloc) { + // Stack-allocated trivial value type: needs no destructor, but the + // invoker must bracket the call in a stack frame. + destructorSig = 's'; + } else if (arg.destructorFunction === undefined) { destructorSig = 'u'; } else if (arg.destructorFunction === null) { destructorSig = 'n'; @@ -209,13 +227,29 @@ var LibraryEmbindShared = { return requiredArgCount; }, - $createJsInvoker__deps: ['$usesDestructorStack', + $createJsInvoker__deps: ['$usesDestructorStack', '$argsUseStackAlloc', #if ASSERTIONS '$checkArgCount', #endif ], $createJsInvoker(argTypes, isClassMethodFunc, returns, isAsync) { var needsDestructorStack = usesDestructorStack(argTypes); + var argsNeedStack = argsUseStackAlloc(argTypes); +#if ASYNCIFY == 1 + // Any call may suspend under Asyncify, and destructors run deferred in + // onDone, after a stack frame would already be gone. + var useStackFrame = false; +#else + // JSPI-async invokers resume after the frame would be gone, so they + // defer through the destructors array instead. + var useStackFrame = argsNeedStack && !isAsync && !needsDestructorStack; +#endif + if (argsNeedStack && !useStackFrame) { + // A stack-allocating type must never see a null destructors argument + // without a bracketing frame; route it through the destructors array + // (it heap-allocates on that path). + needsDestructorStack = true; + } var argCount = argTypes.length - 2; var argsList = []; var argsListWired = ['fn']; @@ -242,9 +276,18 @@ var LibraryEmbindShared = { if (needsDestructorStack) { invokerFnBody += 'var destructors = [];\n'; } + if (useStackFrame) { + // The frame must be released on every completion, including a throwing + // argument conversion or callee: a skipped stackRestore permanently + // leaks wasm stack. `var` declarations hoist out of the try block. + invokerFnBody += 'var sp = stackSave();\ntry {\n'; + } var dtorStack = needsDestructorStack ? 'destructors' : 'null'; var args1 = ['humanName', 'throwBindingError', 'invoker', 'fn', 'runDestructors', 'fromRetWire', 'toClassParamWire']; + if (useStackFrame) { + args1.push('stackSave', 'stackRestore'); + } #if EMSCRIPTEN_TRACING args1.push('Module'); @@ -261,6 +304,11 @@ var LibraryEmbindShared = { } invokerFnBody += (returns || isAsync ? 'var rv = ' : '') + `invoker(${argsListWired});\n`; + if (useStackFrame) { + // The callee has consumed the stack-allocated argument temporaries; + // release the frame before any post-call work. + invokerFnBody += '} finally {\nstackRestore(sp);\n}\n'; + } var returnVal = returns ? 'rv' : ''; #if ASYNCIFY == 1 diff --git a/src/lib/libsigs.js b/src/lib/libsigs.js index 89e5cba52aa9b..601f26850933a 100644 --- a/src/lib/libsigs.js +++ b/src/lib/libsigs.js @@ -320,9 +320,9 @@ sigs = { _embind_register_std_wstring__sig: 'vppp', _embind_register_user_type__sig: 'vpp', _embind_register_user_type_definition__sig: 'vppp', - _embind_register_value_array__sig: 'vpppppp', + _embind_register_value_array__sig: 'vpppppppi', _embind_register_value_array_element__sig: 'vppppppppp', - _embind_register_value_object__sig: 'vpppppp', + _embind_register_value_object__sig: 'vpppppppi', _embind_register_value_object_field__sig: 'vpppppppppp', _embind_register_void__sig: 'vpp', _emscripten_atomic_wait_promise__sig: 'ppid', diff --git a/system/include/emscripten/bind.h b/system/include/emscripten/bind.h index 2416ebd079f35..5c37402d5b4b1 100644 --- a/system/include/emscripten/bind.h +++ b/system/include/emscripten/bind.h @@ -112,7 +112,9 @@ void _embind_register_value_array( const char* constructorSignature, GenericFunction constructor, const char* destructorSignature, - GenericFunction destructor); + GenericFunction destructor, + size_t valueSize, + bool isTrivial); void _embind_register_value_array_element( TYPEID tupleType, @@ -133,7 +135,9 @@ void _embind_register_value_object( const char* constructorSignature, GenericFunction constructor, const char* destructorSignature, - GenericFunction destructor); + GenericFunction destructor, + size_t valueSize, + bool isTrivial); void _embind_register_value_object_field( TYPEID structType, @@ -798,7 +802,14 @@ class value_array : public internal::noncopyable { getSignature(constructor), reinterpret_cast(constructor), getSignature(destructor), - reinterpret_cast(destructor)); + reinterpret_cast(destructor), + sizeof(ClassType), + // Stack temporaries come from stackAlloc, which guarantees + // STACK_ALIGN (== __BIGGEST_ALIGNMENT__) alignment; over-aligned + // types keep the heap path. + std::is_trivially_constructible::value && + std::is_trivially_destructible::value && + alignof(ClassType) <= __BIGGEST_ALIGNMENT__); } ~value_array() { @@ -893,7 +904,14 @@ class value_object : public internal::noncopyable { getSignature(ctor), reinterpret_cast(ctor), getSignature(dtor), - reinterpret_cast(dtor)); + reinterpret_cast(dtor), + sizeof(ClassType), + // Stack temporaries come from stackAlloc, which guarantees + // STACK_ALIGN (== __BIGGEST_ALIGNMENT__) alignment; over-aligned + // types keep the heap path. + std::is_trivially_constructible::value && + std::is_trivially_destructible::value && + alignof(ClassType) <= __BIGGEST_ALIGNMENT__); } ~value_object() {