Fix Windows/MSVC handling of large WAT input files and increase stack reserve#2748
Fix Windows/MSVC handling of large WAT input files and increase stack reserve#2748aydgelee wants to merge 1 commit into
Conversation
|
For the 64-bit file ops can we do something simpler like? For the stack size do you know why we would need such a high value? IIUC the 8mb limit on linux seems to be enough there. |
|
For the file APIs, I agree the goal is simply to use the 64-bit MSVC CRT variants. I avoided globally redefining For the stack size, one clarification: The reason this is included here is that, after the 64-bit file I/O fix, the large WAT test no longer failed with on Windows. MSVC-linked executables typically default to a 1MB stack reserve, while Linux/macOS environments commonly have larger or configurable stack limits, for example an 8MB soft stack limit on many Linux systems. That likely explains why the same workload can pass on Linux/macOS but fail on Windows. So the larger /STACK value is intended to raise the Windows stack reserve ceiling for very large WAT parsing workloads. It does not mean the process commits that amount of memory at startup. |
177c0d9 to
e5e08dc
Compare
|
CI only reported a clang-format issue. No behavior change. Applied the suggested formatting diff. |
| target_link_options(wasm2wat PRIVATE "/STACK:536870912") | ||
| endif () | ||
| if (TARGET wat2wasm) | ||
| target_link_options(wat2wasm PRIVATE "/STACK:536870912") |
There was a problem hiding this comment.
If we need a bigger stack on windows can we just go with 8mb (to match the linux default).
Also, can we just include this in wabt_executable so that it applies to all binaries? We might as well be consistent.
There was a problem hiding this comment.
Done. I moved the MSVC stack reserve into wabt_executable so it applies consistently to all WABT executables, and changed it to /STACK:8388608.
Verified on Windows x64 with a 2,978,116,509-byte WAT file. wat2wasm completed with exit code 0 and generated a 92,039,608-byte wasm output.
I'd much rather do it by via redefining these macros, keeping the the code otherwise the same on all platforms. There really aren't very many file I/O codepaths in wabt, and I imagine we want to be able to handle large files in all of them. |
Use MSVC macro redirection for stat, fseek, and ftell so the ReadFile path stays shared across platforms. Move the MSVC stack reserve setting into the common wabt_executable helper and reduce it to 8MB.
e5e08dc to
180b6bc
Compare
|
Updated.
Windows x64 verification:
No "value too large", no "unable to read file", and no STATUS_STACK_OVERFLOW with the 8MB stack reserve. |
| } | ||
|
|
||
| static int SeekFile(FILE* file, size_t offset) { | ||
| if (offset > static_cast<size_t>(std::numeric_limits<int64_t>::max())) { |
There was a problem hiding this comment.
If this SeekFile really needed?
We are never going be seeing outside of int64_t range right? If we did that would fail on all platforms, no just windows.
In fact, fseek on non-windows is limited to long which can be 32-bit on all systems right?
Maybe just add this check for data.size() <= LONG_MAX before the call tofseek` rather than adding a separate function.
|
Why do we need to read and write the files in chunks on windows? But not on other platforms? Could you update the PR description to include that maybe? |
|
Are you seeing these error on 64-bit windows too? Or is this mostly a fix for 32-bit windows? |
Fix Windows/MSVC handling of large WAT input files and increase stack reserve
Summary
This PR fixes Windows/MSVC handling of very large WAT input files in WABT tools, mainly
wat2wasm.exeandwasm2wat.exe.On Windows,
wat2wasmcan fail when reading WAT files larger than 2GB with errors such as:The failure is caused by the MSVC file-reading path using APIs and types that are not safe for large files.
After fixing the large-file read path, processing very large WAT files can also trigger a Windows stack overflow during parsing:
This PR addresses both issues for Windows/MSVC builds while keeping non-Windows behavior unchanged.
Changes
Windows/MSVC large-file reading
In
src/common.cc, the Windows/MSVC file-reading path is updated to use 64-bit file APIs and file-size types:The MSVC path also performs size checks before resizing buffers and reads large files in chunks.
Windows/MSVC stack reserve
In
CMakeLists.txt, Windows/MSVC builds now set a larger stack reserve for both tools:536870912bytes equals 512MB.This is only applied for MSVC builds.
Non-Windows behavior preserved
Linux and macOS code paths remain unchanged. The large-file API changes are limited to
COMPILER_IS_MSVC/MSVCbranches.No WebAssembly parser semantics are changed.
Validation
Validation was performed on Windows x64 using a WAT file larger than 2GB.
Baseline behavior
Using the baseline code, even with a larger stack reserve,
wat2wasmstill failed during file reading:This confirms that increasing stack size alone does not fix the issue. The file-reading path must be updated.
Large-file IO fix without larger stack
After updating the Windows/MSVC file-reading path,
wat2wasmno longer failed withvalue too large, but processing the large WAT file exited with:This confirms that the large-file reading fix works, and the next failure point is parser stack usage.
Full fix
With both changes applied:
wasm2watandwat2wasmThe round-trip conversion succeeds:
Result:
Impact
Windows/MSVC
Windows builds can now handle WAT files larger than 2GB in the tested scenario.
Affected tools:
Linux/macOS
Linux and macOS behavior is not changed.
The non-MSVC file-reading path remains the original WABT behavior, and
/STACKis only applied underMSVC.Build instructions
Expected output files:
Verification commands
Generate WAT from wasm
Expected result:
Convert WAT back to wasm
Expected result:
Notes for reviewers
/STACK:536870912option is only applied underMSVC.