Closed Bug 647695 Opened 9 years ago Closed 9 years ago

TM: Crash [@ js::types::TypeFailure] involving missing type XML:prototype:new


(Core :: JavaScript Engine, defect, critical)

Not set





(Reporter: gkw, Unassigned)


(Blocks 3 open bugs)


(Keywords: crash, testcase)

Crash Data


(1 file)

let(x = XMLList(7), y = let(a = x)("")) {}

crashes js debug shell on JM changeset a58525f1f4be with -m, -a and -n at js::types::TypeFailure with the message:

[infer failure] Missing type at #2:00018 pushed 0: XML:prototype:new
Tested on 64-bit.
This looks like a bug in TM. Here is the disassembly:

let(x = XMLList(7), y = let(a = x)("")) {}

00000:  enterblock depth 0 {y: 1, x: 0}
00003:  callgname "XMLList"
00006:  int8 7
00008:  call 1
00011:  setlocal 0
00014:  pop
00015:  enterblock depth 2 {a: 0}
00018:  getgname "x"
00021:  setlocal 2
00024:  pop
00025:  string ""
00028:  leaveblockexpr 1
00033:  setlocal 1
00036:  pop
00037:  leaveblock 2

The access to 'x' in 'a = x' is a JSOP_GETGNAME.  The interpreter sees this and executes it the same way as a JSOP_NAME, getting the 'x' in the let block.  There is no assertion in the interpreter for JSOP_GETGNAME that it is actually getting a property on a global object, this inconsistency has bitten before on XML filters (bug 605200) and should be fixed.

What are the right semantics here?  If the 'x' in 'a = x' should in fact be getting a property on the global then the interpreter's JSOP_GETGNAME case is broken, and if it should be getting the let block 'x' then the emitter is broken.  Cc'ing brendan/dherman.
Duplicate of this bug: 648968
Summary: TI: Crash [@ js::types::TypeFailure] involving missing type XML:prototype:new → TM: Crash [@ js::types::TypeFailure] involving missing type XML:prototype:new
bhackett, is this related?

    y: Float64Array
for each(let y in [y]) {}


[infer failure] Missing type at #2:00020 pushed 0: Float64Array
Yeah, this looks like the same problem, the 'y' in '[y]' is a GETGNAME.  This looks somewhat different (and worse) though, as it is ignoring not just the 'let y' but also the 'with'.
It gets better!

js> var x = 1;
js> (let (x=42, y=x) y)
js> (let (x=42, q=function(c) { eval(c); }, y=x) y)

Essentially, SpiderMonkey makes no effort to evaluate the initializers of a let in any particular scope.
I need to do a partial fix here and remove the crash without fixing the underlying semantic issue in Spidermonkey.  The basic problem is that the interpreter's semantics (maybe the JIT too) do not match the semantics which type inference assumes for GNAME opcodes --- inference assumes the receiver object will be the script's global, whereas the interpreter treats them like NAME.  That issue needs to get fixed first, as we don't want correctness of type inference to depend on having emitted correct bytecode in the first place.  (This will also fix the crash in bug 655499, though again not the semantic bug).
I think the essential problem here is that the bytecode emitter produces BLOCKCHAIN instructions (really, annotations for the preceding bytecode) that include lexical blocks whose initializers we're not done evaluating.

js> dis(function(){(let (x=42, q=function(c) { x; }, y=x) y)})
off     op
-----   --
00000:  enterblock depth 0 {y: 2, q: 1, x: 0}
00003:  int8 42
00005:  setlocal 0
00008:  pop
00009:  lambda (function (c) {x;})
00012:  blockchain depth 0 {y: 2, q: 1, x: 0}
00015:  setlocal 1
00018:  pop
00019:  getgname "x"
00022:  setlocal 2
00025:  pop
00026:  getlocal 2
00029:  leaveblockexpr 3
00034:  pop
00035:  stop

If that 'blockchain' were a 'nullblockchain', then this would be okay. In general, the bytecode emitter needs to put off extending the scope chain until the initializers are done. We can't treat the enterblock as the start of the scope.
Attached patch patchSplinter Review
Always skip to the global object for GNAME opcodes.  Previously we sometimes did this (mjit, some interpreter cases) and sometimes didn't (most interpreter cases and some mjit stubs), but inference assumes GNAME accesses are on the global and that should straightforwardly match what the interpreter and jits actually do.

A few debugger tests relied on this broken interpreter/mjit behavior, introducing variable bindings on a function frame which were supposed to be seen by a GNAME opcode.  This patch just adds eval() to these tests to deoptimize the GNAMEs, but a real solution is needed for handling variable bindings introduced by EvaluateInFrame.  Filed bug 662822 on that.
Attachment #538040 - Flags: review?(dmandelin)
Attachment #538040 - Flags: review?(dmandelin) → review?(dvander)
Crash Signature: [@ js::types::TypeFailure]
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 646968
Blocks: 349611
A testcase for this bug was already added in the original bug (bug 646968).
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.