[browser] Turn the >2GB memory test into an interop conformance test#131360
Open
lewing wants to merge 1 commit into
Open
[browser] Turn the >2GB memory test into an interop conformance test#131360lewing wants to merge 1 commit into
lewing wants to merge 1 commit into
Conversation
The wasm/JS boundary does not preserve signedness: a wasm i32 arrives in JavaScript as a signed number, so every pointer above 2GB shows up negative. Missing coercions mostly corrupt data silently (a negative subarray start is clamped relative to the end of the heap) rather than throwing, so the previous scenario - which only checked that a string JSImport kept working - could not detect them. Exercise the interop surface with data integrity assertions while all runtime allocations live above the 2GB boundary: strings, byte[]/int[]/double[] in both directions, memory views, JSObject, exceptions, promises, the JSExport direction, crypto and globalization. Also assert that the jiterpreter does not report code generation failures, which is how invalid pointer encoding shows up in the trace compiler. Verified against 10.0.9 and 11.0.0-preview.6 runtime packs with 2.2GB and 3.6GB of heap wasted before startup: 10.0.9 fails the crypto check (the buffer stays zeroed) and logs jiterpreter code generation failures, 11.0 passes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8b51d9f7-816f-483e-b4cc-ec6e2dc45a6a
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens the existing “>2GB heap” wasm scenario by turning it into a more comprehensive interop conformance test, exercising multiple managed↔JS marshalling paths while allocations are intended to occur above the 2GB boundary.
Changes:
- Extend the WasmBasicTestApp
MemoryTestscenario into an async test suite covering strings, arrays, memory views, JSObject interaction, exceptions, tasks, RNG, and globalization. - Add richer JS-side imports/exports in
main.jsfor validating both JSImport and JSExport integrity. - Update the build test to assert the scenario actually completed successfully and that no jiterpreter “code generation failed” messages appeared.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/mono/wasm/Wasm.Build.Tests/MemoryTests.cs | Validates the scenario ran to completion and checks console output for jiterpreter compilation failures. |
| src/mono/wasm/testassets/WasmBasicTestApp/App/wwwroot/main.js | Expands module imports and adds JS-driven calls into managed exports for additional coverage. |
| src/mono/wasm/testassets/WasmBasicTestApp/App/MemoryTest.cs | Rewrites the scenario into a multi-check conformance suite (including new JSExport surface for the test). |
Comment on lines
139
to
142
| try | ||
| { | ||
| arrayHolder[i] = new int[1024 * 1024 * 25]; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rewrites the
AllocateLargeHeapThenInteropscenario into an actual conformance test for interop above the 2GB boundary.Why
The wasm/JS boundary does not preserve signedness. A wasm
i32arrives in JavaScript as a signed number, so every pointer above 2GB shows up negative. The failure modes are mostly silent — a negativesubarraystart is clamped relative to the end of the heap, a negativeHEAPU8index yieldsundefined— so a scenario that only checks "a stringJSImportstill returns the right length" cannot detect them.What
The scenario now asserts data integrity across the interop surface while every runtime allocation lives above 2GB:
byte[]/int[]/double[]in both directionsSpan<byte>andArraySegment<byte>memory viewsJSObjectproperties, exception propagation,Task<int>JSExportdirection (JS calling into managed withbyte[]/stringpayloads)RandomNumberGenerator.Filland globalizationIt also reports the address of a pinned buffer and fails if allocations did not actually land above 2GB, so the test cannot silently stop testing what it is named after. The
MemoryTestsWBT additionally asserts that the run produced no jiterpretercode generation failedmessages, which is how invalid pointer encoding surfaces in the trace compiler.Validation
Ran the same test app against the shipped runtime packs with
EmccMaximumHeapSize=4294901760and 2.2GB / 3.6GB of heap wasted before startup:RandomNumberGenerator.Fillsilently returns an all-zero buffer; jiterpreter logscode generation failed: CompileError … extra bits in varintSo the new assertions catch the pre-#121221 behaviour rather than just passing everywhere.
Note
This pull request was authored with GitHub Copilot.