Summary: | Wasm should cage the memory base pointers in structs | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Keith Miller <keith_miller> | ||||
Component: | WebAssembly | Assignee: | Keith Miller <keith_miller> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, commit-queue, dbates, ews-watchlist, mark.lam, msaboff, saam, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Keith Miller
2019-05-06 12:05:07 PDT
Created attachment 370065 [details]
Patch
Comment on attachment 370065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370065&action=review r=me > Source/JavaScriptCore/ChangeLog:11 > + Currently, we use cageConditionally; this only matters for API > + users since the web content process cannot disable primitive > + gigacage. This patch also adds a set helper for union/intersection > + of RegisterSets. Do we have tests where Gigacage is disabled with Wasm enabled in JSC? > Source/JavaScriptCore/wasm/WasmBinding.cpp:48 > + GPRReg scratch = wasmCallingConventionAir().prologueScratch(0); 👍🏼 > Source/JavaScriptCore/wasm/WasmMemory.cpp:-442 > - m_memory.resize(m_size, desiredSize); Seems like this was a bug? Do we have a test? Comment on attachment 370065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370065&action=review >> Source/JavaScriptCore/ChangeLog:11 >> + of RegisterSets. > > Do we have tests where Gigacage is disabled with Wasm enabled in JSC? I don’t believe so. I’m not sure how important it is since wasn’t memory is always allocated out of the cage anyway. >> Source/JavaScriptCore/wasm/WasmMemory.cpp:-442 >> - m_memory.resize(m_size, desiredSize); > > Seems like this was a bug? Do we have a test? It’s not a bug. I changed the type of the container so I had to change the name of the method. There’s definitely theses that grow (I recall hitting this in the og patch). Comment on attachment 370065 [details] Patch Clearing flags on attachment: 370065 Committed r245432: <https://trac.webkit.org/changeset/245432> All reviewed patches have been landed. Closing bug. |