diff options
author | Primiano Tucci <primiano@google.com> | 2024-05-16 03:42:09 +0100 |
---|---|---|
committer | Steve Golton <stevegolton@google.com> | 2024-05-16 11:56:45 +0000 |
commit | fa446e592c8729df5a3e1158caf2337574fa4a8f (patch) | |
tree | 8bb69dab27ab990c2f62dc406700123b511f4d19 | |
parent | 444bb44f0712aeb12d6ab546035811d4541a8f93 (diff) | |
download | perfetto-ui-stable.tar.gz |
ui: Fix Wasm interop calls with pointers > 2GBui-stable
- We recently bumped the Wasm max memory from 2GB->4GB
in aosp/3018909.
- That can cause some heap pointers to be >2GB.
- Sometimes heap pointers are passed to wasm_bridge.ts:onReply()
when they are HeapBuffered buffers for RPC replies.
- By default the Wasm bindings map uint32 to JS numbers. If the
uint32 is > 2GB the called function sees a negative argument
on the JS side.
- The fix is to triple-shift (>>> 0) which converts the negative
number into the corresponding uint32 positive one.
- That in turn would manifest as slicing the wrong address of
the buffer when figuring out the boundaries of the RPC reply
buffer on the JS side, breaking the RPC sequence.
Bug: 332083475
Change-Id: I61005a442ae2322e4db2499812828053bb936104
(cherry picked from commit 8cac0c1697db9594dbbc5c0ff720b47d5b1aa77f)
-rw-r--r-- | ui/src/engine/wasm_bridge.ts | 21 |
1 files changed, 15 insertions, 6 deletions
diff --git a/ui/src/engine/wasm_bridge.ts b/ui/src/engine/wasm_bridge.ts index de89e97c3..bd72f5166 100644 --- a/ui/src/engine/wasm_bridge.ts +++ b/ui/src/engine/wasm_bridge.ts @@ -53,12 +53,13 @@ export class WasmBridge { }); this.whenInitialized = deferredRuntimeInitialized.then(() => { const fn = this.connection.addFunction(this.onReply.bind(this), 'vii'); - this.reqBufferAddr = this.connection.ccall( - 'trace_processor_rpc_init', - /* return=*/ 'number', - /* args=*/ ['number', 'number'], - [fn, REQ_BUF_SIZE], - ); + this.reqBufferAddr = + this.connection.ccall( + 'trace_processor_rpc_init', + /* return=*/ 'number', + /* args=*/ ['number', 'number'], + [fn, REQ_BUF_SIZE], + ) >>> 0; // >>> 0 = static_cast<uint32_t> (see comment in onReply()). }); } @@ -108,6 +109,14 @@ export class WasmBridge { // This function is bound and passed to Initialize and is called by the C++ // code while in the ccall(trace_processor_on_rpc_request). private onReply(heapPtr: number, size: number) { + // Force heapPtr to be a positive using an unsigned right shift. + // The issue here is the following: the matching code in wasm_bridge.cc + // invokes this function passing arguments as uint32_t. However, in the + // wasm<>JS interop bindings, the uint32 args become Js numbers. If the + // pointer is > 2GB, this number will be negative, which causes the wrong + // behaviour on slice(). + heapPtr = heapPtr >>> 0; // This is static_cast<uint32_t>(heapPtr). + size = size >>> 0; const data = this.connection.HEAPU8.slice(heapPtr, heapPtr + size); assertExists(this.messagePort).postMessage(data, [data.buffer]); } |