Created attachment 8700655 [details] [diff] [review]
We have an AutoWritableJitCode in Linker::newCode. Most callers have to update the JIT code after that, so they have to use another AutoWritableJitCode.
This patch adds a Maybe<AutoWritableJitCode> to the Linker class, so the code is writable as long as the Linker is on the stack. This allows us to remove some AutoWritableJitCode instances.
Ion IC stubs are even worse, we used AutoWritableJitCode for the new stub like 3 or 4 times. This patch refactors the code a bit so we finish the stub completely in linkCode, while it's still writable, and then we attach it.
Comment on attachment 8700655 [details] [diff] [review]
Review of attachment 8700655 [details] [diff] [review]:
@@ +747,5 @@
> JitCompartment::toggleBarriers(bool enabled)
> // Toggle barriers in compartment wide stubs that have patchable pre barriers.
> if (regExpExecStub_)
> + regExpExecStub_->togglePreBarriers(enabled, Reprotect);
Wouldn't it be better to use
in this function?
Keeping this bug open because makeAllWritable/makeExecutable are added in a patch I didn't land yet, so I still have to land that part. I pushed this patch to see how it affects AWFY.
(In reply to Nicolas B. Pierron [:nbp] from comment #1)
> Wouldn't it be better to use
> in this function?
Good suggestion. I'm not sure, I think it depends on the number of Zones. It may indeed be more efficient to toggle all pages in the runtime, or we could use a separate ExecutableAllocator for each Zone... I'll try to do some measurements.
Also, makeAllWritable/makeAllExecutable are less secure, because an attacker could use web workers to write to JIT code while it's writable on the main thread. Still a lot safer than RWX.
Jan, did the second patch land yet?
(In reply to Jon Coppeard (:jonco) from comment #5)
> Jan, did the second patch land yet?
I think we can mark this FIXED and don't need the second patch.