Stack-allocate trivial value_object/value_array argument temporaries by dimokol · Pull Request #27610 · emscripten-core/emscripten

dimokol · 2026-08-26T09:28:43Z

Addresses the argument-marshaling side of #27553.

When a value type is trivially constructible and destructible and does not require over-alignment (alignof(T) <= STACK_ALIGN), its argument temporaries no longer round-trip through new T() plus destructor bookkeeping. The registration passes sizeof(T) and a triviality flag; toWireType places the temporary on the wasm stack when the invoker brackets the call in stackSave/stackRestore (a null destructors argument is that contract), zero-filled so unregistered fields and padding match the heap path's value-initialization. The bracket restores the frame in a finally, so a throwing argument conversion or callee cannot leak stack. Over-aligned types keep the heap path, as do callers that defer destruction (emval returns, property setters), Asyncify builds, and JSPI-async invokers, which outlive the frame. Field/element writes skip their per-write destructors array when the element type registers no destructor, which turns out to be the dominant source of per-call garbage in large modules (V8 sinks those arrays in small benchmarks but not at scale). The AOT generator mirrors the type shape so invoker signatures stay in sync, and libsigs.js is regenerated for the new registration parameters.

Measured on box3d.js (a real embind physics binding) rebuilt with this branch, forced-GC heapUsed deltas, median over 9 rounds of 20k calls:

call	before	after
getter taking one 3-field value_object id	96 B/call	0
getter with id + value_array vec3 argument	208	16
raycast (id, two vec3, filter in; result object out)	179	37
world step (id + two scalars)	125	61

A stock rebuild with an unpatched toolchain reproduces the before column exactly. The remaining nonzeros are value-type returns, which still materialize fresh objects by design and are out of scope here.

other.test_embind, other.test_embind_aot_js, and other.test_embind_no_dynamic pass locally.

Draft because two things are still missing and I'd like direction before writing them: dedicated tests for the new behavior (a trivial POD value type exercising the stack path, plus an allocation regression check if that fits the suite), and the ChangeLog entry. Note the registration arity change means objects built against an older bind.h need a rebuild.

brendandahl · 2026-08-27T22:38:44Z

This looks good. For testing, add a test in test_other.py around the other embind tests. You should be able to override malloc and free and assert nothing is allocated while testing value_object and value_array.

dimokol · 2026-08-30T07:52:04Z

Done. Added other.test_embind_trivial_value_stack next to the other embind tests, with o2 and aot_js variants: malloc and free are overridden to count, and the trivial value_object/value_array loops assert zero allocations. It also covers zero-init of unregistered fields, a throwing conversion mid-marshal (stack pointer checked stable), the non-trivial control staying on the heap path with balanced allocs and destructors, and the alignas(64) fallback. Built with a stock toolchain the same test fails with 400 allocations, so it does guard the regression. ChangeLog entry added under 6.0.9, with a note that the registration arity change needs a rebuild of old objects. Taking it out of draft.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

brendandahl

Looking good. A few little things.

dimokol · 2026-09-05T05:53:11Z

Addressed all three. All four test variants (default, no_dynamic, aot_js, wasm64) plus test_embind, test_embind_aot_js and test_embind_no_dynamic pass locally.

brendandahl

One last thing on the changelog and I'll get this merged in.

When a value type is trivially constructible and destructible (and alignof(T) <= STACK_ALIGN), its argument temporaries no longer round-trip through new T() plus destructor bookkeeping. The registration passes sizeof(T) and a triviality flag; toWireType places the temporary on the wasm stack when the invoker brackets the call in stackSave/stackRestore (a null destructors argument is that contract), zero-filled so unregistered fields and padding match the heap path's value-initialization. The bracket is a try/finally, so a throwing argument conversion or callee cannot leak stack. Callers that defer destruction (emval returns, property setters) keep the heap path, as do Asyncify builds and JSPI-async invokers, which outlive the frame. Field and element writes skip their per-write destructors array when the element type registers no destructor, the dominant source of per-call garbage in large modules. The AOT generator mirrors the type shape so invoker signatures stay in sync ('s' kind), and libsigs.js is regenerated for the new registration parameters. Note the registration arity change means objects built against an older bind.h need a rebuild.

…der test matrix

…rame

sbc100 · 2026-09-15T18:25:53Z

+            // 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);


We have a zeroMemory helper for this

Missed this. Switching both spots over in a followup

sbc100 · 2026-09-15T18:27:48Z

+            HEAPU8.fill(0, ptr, ptr + valueSize);
+          } else {
+            ptr = rawConstructor();
+            if (destructors !== null) {


Can we just use the truthiness of destructors here and above (rather than comparing explictly with null?)

Sure, it's only ever an array or null. Same followup.

sbc100 · 2026-09-15T18:31:01Z

+  $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) {


Can argTypes[i] ever be null here?

Yeah, for free functions _embind_register_function splices a null into slot 1 for the this type, and craftInvokerFunction checks argTypes[1] !== null to tell methods apart. usesDestructorStack above has the same guard for the same reason.

sbc100 · 2026-09-15T18:34:38Z

+      var rv;
+      // The frame must be released on every completion, including a throwing
+      // argument conversion or callee: a skipped stackRestore permanently
+      // leaks wasm stack.


Is this true? In other places in JS we don't restore the stack on the exception path (see withStackSave, for example). I think the idea is that when an exception happens the module is in an undefined state, no need to restore the sp in this case I think?

So maybe this try/catch can be removed?

I think it's worth keeping. The case it covers is a JS getter throwing during argument conversion, after stackAlloc already ran for that argument and with every wasm call so far returned cleanly. I tried it with the restore on the normal path only and the 1000-iteration getter loop in the new test leaks 16000 bytes. It's the same story for a C++ exception escaping to JS under wasm EH (a small probe leaks 32 bytes per throw) and the exceptions docs recommend exactly this stackSave/stackRestore around the catch for that (https://emscripten.org/docs/porting/exceptions.html#handling-c-exceptions-from-javascript)

withStackSave is probably fine promising less since its callers are runtime internals, but here the throws are things callers catch and carry on from, so the leak adds up.

But wouldn't anyone actually catching a C exception from JS need to restore the SP themselves? This restore might work for the leaf function but what if there is other LLVM stuff on the stack when the exception is thrown.

IIUC it should be up to the catcher to restore the stack since LLVM stack frames don't restore as they unwind.

Fair, for a C++ exception the catcher has to restore anyway since the frames between don't. The case I care about is a plain JS throw during argument conversion (a getter throwing, or a later argument failing after an earlier one already took its frame) where no wasm frame is unwinding at all. Before this change an embind call never moved the stack pointer, so a JS-side throw couldn't move it either, and a JS caller has no reason to wrap an embind call in stackSave/stackRestore. The finally keeps that property, without it the getter loop in the test drifts by one frame per throw (16 bytes for the test's type). Happy to narrow the comment to say that's what it's for.

But your try/catch in the test, is wrapping native call Module['sumVec']. Any caller what is catching exceptions coming from a calling the Wasm module will need to restore the stack point if they want to continue to use the module and also avoid leakes.

Imagine for example that sumVec itself used some stack space and then trapped. IIUC the only safe thing to do is assume that stack space is leaked whenever an exception comes out of the Module.

The corollary of that is that code within the module should not need to worry itself about restoring the stack pointer when exceptions are thrown. Even trying to make a best effort to do this in some cases I think just muddies the water.

Agreed on the rule. Once wasm has run, whoever catches owns the stack pointer, and embind shouldn't half-promise cleanup for exceptions coming out of the module. I'll drop the try/finally around the call.

The one case I'd still like to cover is different from that. A conversion error (missing field, or a getter throwing) happens in embind's own JS before the C++ function is entered, so from the caller's side no module code ran and there's nothing they'd know to restore. Before this change an embind call never moved the stack pointer, so that throw couldn't leak, and it's the case the getter loop in the test measures (16 bytes per throw without a restore). I could restore in a catch around the argument conversion only and leave the call itself as you describe, which keeps the rule intact, exceptions out of the module are the caller's, exceptions before it never touch the stack.

If you'd rather keep embind out of it entirely, I'll drop that too and take the throwing section out of the test.

I'm not sure we can/should consider conversion errors that happen at the boundary any differencly to wasm calls themselves. i.e. anyone wanting to recover from such error should really be doing stackRestore, just in case.

This is how I see it: As a general rule, if you call into emscripten generated code (either native wasm code, or JS library code, or embind wrapper code, it doesn't matter) and you want to recover from a thrown exception you should always handle the stack restoration before calling back in to the module code.

Emscripten does not, as a rule, take care of the shadow stack restoration during exception unwinding. Its up to each try/catch to handle that at the catch site.

Since the documenation was not completely clear on this I created #27748. I think this accurately reflects the reality.

Makes sense, and the docs change settles it. I took the try/finally out and restore on the normal path only, and dropped the throwing section of the test: #27750

Follow-ups for the review comments sbc100 left on #27610 after it merged: zeroMemory instead of HEAPU8.fill for the stack temporaries, truthiness checks on destructors, and my AUTHORS entry. The try/finally question is answered on the original thread.

dimokol mentioned this pull request Aug 26, 2026

embind: value_object/value_array arguments allocate on every call, and there is no fill-in-place read path #27553

Open

dimokol marked this pull request as ready for review August 30, 2026 07:52

Copilot AI lite review requested due to automatic review settings August 30, 2026 07:52

Copilot AI reviewed Aug 30, 2026

dimokol force-pushed the embind-trivial-value-stack branch from 801dfa4 to 1a0f62c Compare August 30, 2026 07:55

brendandahl requested changes Sep 3, 2026

View reviewed changes

Comment thread test/test_other.py

Comment thread src/lib/libembind.js

Comment thread system/include/emscripten/bind.h Outdated

dimokol force-pushed the embind-trivial-value-stack branch from 067cffc to 660e695 Compare September 10, 2026 07:20

dimokol requested a review from brendandahl September 11, 2026 09:02

brendandahl approved these changes Sep 11, 2026

View reviewed changes

Comment thread ChangeLog.md

dimokol added 5 commits September 13, 2026 13:17

Add tests and a ChangeLog entry for stack-allocated trivial value types

8c8a0cf

Address review: shared triviality helper, matching write closures, wi…

a1dd222

…der test matrix

Rebaseline hello_embind codesize expectations for the invoker stack f…

3a1a33b

…rame

Move the embind changelog entry into the in-development section

86e6424

dimokol force-pushed the embind-trivial-value-stack branch from 660e695 to 86e6424 Compare September 13, 2026 10:18

brendandahl merged commit cfb87fa into emscripten-core:main Sep 15, 2026
42 checks passed

sbc100 reviewed Sep 15, 2026

View reviewed changes

dimokol mentioned this pull request Sep 16, 2026

[embind] Post-merge cleanups from #27610 #27722

Merged

sbc100 mentioned this pull request Sep 18, 2026

[docs] Clarify stack restoration when catching exceptions from JS #27748

Open

Conversation

dimokol commented Aug 26, 2026

Uh oh!

brendandahl commented Aug 27, 2026

Uh oh!

dimokol commented Aug 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

brendandahl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dimokol commented Sep 5, 2026

Uh oh!

brendandahl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbc100 Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sbc100 Sep 17, 2026 •

edited

Loading