852016 - Assertion failure: v.isBoolean(), at jsobj.cpp or Assertion failure: !val.isMagic(), at jsobj.cpp

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

Attachment: stack for Assertion failure: v.isBoolean(), at jsobj.cpp

(function() {
    x = "arguments";
    f = eval("(function(){return(c for (x in eval(\"" + x + "\")))})");
    for (e in f()) {
        (function() {});
    }
})()

asserts js debug shell on m-c changeset a9a25781850e without any CLI argument at Assertion failure: v.isBoolean(), at jsobj.cpp

A variant,

(function() {
    x = "arguments.s";
    f = eval("(function(){return(c for (x in eval(\"" + x + "\")))})");
    for (e in f()) {
        (function() {});
    }
})()

asserts js debug shell on m-c changeset a9a25781850e without any CLI argument at Assertion failure: !val.isMagic(), at jsobj.cpp

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   122445:e3b899354a6f
user:        Brian Hackett
date:        Wed Feb 20 04:54:13 2013 -0700
summary:     Bug 842522 - Don't force construction of arguments objects in the presence of dynamic name accesses, r=luke.

(both testcases seem to point to bug 842522.)

Comment 1

[Brian Hackett [Laid off!]]

Attachment: patch

Generator functions don't have arguments, and it's an error to use 'arguments' within them.  There was no checking going on for this wrt eval scripts, so eval("arguments") within a generator would get whatever arguments value that could be found on the scope chain.  After bug 842522 this would find an optimized arguments value.  The patch fixes this to report an error on 'arguments' within a generator eval, as we do for uses directly in the generator.

Comment 2

[Luke Wagner [:luke]]

Perhaps I'm missing some more recent developments, but it seems like we do allow 'arguments' in generators, because they do have arguments:

js> function f() { yield arguments } 
js> f(1,2,3).next()                  
({0:1, 1:2, 2:3})

Comment 3

[Brian Hackett [Laid off!]]

Oh, this may just be with generator expressions.

Comment 4

[Luke Wagner [:luke]]

Comment on attachment 726769 [details] [diff] [review]
patch

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

::: js/src/frontend/BytecodeCompiler.cpp
@@ +66,5 @@
>              return false;
>      }
>  
> +    // It's an error to use |arguments| in a generator.
> +    if (script->isGenerator || script->isGeneratorExp) {

Ah, of course.  Then r+ with the first disjunct removed.

Comment 5

[Brian Hackett [Laid off!]]

Pushed, with a second test for eval("arguments") in generator functions.

https://hg.mozilla.org/integration/mozilla-inbound/rev/26b4880c8a64

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/26b4880c8a64