Last Comment Bug 684525 - "Assertion failure: shape->compartment() == compartment()," or "Assertion failure: compartment mismatched,"
: "Assertion failure: shape->compartment() == compartment()," or "Assertion fai...
Status: RESOLVED FIXED
js-triage-done
: assertion, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Windows 7
: -- critical (vote)
: ---
Assigned To: general
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: jsfunfuzz
  Show dependency treegraph
 
Reported: 2011-09-03 12:12 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2011-09-07 02:38 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments

Description Gary Kwong [:gkw] [:nth10sd] 2011-09-03 12:12:32 PDT
function f(code) {
    g(code.replace(), "new-compartment")
}
function g(a, b) {
    evalcx(a, newGlobal(b))
}
f("\
  let([]=#0=*) {\
    eval(\"''.e()\")\
  }\
")

asserts js debug shell on m-c changeset 472716252ea3 with -m and -a and the patch in bug 683966 comment 1 at Assertion failure: compartment mismatched,

A variant,

function f(code) {
    g(code, "new-compartment")
}
function g(a, b) {
    evalcx(a, newGlobal(b))
}
f("\
  let([]=#0=*) {\
    eval(\"''.e()\")\
  }\
")

asserts at Assertion failure: shape->compartment() == compartment(),
Comment 1 Bill McCloskey (:billm) 2011-09-06 14:12:22 PDT
I found a simpler testcase:

evalcx("\
  let([]=#0=*) {\
    eval(\"''.e()\")\
  }\
", newGlobal("new-compartment"));

I think this is a TI bug. Somehow script->where.global (which I think is new) is not set properly. It's in a different compartment from script, which seems bad.
Comment 2 Bill McCloskey (:billm) 2011-09-06 15:29:20 PDT
Actually, it looks like the problem is here, in JSScript::NewScriptFromCG:

    if (script->compileAndGo) {
        GlobalScope *globalScope = cg->compiler()->globalScope;
        if (globalScope->globalObj && globalScope->globalObj->isGlobal())
            script->where.global = globalScope->globalObj->asGlobal();
        else if (cx->globalObject->isGlobal())
>>          script->where.global = cx->globalObject->asGlobal();
    }

My understanding is that we're not supposed to be relying on cx->globalObject in this way, but I could be wrong.
Comment 3 Brian Hackett (:bhackett) 2011-09-06 15:35:17 PDT
Yeah, this is the problem.  What are the circumstances when cx->globalObject can be relied on for anything?  This is a TI bug --- TI needs to know about the global which compileAndGo scripts run against, and the parser does not always have this information available (no clue why).

This will get fixed with the next merge from JM (should be later today).  That makes computing the script's global lazier, not occurring until the script has executed at least once and we can determine the global for compileAndGo scripts from the scope chain.
Comment 4 Luke Wagner [:luke] 2011-09-06 17:43:40 PDT
cx->globalObject is very dangerous b/c it is almost never correct to use it.  Pretend it was named cx->globalObjectIfNothingElseIsRunning.  A JSContext has no long-term association with any global object, so the only valid question one may ask is "what is the 'current' global object" via GetGlobalForScopeChain.  It sounds like this isn't what you want, though.
Comment 5 Brendan Eich [:brendan] 2011-09-06 18:07:26 PDT
cx->globalObject should die. There's a bug on this, IINM.

Brian, re: comment 3: you should be able to get the object presented to the compiler as the compile-n-go global object, without any reliability problems. See compiler.globalScope init in js::Compiler::compileScript in jsparse.cpp.

/be
Comment 6 Brian Hackett (:bhackett) 2011-09-06 18:14:19 PDT
(In reply to Brendan Eich [:brendan] from comment #5)
> cx->globalObject should die. There's a bug on this, IINM.
> 
> Brian, re: comment 3: you should be able to get the object presented to the
> compiler as the compile-n-go global object, without any reliability
> problems. See compiler.globalScope init in js::Compiler::compileScript in
> jsparse.cpp.
> 
> /be

Yeah, but compiler.globalScope is not always set for compileAndGo scripts (or is set to a non-global object), that's the case where cx->globalObject was being incorrectly used instead.  (rev 554045e04d89 removes this stuff entirely, http://hg.mozilla.org/mozilla-central/diff/554045e04d89/js/src/jsscript.cpp)
Comment 7 Gary Kwong [:gkw] [:nth10sd] 2011-09-07 00:41:55 PDT
Did this get fixed somehow? m-c changeset 09935ede3c77 seems to no longer assert with the testcases.
Comment 8 Brian Hackett (:bhackett) 2011-09-07 02:37:52 PDT
Yeah, a fix for this came in as part of rev 554045e04d89, on a merge from JM.

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