Bug 76724 - ENABLE(ASSEMBLER_WX_EXCLUSIVE): LinkBuffer can leave pages not marked as executable.
Summary: ENABLE(ASSEMBLER_WX_EXCLUSIVE): LinkBuffer can leave pages not marked as exec...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Critical
Assignee: Yong Li
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-20 12:05 PST by Yong Li
Modified: 2012-02-10 15:02 PST (History)
4 users (show)

See Also:

Attachments
workaround (2.40 KB, patch)
2012-01-20 12:11 PST, Yong Li 		no flags Details | Formatted Diff | Diff
the patch that fixes LinkBuffer (3.94 KB, patch)
2012-02-09 09:02 PST, Yong Li 		rwlbuis: review-
Details | Formatted Diff | Diff
Updated (4.15 KB, patch)
2012-02-10 11:07 PST, Yong Li 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yong Li 2012-01-20 12:05:40 PST 
MetaAllocator breaks ENABLE(ASSEMBLER_WX_EXCLUSIVE) because the flags can only be applied to whole pages.
Comment 1 Yong Li 2012-01-20 12:11:59 PST 
Created attachment 123353 [details]
workaround
Comment 2 Geoffrey Garen 2012-01-20 12:34:37 PST 
Do any platforms still use WX_EXCLUSIVE?
Comment 3 Yong Li 2012-01-20 13:01:05 PST 
(In reply to comment #2)
> Do any platforms still use WX_EXCLUSIVE?

Yes BlackBerry/QNX uses it
Comment 4 Geoffrey Garen 2012-01-20 13:16:45 PST 
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.
Comment 5 Yong Li 2012-01-20 13:19:05 PST 
(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...
Comment 6 Yong Li 2012-02-09 08:57:43 PST 
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?
Comment 7 Yong Li 2012-02-09 09:02:35 PST 
Created attachment 126314 [details]
the patch that fixes LinkBuffer
Comment 8 Yong Li 2012-02-09 10:28:23 PST 
(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.
Comment 9 Eric Seidel (no email) 2012-02-10 10:19:07 PST 
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.
Comment 10 Rob Buis 2012-02-10 10:59:14 PST 
Comment on attachment 126314 [details]
the patch that fixes LinkBuffer

r- because it is fine for ENABLE(BRANCH_COMPACTION), but not the other case.
Comment 11 Yong Li 2012-02-10 11:00:29 PST 
(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)...
Comment 12 Yong Li 2012-02-10 11:07:45 PST 
Created attachment 126537 [details]
Updated
Comment 13 Rob Buis 2012-02-10 11:11:03 PST 
Comment on attachment 126537 [details]
Updated

Looks good.
Comment 14 WebKit Review Bot 2012-02-10 15:02:15 PST 
Comment on attachment 126537 [details]
Updated

Clearing flags on attachment: 126537

Committed r107447: <http://trac.webkit.org/changeset/107447>
Comment 15 WebKit Review Bot 2012-02-10 15:02:20 PST 
All reviewed patches have been landed.  Closing bug.