Last Comment Bug 727223 - "Assertion failure: !obj->isBlock(),"
: "Assertion failure: !obj->isBlock(),"
: assertion, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Mac OS X
-- critical (vote)
: mozilla13
Assigned To: Brian Hackett (:bhackett)
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: jsfunfuzz 706914
  Show dependency treegraph
Reported: 2012-02-14 13:07 PST by Gary Kwong [:gkw] [:nth10sd]
Modified: 2012-09-14 19:33 PDT (History)
6 users (show)
gary: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

stack (1.89 KB, text/plain)
2012-02-14 13:07 PST, Gary Kwong [:gkw] [:nth10sd]
no flags Details
patch (2.62 KB, patch)
2012-02-16 13:38 PST, Brian Hackett (:bhackett)
dvander: review+
Details | Diff | Splinter Review

Description User image Gary Kwong [:gkw] [:nth10sd] 2012-02-14 13:07:13 PST
Created attachment 597151 [details]

try {
    function x() {}
} catch (e) {}
(function() {
    for (let c in [0, 0, 0]) {
        let c
        for (y in decodeURI()) {
            (function() {

asserts js debug shell on m-c changeset ebafee0cea36 with -m, -n and -a when the testcase is passed in as a CLI argument, at Assertion failure: !obj->isBlock(). Tested on 32-bit 10.6 js shell.

Guessing related to chunk patch in bug 706914 due to presence of mjitChunkLimit.
Comment 1 User image Brian Hackett (:bhackett) 2012-02-16 13:38:17 PST
Created attachment 597979 [details] [diff] [review]

This is actually an old bug in the BINDNAME IC (not sure how old, but seems to predate objshrink).  It tests shapes incorrectly (testing the scope's shape twice, and then off by one going up to the object.  This normally means it will miss, but it can get spurious hits when adjacent objects on the scope chain have identical shapes.  Not quite sure why mjitChunkLimit is required, but I think a specific order of invalidation/compilation is required for this testcase to assert.
Comment 2 User image David Anderson [:dvander] 2012-02-16 15:52:30 PST
Comment on attachment 597979 [details] [diff] [review]

Review of attachment 597979 [details] [diff] [review]:

Bleh. Nice catch.
Comment 3 User image Jan de Mooij [:jandem] 2012-02-20 07:29:02 PST
Comment on attachment 597979 [details] [diff] [review]

Review of attachment 597979 [details] [diff] [review]:

::: js/src/methodjit/PolyIC.cpp
@@ +1770,5 @@
> +            JSObject *tobj = &scopeChain->asScope().enclosingScope();
> +            Address parent(pic.objReg, ScopeObject::offsetOfEnclosingScope());
> +            while (tobj) {
> +                if (!IsCacheableNonGlobalScope(tobj))
> +                    return disable("non-cacheable obj in scope chain");

AFAICS this no longer calls IsCacheableNonGlobalScope for the first link on the scope chain, right? I'm working on a BINDNAME IC for IonMonkey (bug 728311) so I'm investigating what JM does.
Comment 4 User image Brian Hackett (:bhackett) 2012-02-20 07:39:02 PST
Yeah, IsCacheableNonGlobalScope should be tested for all objects on the scope chain.  I'll fix this before checking in.  I don't know whether it actually matters --- 'with' objects won't be the topmost object, since JSOP_WITH isn't compiled, but there could be problems with non-global scopes for event handlers and such.
Comment 5 User image Brian Hackett (:bhackett) 2012-02-21 06:49:14 PST
Comment 6 User image Tom Schuster [:evilpie] 2012-02-21 07:08:20 PST
You might want to check-in the test case.
Comment 7 User image Ed Morley [:emorley] 2012-02-22 04:40:33 PST
Comment 8 User image Ed Morley [:emorley] 2012-09-14 03:08:59 PDT
Gary, don't suppose you could check in the testcase? :-)
Comment 9 User image Gary Kwong [:gkw] [:nth10sd] 2012-09-14 08:34:48 PDT
Comment 10 User image Ed Morley [:emorley] 2012-09-14 09:50:01 PDT
Thank you :-)
Comment 11 User image Ryan VanderMeulen [:RyanVM] 2012-09-14 18:18:38 PDT
Comment 12 User image Gary Kwong [:gkw] [:nth10sd] 2012-09-14 19:33:30 PDT
Test was landed in testsuite. -> VERIFIED

Note You need to log in before you can comment on or make changes to this bug.