| Summary: | ENABLE(ASSEMBLER_WX_EXCLUSIVE): LinkBuffer can leave pages not marked as executable. | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yong Li <yong.li.webkit> | ||||||||
| Component: | JavaScriptCore | Assignee: | Yong Li <yong.li.webkit> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Critical | CC: | barraclough, fpizlo, ggaren, webkit.review.bot | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Other | ||||||||||
| OS: | Other | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Yong Li
2012-01-20 12:05:40 PST
Created attachment 123353 [details]
workaround
Do any platforms still use WX_EXCLUSIVE? (In reply to comment #2) > Do any platforms still use WX_EXCLUSIVE? Yes BlackBerry/QNX uses it 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.
(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... 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? Created attachment 126314 [details]
the patch that fixes LinkBuffer
(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 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 on attachment 126314 [details]
the patch that fixes LinkBuffer
r- because it is fine for ENABLE(BRANCH_COMPACTION), but not the other case.
(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)... Created attachment 126537 [details]
Updated
Comment on attachment 126537 [details]
Updated
Looks good.
Comment on attachment 126537 [details] Updated Clearing flags on attachment: 126537 Committed r107447: <http://trac.webkit.org/changeset/107447> All reviewed patches have been landed. Closing bug. |