|
| 1 | +# Stock-`xst` A/B — flat/flatMap value-stack overflow before/after `73aad47b` |
| 2 | + |
| 3 | +This harness answers mhofman's follow-up on kriskowal/garden#9 |
| 4 | +([comment 4907678857](https://github.com/kriskowal/garden/issues/9#issuecomment-4907678857)): |
| 5 | + |
| 6 | +> Can you verify the stack overflow using `xst` instead of our worker? You should |
| 7 | +> be able to use release `xst`, from before and after `73aad47b` to confirm. If |
| 8 | +> the pop at the end of the block is indeed the more correct place, Moddable needs |
| 9 | +> a test case to reproduce the issue against the latest release. |
| 10 | +
|
| 11 | +Everything here runs on the **stock upstream `xst`** (the standalone XS shell) built |
| 12 | +straight from tagged **Moddable-OpenSource/moddable releases** — no agoric fork, no |
| 13 | +xsnap worker, no patches. It complements `../engine-flatmap-ab/` (which A/B's the |
| 14 | +three `mxPop()` placements on the agoric xsnap worker at the on-chain |
| 15 | +`stackCount=4096`); this one confirms the same behavior on the canonical engine |
| 16 | +release binaries, at `xst`'s own default 256K-slot value stack. |
| 17 | + |
| 18 | +## The release window |
| 19 | + |
| 20 | +[`Moddable-OpenSource/moddable@73aad47b`](https://github.com/Moddable-OpenSource/moddable/commit/73aad47b3eb5f5f13baf401bd28d1609c14f23ab) |
| 21 | +("XS: fix array flat/sort stack overflows") landed **2026-01-20**. |
| 22 | + |
| 23 | +| release | date | contains `73aad47b`? | `flatAux` leaf branch | |
| 24 | +| --- | --- | --- | --- | |
| 25 | +| **7.0.0** | 2026-01-16 | **no** (BEFORE) | `mxDefineIndex; start++` — no pop | |
| 26 | +| **7.1.0** | 2026-02-11 | yes (first AFTER) | `mxDefineIndex; mxPop(); start++` | |
| 27 | +| **8.3.0** | 2026-07-03 | yes (LATEST) | `mxDefineIndex; mxPop(); start++` | |
| 28 | + |
| 29 | +Confirmed with `gh api .../compare/73aad47b...<tag>`: 7.0.0 is *behind* the commit |
| 30 | +(does not contain it), 7.1.0 and 8.3.0 are *ahead* (contain it). |
| 31 | + |
| 32 | +## The two branches of `fx_Array_prototype_flatAux` |
| 33 | + |
| 34 | +`fx_Array_prototype_flatAux` (`xs/sources/xsArray.c`) builds the flattened result on |
| 35 | +the heap but, in stock XS, leaves per-iteration slots resident on the **value |
| 36 | +stack**, so peak use is O(flattened output), not O(depth). Two leak sites: |
| 37 | + |
| 38 | +- **LEAF branch** — `item` defined into the result via `mxDefineIndex`; `fxDefineAll` |
| 39 | + pops only the result-array reference, leaving the value below it. `73aad47b` adds |
| 40 | + `mxPop()` here. This is the ymax0 `hex.js` class (`new Map(RI.flatMap(...))`, |
| 41 | + leaf-dominated). |
| 42 | +- **NESTED branch** — `item = the->stack` (the sub-array), pushed before descending; |
| 43 | + after the recursion returns it is **never popped**. `73aad47b` does **not** touch |
| 44 | + this branch, so it still leaks one slot per nested element on the latest release. |
| 45 | + kriscendobot/moddable#1 moves the pop to the **end of the `if (fxHasIndex)` block**, |
| 46 | + covering both branches. |
| 47 | + |
| 48 | +## Build the three release `xst`s |
| 49 | + |
| 50 | +```sh |
| 51 | +for tag in 7.0.0 7.1.0 8.3.0; do ./build-xst.sh $tag; done |
| 52 | +# each prints .../src/moddable-<tag>/build/bin/lin/release/xst |
| 53 | +``` |
| 54 | + |
| 55 | +## Run the A/B |
| 56 | + |
| 57 | +```sh |
| 58 | +XST() { echo ~/.cache/garden-scratch/xst-ab/src/moddable-$1/build/bin/lin/release/xst; } |
| 59 | +# LEAF (ymax0 hex.js class) — fast: |
| 60 | +for tag in 7.0.0 7.1.0 8.3.0; do ./run-ab.sh leaf 300000 "$(XST $tag)"; done |
| 61 | +# NESTED (the branch 73aad47b leaves resident) — SLOW at 256K stack (see note): |
| 62 | +for tag in 7.0.0 7.1.0 8.3.0; do ./run-ab.sh nested 300000 "$(XST $tag)" 600; done |
| 63 | +``` |
| 64 | + |
| 65 | +## Verified results (2026-07-07, x86-64 linux, GOAL=release) |
| 66 | + |
| 67 | +Stock `xst` runs a script file with a **256K-slot** value stack |
| 68 | +(`xs/tools/xst.c` `_creation.stackCount = 256*1024`), so thresholds are ~256K |
| 69 | +elements — far above the on-chain xsnap 4096, but the **same leak**. A value-stack |
| 70 | +exhaustion surfaces as `Error: JavaScript stack overflow` (exit 1). |
| 71 | + |
| 72 | +### LEAF — `[inner].flat()`, inner = K scalars (the ymax0 `flatMap` class) |
| 73 | + |
| 74 | +| K | 7.0.0 (before) | 7.1.0 (after) | 8.3.0 (latest) | |
| 75 | +| --- | --- | --- | --- | |
| 76 | +| 300000 | **STACK OVERFLOW** | OK | OK | |
| 77 | + |
| 78 | +Fast (< 1 s): the leaf loop allocates almost nothing, so it fills the stack in one |
| 79 | +near-linear pass. **`73aad47b` fixes the ymax0-class (leaf) overflow on release |
| 80 | +`xst`, before → overflow, after → clean.** This is the decisive "verify with `xst`, |
| 81 | +before and after" result. |
| 82 | + |
| 83 | +### NESTED — `a = [inner,inner,…] (N×); a.flat(1)`, every element an array |
| 84 | + |
| 85 | +| N | 7.0.0 (before) | 7.1.0 (after) | 8.3.0 (latest) | |
| 86 | +| --- | --- | --- | --- | |
| 87 | +| 130000 | OK (42 s) | — | — | |
| 88 | +| 160000 | **STACK OVERFLOW** (76 s) | — | — | |
| 89 | +| 300000 | **STACK OVERFLOW** | **STACK OVERFLOW** (229 s) | **STACK OVERFLOW** (218 s) | |
| 90 | + |
| 91 | +Before the fix the nested case leaks **two** slots per element (leaf + sub-array), so |
| 92 | +it overflows at N ≈ 150K. After `73aad47b` the leaf slot is popped but the sub-array |
| 93 | +slot is **not**, so the leak halves (one slot/element) and the overflow simply moves |
| 94 | +out to N ≈ 256K — **it does not go away on the latest release.** That is the case |
| 95 | +`73aad47b` leaves open and kriscendobot/moddable#1's end-of-block pop closes. |
| 96 | + |
| 97 | +> **Timing note.** At the 256K stock stack the nested overflow is O(N²) to reach — |
| 98 | +> XS's GC rescans the growing value stack on every collection — so each nested run |
| 99 | +> is minutes, not seconds. This is a property of the large default stack, not the |
| 100 | +> bug: at the on-chain `stackCount=4096` the identical nested overflow trips at |
| 101 | +> N ≈ 4200 in well under a second (`../engine-flatmap-ab/README.md`). |
| 102 | +
|
| 103 | +## Test cases for Moddable |
| 104 | + |
| 105 | +- [`testcase-flat-leaf-overflow.js`](./testcase-flat-leaf-overflow.js) — reproduces |
| 106 | + the overflow `73aad47b` fixed; a regression guard for the leaf pop (overflows |
| 107 | + ≤ 7.0.0, passes ≥ 7.1.0). |
| 108 | +- [`testcase-flat-nested-overflow.js`](./testcase-flat-nested-overflow.js) — **the |
| 109 | + one mhofman asked for**: still overflows on the **latest** release `xst` (8.3.0), |
| 110 | + because 73aad47b's leaf-only pop leaves the nested branch resident. Passes only |
| 111 | + with kriscendobot/moddable#1's end-of-block pop. Runs in minutes at the 256K stock |
| 112 | + stack; Moddable's harness can create the machine with a small `stackCount` (e.g. |
| 113 | + 4096) to trip it in well under a second. |
| 114 | + |
| 115 | +## Conclusion |
| 116 | + |
| 117 | +1. **Verified on release `xst`, before and after `73aad47b`:** 7.0.0 overflows on the |
| 118 | + ymax0 (leaf) class, 7.1.0 and 8.3.0 clear it. The cherry-pick is confirmed on the |
| 119 | + canonical engine, not just the agoric worker. |
| 120 | +2. **The end-of-block pop is the more correct place, confirmed on release `xst`:** the |
| 121 | + nested branch still leaks on the latest release (8.3.0) — its overflow only moves |
| 122 | + out from N ≈ 150K to N ≈ 256K, it does not disappear. `testcase-flat-nested-overflow.js` |
| 123 | + is the reproduction against the latest release Moddable can adopt. |
| 124 | + |
| 125 | +Scope: read-only analysis + on-host builds/runs of the open-source XS engine from |
| 126 | +public Moddable release tarballs. No upstream `Moddable-OpenSource/moddable` or |
| 127 | +`agoric/agoric-sdk` interaction. |
0 commit comments