Bug 76724 – ENABLE(ASSEMBLER_WX_EXCLUSIVE): LinkBuffer can leave pages not marked as executable.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yong Li]

MetaAllocator breaks ENABLE(ASSEMBLER_WX_EXCLUSIVE) because the flags can only be applied to whole pages.

[Yong Li]

Created attachment 123353 [details]
workaround

[Geoffrey Garen]

Do any platforms still use WX_EXCLUSIVE?

[Yong Li]

(In reply to comment #2)
> Do any platforms still use WX_EXCLUSIVE?

Yes BlackBerry/QNX uses it

[Geoffrey Garen]

Comment on attachment 123353 [details]
workaround

I guess I'll say r+ to unbreak that platform. But you're going to get really bad memory use if every JS function is 1 page at minimum.

[Yong Li]

(In reply to comment #4)
> (From update of attachment 123353 [details])
> I guess I'll say r+ to unbreak that platform. But you're going to get really bad memory use if every JS function is 1 page at minimum.

Thanks. I'll try to think about some other solution...

[Yong Li]

Found a bug in LinkedBuffer. The size used to call makeExecutable can be smaller than the one that was used for makeWritable. So it can leave pages that are not set back to default flags. When an assembly on that page is executed or JIT returns to that page in the case it was already executing that page, the software will crash.

Patch is on the way.

But I'm not sure MetaAllocator is safe with ENABLE(ASSEMBLER_WX_EXCLUSIVE) even with this fix. Geoffrey, do you see potential problems with MetaAllocator + ENABLE(ASSEMBLER_WX_EXCLUSIVE) + Workers?

[Yong Li]

Created attachment 126314 [details]
the patch that fixes LinkBuffer

[Yong Li]

(In reply to comment #6)
> Found a bug in LinkedBuffer. The size used to call makeExecutable can be smaller than the one that was used for makeWritable. So it can leave pages that are not set back to default flags. When an assembly on that page is executed or JIT returns to that page in the case it was already executing that page, the software will crash.
> 
> Patch is on the way.
> 
> But I'm not sure MetaAllocator is safe with ENABLE(ASSEMBLER_WX_EXCLUSIVE) even with this fix. Geoffrey, do you see potential problems with MetaAllocator + ENABLE(ASSEMBLER_WX_EXCLUSIVE) + Workers?

never mind. worker should OK because worker uses its own global object.

[Eric Seidel (no email)]

Comment on attachment 123353 [details]
workaround

Cleared Geoffrey Garen's review+ from obsolete attachment 123353 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[Rob Buis]

Comment on attachment 126314 [details]
the patch that fixes LinkBuffer

r- because it is fine for ENABLE(BRANCH_COMPACTION), but not the other case.

[Yong Li]

(In reply to comment #10)
> (From update of attachment 126314 [details])
> r- because it is fine for ENABLE(BRANCH_COMPACTION), but not the other case.

Good catch! it would cause problem when !ENABLE(BRANCH_COMPACTION) && ENABLE(ASSEMBLER_WX_EXCLUSIVE)...

[Yong Li]

Created attachment 126537 [details]
Updated

[Rob Buis]

Comment on attachment 126537 [details]
Updated

Looks good.

[WebKit Review Bot]

Comment on attachment 126537 [details]
Updated

Clearing flags on attachment: 126537

Committed r107447: <http://trac.webkit.org/changeset/107447>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.