[release/10.0][browser][mono] Make sure we treat the 4GB ptrs as unsigned#131361
Draft
lewing wants to merge 2 commits into
Draft
[release/10.0][browser][mono] Make sure we treat the 4GB ptrs as unsigned#131361lewing wants to merge 2 commits into
lewing wants to merge 2 commits into
Conversation
Backport of #121221 (f813007) to release/10.0. The wasm/JS boundary does not preserve signedness: a wasm i32 arrives in JS as a signed number, so pointers above 2GB show up negative and silently corrupt data. This coerces those pointers back to unsigned across the mono browser runtime. The upstream commit also touched src/native/libs/System.Native.Browser (CoreCLR-on-wasm), which does not exist in release/10.0; those files are excluded from this backport. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d0b1c48-73de-4531-aeff-49ed82385c20
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
There was a problem hiding this comment.
Pull request overview
Backports the >2GB (4GB address space) pointer signedness fixes for the Mono browser/WASM JavaScript runtime to release/10.0, and strengthens validation by turning the large-heap scenario into an interop conformance test that checks data integrity (not just “no exceptions”) above the 2GB boundary.
Changes:
- Coerce pointer/offset values at the JS/wasm boundary to unsigned (via
>>> 0/fixupPointer) across key runtime subsystems (strings, websockets, debugger, diagnostics, startup, marshaling, and jiterpreter). - Update the jiterpreter wasm generation paths to avoid emitting invalid encodings when pointer-like values appear negative in JS.
- Expand the
AllocateLargeHeapThenInteroptest scenario into a richer interop conformance run and assert success + absence of jiterpreter codegen failures.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/mono/wasm/Wasm.Build.Tests/MemoryTests.cs | Strengthens the large-heap test by asserting success output and no jiterpreter “code generation failed” messages. |
| src/mono/wasm/testassets/WasmBasicTestApp/App/wwwroot/main.js | Adds JS imports/exports used by the expanded interop conformance test and invokes the new async managed entrypoint. |
| src/mono/wasm/testassets/WasmBasicTestApp/App/MemoryTest.cs | Reworks the scenario into a conformance-style test covering many interop surfaces above 2GB. |
| src/mono/browser/runtime/web-socket.ts | Fixes buffer/view creation and stored pointers to treat offsets as unsigned. |
| src/mono/browser/runtime/strings.ts | Ensures string decoding/copying paths treat heap indices/pointers as unsigned. |
| src/mono/browser/runtime/startup.ts | Fixes assembly/pdb byte view creation to use unsigned offsets. |
| src/mono/browser/runtime/roots.ts | Ensures root buffer/external root addresses are treated as unsigned offsets in JS. |
| src/mono/browser/runtime/memory.ts | Adds fixupPointer and applies unsigned coercion for byte-indexed heap reads/writes and zeroing. |
| src/mono/browser/runtime/marshal.ts | Fixes pointer extraction and memory-view handling to use unsigned pointer values. |
| src/mono/browser/runtime/jiterpreter.ts | Coerces key pointer-like values to unsigned and updates builder usage. |
| src/mono/browser/runtime/jiterpreter-trace-generator.ts | Uses pointer-safe constant emission and unsigned offsets where required for wasm encoding correctness. |
| src/mono/browser/runtime/jiterpreter-support.ts | Removes constant-slot machinery and emits pointer constants in a way compatible with signed wasm immediates while fixing unsigned encodings where needed. |
| src/mono/browser/runtime/jiterpreter-jit-call.ts | Coerces pointer-like trampoline args to unsigned before use. |
| src/mono/browser/runtime/jiterpreter-interp-entry.ts | Ensures imethod/pointer-like values are handled as unsigned. |
| src/mono/browser/runtime/diagnostics/index.ts | Fixes diagnostic message buffer views to use unsigned offsets. |
| src/mono/browser/runtime/diagnostics/common.ts | Fixes writes back into HEAPU8 to use unsigned offsets. |
| src/mono/browser/runtime/debug.ts | Fixes debugger message buffer views to use unsigned offsets. |
| src/mono/browser/runtime/crypto.ts | Coerces entropy buffer pointer to unsigned before slicing heap views. |
Comment on lines
140
to
142
| { | ||
| arrayHolder[i] = new int[1024 * 1024 * 25]; | ||
| } |
Contributor
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
Open
3 tasks
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.
Fixes Issue #58382
main PR #121221
Description
The wasm/JS boundary does not preserve signedness — a wasm
i32arrives in JavaScript as a signed number, so every pointer above the 2GB mark shows up negative. Most of the resulting damage is silent rather than a crash: a negativesubarraystart is clamped relative to the end of the heap, and a negativeHEAPU8index yieldsundefined.This is a backport of #121221, which coerces the pointers to unsigned at the JavaScript boundary: crypto buffers, debugger buffers, WebSocket buffers, event-pipe buffers, jiterpreter traces, JS
MemoryView/Span, the various memory JS APIs, asset loading and string marshaling.Relative to the original PR, the two
src/native/libs/System.Native.Browser/*files are dropped — that is CoreCLR-on-wasm, which does not exist on this branch. Everything undersrc/mono/browser/runtime/is taken as-is; the only textual adaptation is incrypto.ts, where the function is still namedmono_wasm_browser_entropyhere.This also brings over the test from #131360 so the behaviour is covered going forward.
The two predecessor fixes in this area, #104662 and #109079, are already in release/10.0, and #124633 is CoreCLR-on-wasm only, so #121221 is the remaining piece.
Customer Impact
Requested by a customer who needs a mono wasm heap larger than 2GB on .NET 10.
Without this fix, an app configured with
<EmccMaximumHeapSize>above 2GB corrupts data silently once allocations cross the boundary. Measured on the shipped 10.0.9 runtime pack with 2.2GB of heap in use,RandomNumberGenerator.Fillreturns an all-zero buffer with no error — a security-relevant failure — and the jiterpreter emits invalid wasm modules (code generation failed: CompileError … extra bits in varint), silently losing tier-up. The remaining affected areas (debugger, WebSocket, event-pipe, asset loading, memory views) fail in similarly quiet ways.Support above 2GB stays opt-in and "use at your own risk", but today it is quietly wrong rather than unsupported.
Regression
No. This has never worked; it is a long-standing limitation tracked by #58382.
Testing
Measured against the shipped runtime packs using a
Microsoft.NET.Sdk.WebAssemblyapp withEmccMaximumHeapSize=4294901760,WasmBuildNative=true, and 2.2GB / 3.6GB of heap wasted inpreRunbefore the runtime starts:RandomNumberGenerator.Fillsilently returns an all-zero buffer; jiterpreter logscode generation faileddotnet.runtime.jsThe included
MemoryTestsscenario asserts data integrity — not just the absence of exceptions — across strings,byte[]/int[]/double[]in both directions, memory views,JSObject, exceptions,Task<int>, theJSExportdirection, crypto and globalization, with every allocation above 2GB. It also fails if allocations did not actually land above 2GB, and asserts the run produced no jiterpreter code generation failures.Risk
Low. The change is confined to the browser mono runtime JavaScript and is inert below 2GB: the added
>>> 0/fixupPointercoercions are no-ops for pointers that already fit in a signed 32-bit integer, which is every pointer in the default (2GB-capped) configuration. Reaching a heap above 2GB additionally requires<EmccMaximumHeapSize>plus a native relink via thewasm-toolsworkload, so the affected configuration is opt-in.The one behavioural change beyond coercion is in the jiterpreter, where support for the unused
constantSlotsfeature was dropped rather than fixed.Note
This pull request was authored with GitHub Copilot.