Implement wrapper tracing verifier

Implement wrapper tracing verifier

Verifier simulates stop-the-world wrapper tracing and checks, that all objects
that would be found during atomic pause were actually found by incremental
wrapper tracing too. If not, that means some write barrier is missing.

It works by recording all v8 reported wrappers and all newly associated
wrappers, tracing from these in the atomic pause, and checking whether all
reachable objects were marked.

LOG=no
BUG=468240
Committed: https://crrev.com/a973a859a9b6c995ff5081174426306a9fa3914d
Cr-Commit-Position: refs/heads/master@{#401580}

[Marcel Hlopko, 2016-06-21 13:45:50]

hlopko@chromium.org changed reviewers:
+ haraken@chromium.org, hpayer@chromium.org, jochen@chromium.org

[Marcel Hlopko, 2016-06-21 13:45:56]

Ptal.

[jochen (gone - plz use gerrit), 2016-06-22 13:33:43]

landing this would make sense, no?

[Marcel Hlopko, 2016-06-22 13:38:28]

On 2016/06/22 at 13:33:43, jochen wrote:
> landing this would make sense, no?

Yep I guess so. I just wrote an email to Kentaro asking about it. The funny
thing is that with current v8 implementation it just duplicates work of tracing.
But once v8 starts asking blink to do incremental tracing steps, the verifier
will shine :)

[haraken, 2016-06-22 15:43:12]

LGTM

I'm just curious but if you enable the verification on Debug builds (instead of
using RuntimeEnabledFeatures), will it be unacceptably slow?

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitor.cpp
(right):

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitor.cpp:99:
m_advancingTracing = true;

You can use TemporaryChange<>.

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitor.h
(right):

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitor.h:148: * Is
wrapper tracing currently in progres? True if TracePrologue has been

progress

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitorVerifier.h
(right):

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitorVerifier.h:16:
#include "core/html/imports/HTMLImportsController.h"

Remove unnecessary header includes.

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitorVerifier.h:41:
NOTREACHED();

Add a comment and explain what this is checking.

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/heap/Heap.cpp (right):

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/Heap.cpp:575:
state->invalidateDeadObjectsInWrappersMarkingDeque();

Why do we need this change?

[Marcel Hlopko, 2016-06-23 09:09:13]

Agreeing on all points, fixed all, landing :)

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitor.cpp
(right):

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitor.cpp:99:
m_advancingTracing = true;
On 2016/06/22 at 15:43:12, haraken wrote:
> You can use TemporaryChange<>.

Wonderful! Done.

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitor.h
(right):

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitor.h:148: * Is
wrapper tracing currently in progres? True if TracePrologue has been
On 2016/06/22 at 15:43:12, haraken wrote:
> progress

Done.

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitorVerifier.h
(right):

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitorVerifier.h:16:
#include "core/html/imports/HTMLImportsController.h"
On 2016/06/22 at 15:43:12, haraken wrote:
> Remove unnecessary header includes.

All are requried because of
WRAPPER_VISITOR_SPECIAL_CLASSES(DECLARE_DISPATCH_TRACE_WRAPPERS); They are only
forward-declared in the ScritpWrappableVisitor.h

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/ScriptWrappableVisitorVerifier.h:41:
NOTREACHED();
On 2016/06/22 at 15:43:12, haraken wrote:
> Add a comment and explain what this is checking.

Done.

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/heap/Heap.cpp (right):

https://codereview.chromium.org/2084933002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/heap/Heap.cpp:575:
state->invalidateDeadObjectsInWrappersMarkingDeque();
On 2016/06/22 at 15:43:12, haraken wrote:
> Why do we need this change?

We don't. Git rebase hates me :)

[Marcel Hlopko, 2016-06-23 09:09:17]

The CQ bit was checked by hlopko@chromium.org

[Marcel Hlopko, 2016-06-23 09:09:17]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2084933002/#ps20001
(title: "Use verifier when dchecks are on")

[commit-bot: I haz the power, 2016-06-23 09:09:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2084933002/20001

[commit-bot: I haz the power, 2016-06-23 11:28:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-23 11:29:44]

Description was changed from

==========
Implement wrapper tracing verifier

Verifier simulates stop-the-world wrapper tracing and checks, that all objects
that would be found during atomic pause were actually found by incremental
wrapper tracing too. If not, that means some write barrier is missing.

It works by recording all v8 reported wrappers and all newly associated
wrappers, tracing from these in the atomic pause, and checking whether all
reachable objects were marked.

LOG=no
BUG=468240
==========

to

==========
Implement wrapper tracing verifier

Verifier simulates stop-the-world wrapper tracing and checks, that all objects
that would be found during atomic pause were actually found by incremental
wrapper tracing too. If not, that means some write barrier is missing.

It works by recording all v8 reported wrappers and all newly associated
wrappers, tracing from these in the atomic pause, and checking whether all
reachable objects were marked.

LOG=no
BUG=468240

Committed: https://crrev.com/a973a859a9b6c995ff5081174426306a9fa3914d
Cr-Commit-Position: refs/heads/master@{#401580}
==========

[commit-bot: I haz the power, 2016-06-23 11:29:45]

Patchset 2 (id:??) landed as
https://crrev.com/a973a859a9b6c995ff5081174426306a9fa3914d
Cr-Commit-Position: refs/heads/master@{#401580}