Bug 96047 – JSC should have a zombie mode

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Hahnenberg]

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.

[Mark Hahnenberg]

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.

[Mark Hahnenberg]

Created attachment 162633 [details]
Patch

[Geoffrey Garen]

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.

[Mark Hahnenberg]

Created attachment 162638 [details]
Patch

[Geoffrey Garen]

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().

[Mark Hahnenberg]

Created attachment 162642 [details]
Patch

[Geoffrey Garen]

Comment on attachment 162642 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 162642 [details]
Patch

Clearing flags on attachment: 162642

Committed r127829: <http://trac.webkit.org/changeset/127829>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

This seems worth a blog post, or at very least a webkit-dev one.