Closed Bug 684525 Opened 11 years ago Closed 11 years ago

"Assertion failure: shape->compartment() == compartment()," or "Assertion failure: compartment mismatched,"

Categories

(Core :: JavaScript Engine, defect)

x86
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: gkw, Unassigned)

Details

(Keywords: assertion, testcase, Whiteboard: js-triage-done)

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(),
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.
Whiteboard: js-triage-needed → js-triage-done
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.
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.
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.
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
(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)
Did this get fixed somehow? m-c changeset 09935ede3c77 seems to no longer assert with the testcases.
Yeah, a fix for this came in as part of rev 554045e04d89, on a merge from JM.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.