951722 - GenerationalGC: Add asserts that hash table postbarriers are working

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

For hashtables that are not testable from JS code, add assertions that our postbarriers have worked correctly.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: check-debug-scopes

Add assertions for debug scopes.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: check-new-type-objects

Add assertions for new type objects table.

Comment 3

[Terrence Cole [:terrence]]

Comment on attachment 8349517 [details] [diff] [review]
check-debug-scopes

Review of attachment 8349517 [details] [diff] [review]:
-----------------------------------------------------------------

Great! r=me

::: js/src/vm/ScopeObject.cpp
@@ +1697,5 @@
> +void
> +DebugScopes::checkHashTables(JSRuntime *runtime)
> +{
> +    /*
> +     * This is called from MarkRuntime after the storebuffer has been marked to

This is called directly from StoreBuffer::mark now, did you mean to update this comment?

::: js/src/vm/ScopeObject.h
@@ +747,5 @@
>    public:
>      void mark(JSTracer *trc);
>      void sweep(JSRuntime *rt);
> +#if defined(DEBUG) && defined(JSGC_GENERATIONAL)
> +    void checkHashTables(JSRuntime *rt);

This is a bit generic; how about checkHashTablesAfterMovingGC?

Comment 4

[Terrence Cole [:terrence]]

Comment on attachment 8349518 [details] [diff] [review]
check-new-type-objects

Review of attachment 8349518 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent! r=me

::: js/src/jscompartment.h
@@ +247,5 @@
>      js::types::TypeObjectWithNewScriptSet newTypeObjects;
>      js::types::TypeObjectWithNewScriptSet lazyTypeObjects;
>      void sweepNewTypeObjectTable(js::types::TypeObjectWithNewScriptSet &table);
> +#if defined(DEBUG) && defined(JSGC_GENERATIONAL)
> +    void checkNewTypeObjectTable();

Same comment as before; checkNewTypeObjectTableAfterMovingGC.

Comment 5

[Jon Coppeard (:jonco)]

Comments fixed and pushed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/8d8b6b62294b
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e6b39fbd81e

Comment 6

[Jon Coppeard (:jonco)]

This causes failures on inbound, although I can't repro locally yet:

TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/gc/bug-880776.js | --no-baseline --no-ion
INFO stderr 2> Assertion failure: !IsInsideNursery(rt, proto.toObject()), at /builds/slave/m-in_l64-d_sm-ggc-000000000000/src/js/src/jsinfer.cpp:3976
TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/gc/bug-880776.js | --no-baseline --no-ion --no-ti
INFO stderr 2> Assertion failure: !IsInsideNursery(rt, proto.toObject()), at /builds/slave/m-in_l64-d_sm-ggc-000000000000/src/js/src/jsinfer.cpp:3976
TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/gc/bug-880776.js |
INFO stderr 2> Assertion failure: !IsInsideNursery(rt, proto.toObject()), at /builds/slave/m-in_l64-d_sm-ggc-000000000000/src/js/src/jsinfer.cpp:3976
make: *** [check-jit-test] Error 2

Comment 7

[Jon Coppeard (:jonco)]

Backed out asserts for new type objects table:

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2b59ec2df63

Comment 8

[Jon Coppeard (:jonco)]

The problem is that StoreBuffer::mark() gets called when verifying post barriers, at which time it does not actually mark the targets of edges in the buffer.  I'll move the checking code to Nursery::collect() instead.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8d8b6b62294b

Comment 10

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a1ec6b78880

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4a1ec6b78880