Closed Bug 852016 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: gkw, Unassigned)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update])

Attachments

(2 files)

(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.)
Attached patch patchSplinter Review
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.
Attachment #726769 - Flags: review?(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})
Oh, this may just be with generator expressions.
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.
Attachment #726769 - Flags: review?(luke) → review+
Pushed, with a second test for eval("arguments") in generator functions.

https://hg.mozilla.org/integration/mozilla-inbound/rev/26b4880c8a64
https://hg.mozilla.org/mozilla-central/rev/26b4880c8a64
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.