Bug 96047 - JSC should have a zombie mode
Summary: JSC should have a zombie mode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-06 17:52 PDT by Mark Hahnenberg
Modified: 2012-09-07 10:15 PDT (History)
4 users (show)

See Also:


Attachments
Patch (9.43 KB, patch)
2012-09-06 18:05 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (7.06 KB, patch)
2012-09-06 18:27 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (6.83 KB, patch)
2012-09-06 18:36 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2012-09-06 17:52:37 PDT
To aid client of JSC as well as the developers of JSC, we should add a zombie mode that scribbles into objects in the MarkedSpace after they are found to be dead to prevent a sort of "use after free" situation. As a first cut we should support a mode that just scribbles on objects prior to their being reused (i.e. while they are "zombies") and a mode in which, in addition to scribbling on zombies, once an object has been marked its mark bit will never be cleared, thus giving us "immortal" zombies.
Comment 1 Mark Hahnenberg 2012-09-06 17:54:48 PDT
I should also mention that these two modes will be enabled through the use of environment variables. For now these will be "JSZombieEnabled" and "JSImmortalZombieEnabled". Setting them to any value will result in the use of the appropriate mode.
Comment 2 Mark Hahnenberg 2012-09-06 18:05:11 PDT
Created attachment 162633 [details]
Patch
Comment 3 Geoffrey Garen 2012-09-06 18:14:25 PDT
Comment on attachment 162633 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162633&action=review

Looks good, but you have a bug.

> Source/JavaScriptCore/heap/Heap.cpp:850
> +class ZombifyCellFunctor : public MarkedBlock::VoidFunctor {

Add a data member that causes us optionally set the mark bit as we zombify, if in immortal mode.

> Source/JavaScriptCore/heap/MarkedBlock.h:313
> +    inline void MarkedBlock::zombieClearMarks()

Remove this.
Comment 4 Mark Hahnenberg 2012-09-06 18:27:39 PDT
Created attachment 162638 [details]
Patch
Comment 5 Geoffrey Garen 2012-09-06 18:29:52 PDT
Comment on attachment 162638 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162638&action=review

> Source/JavaScriptCore/heap/Heap.cpp:741
> +    if (Options::useZombieMode() || sweepToggle == DoSweep) {

Looking at this again, I don't think you can piggy-back on this for zombies. If the call to shrink unmaps some blocks, they may remap into the heap later, failing to be immortal zombies.

I think you should just add a call to sweep() inside zombifyDeadObjects().
Comment 6 Mark Hahnenberg 2012-09-06 18:36:14 PDT
Created attachment 162642 [details]
Patch
Comment 7 Geoffrey Garen 2012-09-06 18:43:41 PDT
Comment on attachment 162642 [details]
Patch

r=me
Comment 8 WebKit Review Bot 2012-09-06 22:53:02 PDT
Comment on attachment 162642 [details]
Patch

Clearing flags on attachment: 162642

Committed r127829: <http://trac.webkit.org/changeset/127829>
Comment 9 WebKit Review Bot 2012-09-06 22:53:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Alexey Proskuryakov 2012-09-07 10:15:11 PDT
This seems worth a blog post, or at very least a webkit-dev one.