Stack frames up to 113 KB against the 64 KB wasm stack; no build uses -Wframe-larger-than
- Dominant language
- C
- Stars
- 268
- Forks
- 18
- Avg merge
- 2h 27m
- Merged PRs (30d)
- 447
Description
Found in a full-repo review at `0ba280f06f`.
Compiling `src/*.c` with the build's own flags plus `-Wframe-larger-than=4096` reports stack frames up to 113 KB. The WebAssembly build sets no `-sSTACK_SIZE` anywhere in `ext/wasm/`, so it gets emscripten's 64 KB default — which `ext/wasm/GNUmakefile:900` already documents as a hazard:
> ACHTUNG: emsdk 3.1.27 reduced the default stack size from 5MB to a mere 64KB, which leads to silent memory corruption via the kvvfs VFS
The wasm build compiles the amalgamation, which contains all of these functions.
| bytes | function | site |
|---|---|---|
| **113,344** | `normalizeSideToMergedLayout` | `src/doltlite_merge_schema.c:1210` |
| **66,576** | `gcWriteCompactedTo` | `src/doltlite_gc.c:677` |
| 32,960 | `statusSequenceTrackedEqualOneWay` | `src/doltlite_status.c:119` |
| 32,880 | `detectFkViolationsForSpec` | `src/doltlite_merge_constraints_fk.c:264` |
| 32,352 | `doltliteBuildIndexEntry` | `src/doltlite_merge_rows.c:374` |
| 32,336 | `advanceToNextRow` | `src/doltlite_diff_table.c:1033` |
| 32,176 | `dsCountModifiedRow` | `src/doltlite_diff_stat.c:244` |
| 32,144 | `diffRecordsEqualFieldwise` | `src/prolly_diff.c:147` |
| 32,112 | `recordPrefixEquals` | `src/doltlite_merge_constraints.c:351` |
| 16,960 | `mergeSplitWorkingCatalog` | `src/doltlite_merge_cmd.c:299` |
| 16,720 | `detectUniqueViolationsForIndexWithoutRowid` | `src/doltlite_merge_constraints_unique.c:530` |
| 16,608 | `loadSchemaFromCatalogMode` | `src/doltlite_schema_diff.c:109` |
| 16,576 | `masterCollectViewTriggerRows` | `src/doltlite_diff.c:48` |
| 16,560 | `loadSchemaEntryFromCatalog` | `src/doltlite_schema_diff.c:300` |
| 16,544 | `loadSchemaCatalogRows` | `src/prolly_btree_catalog.c:729` |
| 16,544 | `checkoutLoadSourceTableSql` | `src/doltlite_checkout.c:67` |
| 16,496 | `statusSchemaHasViewOrTrigger` / `schemaHasAnyViewOrTrigger` | `src/doltlite_status.c:329`, `src/doltlite_diff.c:138` |
| 16,208 | `doltlitePartialIndexMatchesRecord` | `src/doltlite_merge_constraints_unique.c:142` |
| 16,112 | `resultUserCol` | `src/doltlite_record.c:593` |
| 16,096 | `bmColumn` | `src/doltlite_blame.c:974` |
| 16,080 | `patchGetValue` | `src/doltlite_patch.c:937` |
| 15,648 | `diffIterPrefetchPairs` | `src/prolly_diff.c:719` |
| 14,032 | `doltliteSyncChunks` | `src/doltlite_remote.c:269` |
| 10,576 | `prollyDiffIterStep` | `src/prolly_diff.c:1024` |
| 8,560 | `compareCatalogs` | `src/doltlite_status.c:956` |
| 6,384 | `serverWorkerEntry` | `src/doltlite_remotesrv.c:1244` |
| 5,328 | `csReplayWalFrom` | `src/chunk_wal.c:655` |
| 5,280 | `csOriginGetMany` | `src/doltlite_chunk_source.c:67` |
| 4,304 | `csWriteWalCheckpoint` | `src/chunk_wal.c:354` |
`normalizeSideToMergedLayout` alone overflows the whole wasm stack 1.8x on a schema-changing merge; `gcWriteCompactedTo` does so on `dolt_gc`.
Two contributing causes:
1. **`DoltliteRecordInfo` is 16,004 bytes** (`src/prolly_record.h:83-87`: `4 + 2 × SQLITE_MAX_COLUMN × 4`) and lives on the stack. Every 16 KB / 32 KB entry above is one or two of these.
2. **The new diff prefetch allocates 10 KB before checking whether it is needed.** `diffIterPrefetchFrame` (`src/prolly_diff.c:802`) declares `DiffPrefetchPair aPair[256]` and is inlined into `prollyDiffIterStep`; it tail-calls `diffIterPrefetchPairs` (`:719`, another `aPair[256]` plus `aHash[256]`) passing that array, so both frames are live together — **26,224 bytes, 41% of the wasm stack, in two frames**. Both declare their arrays *before* the `if( !pIter->pStore->pChunkSource ) return SQLITE_OK;` guard, so a purely local database pays the full 26 KB on every diff step even though prefetch is a no-op there.
This is the same defect class as #2567 / #2568 / #2570, where one silent stack overflow produced three unrelated-looking wasm bugs.
## Fix
- Add `-Wframe-larger-than` to the build so this cannot recur silently. It is currently used nowhere in the repo, and it is what finds this class first.
- Set an explicit `-sSTACK_SIZE` for the wasm build rather than inheriting emscripten's default.
- Heap-allocate the diff prefetch buffer once per `ProllyDiffIter`, or move it behind the chunk-source check.
- Shrink `DoltliteRecordInfo` to a small inline array with heap spill, which removes most of the table at once.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by reproducing the report with the build's flags plus -Wframe-larger-than=4096, then read ext/wasm/GNUmakefile:900, src/prolly_diff.c:719 and :802, and src/prolly_record.h:83-87. Trace the listed large-frame functions and wasm build settings. Done means frame warnings are enabled, the wasm stack is explicit, and the identified stack allocations no longer overflow it.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, sqlite, wasm
- Domain
- build-system, databases, performance
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 38/100