922028 - Only intern iterator result object shapes in compileAndGo mode

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andy Wingo [:wingo]]

Language constructs that return { value, done } iterator result should only pre-intern a shape when compileAndGo mode is on.

Background:  I was fixing up the addon sdk for new-style for-of iteration, and in doing so added a star generator to one of their files.  It failed to XDR; see bug 907077 comment 49.  When encoding the script, an assertion failed: that the script's object vector only holds functions or static block objects.  Some GDB showed that the object that it failed on was an interned object literal shape.  A long debugging session later, and it turned out that the bytecode was at fault (!).

Comment 1

[Andy Wingo [:wingo]]

Attachment: Only intern iterator result object shapes in compileAndGo mode

Comment 2

[Nicolas B. Pierron [:nbp]]

Comment on attachment 811955 [details] [diff] [review]
Only intern iterator result object shapes in compileAndGo mode

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

::: js/src/frontend/BytecodeEmitter.cpp
@@ +1849,5 @@
>  
>  static bool
>  EmitPrepareIteratorResult(ExclusiveContext *cx, BytecodeEmitter *bce)
>  {
> +    if (bce->script->compileAndGo) {

Note: We are trying to get rid of the compileAndGo flag, adding more differences is probably not the good way to fix that.

Comment 3

[Andy Wingo [:wingo]]

(In reply to Nicolas B. Pierron [:nbp] from comment #2)
> Comment on attachment 811955 [details] [diff] [review]
> Only intern iterator result object shapes in compileAndGo mode
> 
> Review of attachment 811955 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/frontend/BytecodeEmitter.cpp
> @@ +1849,5 @@
> >  
> >  static bool
> >  EmitPrepareIteratorResult(ExclusiveContext *cx, BytecodeEmitter *bce)
> >  {
> > +    if (bce->script->compileAndGo) {
> 
> Note: We are trying to get rid of the compileAndGo flag, adding more
> differences is probably not the good way to fix that.

I'm sympathetic, but the behavior difference is rooted in XDRScript.  Without this change, XDRScript will core when serializing star generators.  this change brings the iterator result creation code in line with other object literal creation code.

Comment 4

[Jason Orendorff [:jorendorff]]

Right. nbp, the code Andy is adding only *looks* like a special case. It is actually simply the implication of an existing difference between CNG and other code, and I'm sure removing it will be straightforward once the existing difference is fixed.

Comment 5

[Jason Orendorff [:jorendorff]]

Comment on attachment 811955 [details] [diff] [review]
Only intern iterator result object shapes in compileAndGo mode

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

Looks good. Thanks.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6682ed134766

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6682ed134766