761864 - Assertion failure: [barrier verifier] Unmarked edge: scope chain, at jsgc.cpp:4443

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

Attachment: Test case for shell (run with -n -m)

The attached test asserts on mozilla-central revision cf4face65451 (options -m -n):

Assuming s-s due to GC-related assertion.

Comment 1

[Luke Wagner [:luke]]

This is bug 659577.

Comment 2

[Luke Wagner [:luke]]

Attachment: fix and test

So what was happening is this:
 1. when we start the igc, the suspended generator frame's scope is reachable
 2. then we execute code which closes the generator
 3. when verifying, the closed generator is no longer traced, so the scope chain isn't reachable and there wasn't a write barrier that marked it

The regression is that https://hg.mozilla.org/mozilla-central/rev/b863ef9946b8#l31.66 made us not trace generator frames of closed generators without adding a write barrier.  Instead of adding a write barrier, I'll revert the change (so closed generators are still traced) and just fix the original problem differently (see comment on onGeneratorFrameChange).

Comment 3

[Luke Wagner [:luke]]

Attachment: fix and test

Oops, wrong version

Comment 4

[Luke Wagner [:luke]]

Attachment: better fix and test

After thinking about it a bit more, I realized that the better fix is to just use the same pre-write barrier that we use when copying a generator onto the VM stack when the generator is closed.

Comment 5

[Terrence Cole [:terrence]]

Comment on attachment 631582 [details] [diff] [review]
better fix and test

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

This is a nice cleanup.

::: js/src/jsiter.cpp
@@ +1474,5 @@
>      return obj;
>  }
>  
> +static void
> +SetGeneratorClosed(JSContext *cx, JSGenerator *gen);

Why do we need to forward declare this below the definition?

Comment 6

[Luke Wagner [:luke]]

(In reply to Terrence Cole [:terrence] from comment #5)
Oops, I read this right after pushing.  I'll throw it away in a subsequent patch.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bef8d091055a

Comment 7

[Luke Wagner [:luke]]

https://hg.mozilla.org/mozilla-central/rev/bef8d091055a

Comment 8

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 9

[Daniel Veditz [:dveditz]]

(In reply to Luke Wagner [:luke] from comment #1)
> This is bug 659577.

I've interpreted this as saying it's a regression from that bug, and that therefore we don't need this in earlier builds. Please clear "unaffected" from those status fields if this is incorrect.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Can anyone suggest a security rating for this?

Comment 11

[Al Billings [:abillings - ex-MoCo]]

(In reply to Daniel Veditz [:dveditz] from comment #9)
> (In reply to Luke Wagner [:luke] from comment #1)
> > This is bug 659577.
> 
> I've interpreted this as saying it's a regression from that bug, and that
> therefore we don't need this in earlier builds. Please clear "unaffected"
> from those status fields if this is incorrect.

How is this true when bug 761863 is also marked as bug 659577 and is marked as won't fix for 14 and 15?

Comment 12

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

> How is this true when bug 761863 is also marked as bug 659577 and is marked
> as won't fix for 14 and 15?

I just marked bug 761863 as WONTFIX at this point in time, for 14 because 14 is already EOL, and 15 because 16 is being released soon (in ~2 weeks).

Comment 13

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Luke is out, but I think that comment 9 in bug 761863 is incorrect. I've fixed the flags there.

Comment 14

[Al Billings [:abillings - ex-MoCo]]

No longer tracking for 16 advisories then.

Comment 15

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929