Bug 197620 – Wasm should cage the memory base pointers in structs

• Component: WebAssembly
• Status: RESOLVED FIXED

[Keith Miller]

...

[Keith Miller]

Created attachment 370065 [details]
Patch

[Saam Barati]

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?

[Keith Miller]

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).

[WebKit Commit Bot]

Comment on attachment 370065 [details]
Patch

Clearing flags on attachment: 370065

Committed r245432: <https://trac.webkit.org/changeset/245432>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/50878478>