Skip to content

[browser][mono] Make sure we treat the 4GB ptrs as unsigned - #121221

Merged
lewing merged 31 commits into
dotnet:mainfrom
lewing:sign-on-ptr
Nov 26, 2025
Merged

lewing merged 31 commits into
dotnet:mainfrom
lewing:sign-on-ptr

Conversation

@lewing

@lewing lewing commented Oct 30, 2025 •

Copy link
Copy Markdown
Member

Fixes problems with pointers over 2GB mark. This is still use on your own risk

  • crypto buffers
  • debugger buffers
  • WS buffers
  • event-pipe buffers
  • jiterp traces, dropped support for unused constantSlots feature
  • JS MemoryView,Span
  • various memory JS APIs
  • asset loading
  • string marshaling

Related #109079
Related #58382

@github-actions github-actions Bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 30, 2025
@lewing lewing changed the title Make sure we treat tread the ptr as unsigned Make sure we treat the ptrs as unsigned Oct 30, 2025
@lewing lewing added area-System.Runtime.InteropServices.JavaScript and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Oct 30, 2025
@pavelsavara

Copy link
Copy Markdown
Member

Please add http test to AllocateLargeHeapThenRepeatedlyInterop WBT

See src\mono\wasm\Wasm.Build.Tests\MemoryTests.cs and src\mono\wasm\testassets\WasmBasicTestApp\App\MemoryTest.cs

@pavelsavara

Copy link
Copy Markdown
Member

getU8 is probably also wrong when called from mono_wasm_bind_cs_function-> get_signature_type

Comment thread src/mono/browser/runtime/http.ts Outdated
@pavelsavara

Copy link
Copy Markdown
Member
  • ws_wasm_send is wrong about buffer_ptr
  • web_socket_receive_buffering with buffer_ptr
  • SystemJS_RandomBytes is wrong with bufferPtr
  • mono_wasm_fire_debugger_agent_message_with_data and mono_wasm_add_dbg_command_received is wrong with data
  • mono_wasm_load_bytes_into_heap and mono_wasm_load_bytes_into_heap_persistent is probably wrong with memoryOffset
  • mono_wasm_asm_loaded is probably wrong
  • diagnosticHelpers.ds_rt_websocket_send is probably wrong
  • viewOrCopy in strings.ts is probably wrong

@pavelsavara

Copy link
Copy Markdown
Member

This is previous attempt #109079

We introduced fixupPointer helper in memory.ts

@lewing
lewing requested a review from radekdoulik November 14, 2025 16:47
Comment thread src/mono/browser/runtime/memory.ts

@radekdoulik radekdoulik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beside the comment it LGTM.

Looking forward to more fun with wasm64

@pavelsavara

Copy link
Copy Markdown
Member

I will have look at this tomorrow and probably add few more. I will also unify it with coreCLR version

@pavelsavara pavelsavara added the arch-wasm WebAssembly architecture label Nov 25, 2025
Comment thread src/mono/browser/runtime/jiterpreter-support.ts Outdated
Comment thread src/mono/browser/runtime/jiterpreter-trace-generator.ts
Comment thread src/mono/browser/runtime/jiterpreter-trace-generator.ts Outdated
@pavelsavara pavelsavara changed the title Make sure we treat the ptrs as unsigned [browser][mono] Make sure we treat the ptrs as unsigned Nov 26, 2025
@pavelsavara pavelsavara changed the title [browser][mono] Make sure we treat the ptrs as unsigned [browser][mono] Make sure we treat the 4GB ptrs as unsigned Nov 26, 2025
@pavelsavara

Copy link
Copy Markdown
Member

Looks good

[14:16:42] info: Allocated 2500 MB at offset 2563124952, total linear memory 2768699392 bytes
[14:16:42] info: Using threadless Xunit runner
[14:16:43] info: Discovering: Common.Tests.dll (method display = ClassAndMethod, method display options = None)
[14:16:44] info: Discovered:  Common.Tests.dll (found 175 of 251 test cases)
[14:16:44] info: Starting:    Common.Tests.dll
[14:17:01] info: Finished:    Common.Tests.dll
[14:17:01] info: Stored /root/helix/work/workitem/uploads/xharness-output/testResults.xml results 301829 bytes
[14:17:01] info: Finished uploading 301829 bytes of RESULTXML
[14:17:01] info: Xml file was written to the provided writer.
[14:17:01] info: 
[14:17:01] info: === TEST EXECUTION SUMMARY ===
[14:17:01] info: Tests run: 1096 Passed: 1096 Inconclusive: 0 Failed: 0 Ignored: 0 Skipped: 0
[14:31:41] info: Finished:    System.Linq.Tests.dll
[14:31:43] info: Stored /root/helix/work/workitem/uploads/xharness-output/testResults.xml results 9405291 bytes
[14:31:43] info: Finished uploading 9405291 bytes of RESULTXML
[14:31:43] info: Xml file was written to the provided writer.
[14:31:43] info: 
[14:31:43] info: === TEST EXECUTION SUMMARY ===
[14:31:43] info: Tests run: 52177 Passed: 52168 Inconclusive: 0 Failed: 0 Ignored: 0 Skipped: 9
[14:18:16] info: Allocated 2500 MB at offset 2562076376, total linear memory 2767650816 bytes
[14:18:17] info: Using threadless Xunit runner
[14:18:17] info: Discovering: Common.Tests.dll (method display = ClassAndMethod, method display options = None)
[14:18:18] info: Discovered:  Common.Tests.dll (found 175 of 251 test cases)
[14:18:18] info: Starting:    Common.Tests.dll
[14:18:25] info: Finished:    Common.Tests.dll
[14:18:26] info: Stored /root/helix/work/workitem/uploads/xharness-output/testResults.xml results 304904 bytes
[14:18:26] info: Finished uploading 304904 bytes of RESULTXML
[14:18:26] info: Xml file was written to the provided writer.
[14:18:26] info: 
[14:18:26] info: === TEST EXECUTION SUMMARY ===
[14:18:26] info: Tests run: 1096 Passed: 1096 Inconclusive: 0 Failed: 0 Ignored: 0 Skipped: 0
[14:18:53] info: Finished:    System.Buffers.Tests.dll
[14:18:53] info: Stored /root/helix/work/workitem/uploads/xharness-output/testResults.xml results 23264 bytes
[14:18:53] info: Finished uploading 23264 bytes of RESULTXML
[14:18:53] info: Xml file was written to the provided writer.
[14:18:53] info: 
[14:18:53] info: === TEST EXECUTION SUMMARY ===
[14:18:53] info: Tests run: 77 Passed: 66 Inconclusive: 0 Failed: 0 Ignored: 0 Skipped: 11
[14:19:24] info: Finished:    System.Net.Http.Functional.Tests.dll
[14:19:24] info: Stored /root/helix/work/workitem/uploads/xharness-output/testResults.xml results 433770 bytes
[14:19:24] info: Finished uploading 433770 bytes of RESULTXML
[14:19:24] info: Xml file was written to the provided writer.
[14:19:24] info: 
[14:19:24] info: === TEST EXECUTION SUMMARY ===
[14:19:24] info: Tests run: 901 Passed: 781 Inconclusive: 0 Failed: 0 Ignored: 0 Skipped: 120
[14:21:32] info: Finished:    System.Net.WebSockets.Client.Tests.dll
[14:21:32] info: Stored /root/helix/work/workitem/uploads/xharness-output/testResults.xml results 119972 bytes
[14:21:32] info: Finished uploading 119972 bytes of RESULTXML
[14:21:32] info: Xml file was written to the provided writer.
[14:21:32] info: 
[14:21:32] info: === TEST EXECUTION SUMMARY ===
[14:21:32] info: Tests run: 315 Passed: 315 Inconclusive: 0 Failed: 0 Ignored: 0 Skipped: 0

@pavelsavara
pavelsavara marked this pull request as ready for review November 26, 2025 18:09
Copilot AI review requested due to automatic review settings November 26, 2025 18:09
@pavelsavara
pavelsavara requested a review from kg November 26, 2025 18:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses pointer handling issues for WebAssembly pointers exceeding 2GB by ensuring all pointer values are treated as unsigned throughout the browser/mono runtime. The changes are critical for supporting applications with memory usage above the 2GB mark.

Key changes:

  • Added unsigned right shift operations (>>> 0) to pointer accesses in memory operations, heap views, and buffer handling
  • Introduced fixupPointer helper function for consistent pointer conversion to unsigned values
  • Removed unused jiterpreter constant slots feature and simplified pointer constant handling
  • Updated pointer parameter handling across crypto, WebSocket, debugger, event-pipe, and string marshaling modules

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/native/libs/System.Native.Browser/utils/memory.ts Applied unsigned shift to HEAP8 array access for setHeapI8 and getHeapI8
src/native/libs/System.Native.Browser/native/crypto.ts Added unsigned conversion for crypto buffer pointer
src/mono/browser/runtime/web-socket.ts Imported fixupPointer and applied to WebSocket buffer pointers and receive status pointer
src/mono/browser/runtime/strings.ts Applied fixupPointer to string buffer pointers and unsigned shift to view slice/subarray operations
src/mono/browser/runtime/startup.ts Applied fixupPointer to assembly and PDB data pointers for debugging
src/mono/browser/runtime/roots.ts Applied unsigned conversion to root buffer offset and external root address
src/mono/browser/runtime/memory.ts Applied unsigned shift to HEAP8/HEAPU8 array accesses and added pointer fixup to _zero_region
src/mono/browser/runtime/marshal.ts Changed getI32 to getU32 for semaphore pointer and added fixupPointer to MemoryView constructor
src/mono/browser/runtime/jiterpreter.ts Added unsigned conversion to TraceInfo ip and jiterpreter function parameters
src/mono/browser/runtime/jiterpreter-trace-generator.ts Changed i32_const to ptr_const for class pointers and applied unsigned shift to offset
src/mono/browser/runtime/jiterpreter-support.ts Removed constant slots feature, updated ptr_const implementation, applied unsigned shift to memarg offset
src/mono/browser/runtime/jiterpreter-jit-call.ts Removed constant slot parameter from WasmBuilder and added unsigned conversion to trampoline parameters
src/mono/browser/runtime/jiterpreter-interp-entry.ts Removed constant slot usage and added unsigned conversion with mask operation for imethod and entry parameters
src/mono/browser/runtime/diagnostics/index.ts Applied unsigned shift to event-pipe buffer pointer
src/mono/browser/runtime/diagnostics/common.ts Applied unsigned shift to diagnostics buffer pointer
src/mono/browser/runtime/debug.ts Applied fixupPointer to debugger message and command buffer pointers
src/mono/browser/runtime/crypto.ts Added unsigned conversion for crypto buffer pointer

Comment thread src/mono/browser/runtime/strings.ts Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread src/mono/browser/runtime/diagnostics/index.ts

@kg kg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jiterp parts LGTM and i looked over everything else as well

@lewing
lewing enabled auto-merge (squash) November 26, 2025 20:25
@lewing
lewing merged commit f813007 into dotnet:main Nov 26, 2025
33 of 35 checks passed
@lewing
lewing deleted the sign-on-ptr branch November 26, 2025 20:35
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript os-browser Browser variant of arch-wasm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants