The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: gkw, Unassigned)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
x86
Windows 7
assertion, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: js-triage-done)

(Reporter)

Description

6 years ago
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.

Comment 4

6 years ago
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)
(Reporter)

Comment 7

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.