703857 - Assertion failure: [infer failure] Missing type pushed 0: float, at jsinfer.cpp:348

Status: RESOLVED DUPLICATE of bug 692274

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following test asserts on mozilla-central revision b62e6ee5ba9b (options -m -n -a):


Function.prototype.X = 42;
function ownProperties() {
    var props = {};
    var r = function () {};
    for (var a in r) {
        let (a = function() { for (var r=0;r<6;++r) ++a; }) { 
            a(); 
        }
        props[a] = true;
    }
}
ownProperties();

Comment 1

[Brian Hackett [Laid off!]]

The 'a' in ownProperties is not marked as closed, but the '++a' in the inner function can mutate it.  This is a similar problem to the one in bug 685321, with I think similar severity (and, similarly, was caught by the TI interpreter assertions but is not a TI bug).

I don't really know whether it is correct behavior to mark 'a' as closed, or for 'a' to be not closed but for the '++a' to access the letvar 'a'.  I also don't know how far this goes back, though I think there has been relatively recent work in this area (letvar initializer binds the letvar itself).  luke, jorendorff?

It will be nice when it is not actually possible for a SetCall{Var,Arg} to change a variable which has not been marked as closed (bug 659577).

Comment 2

[Jason Orendorff [:jorendorff]]

I bet this is a dup. Luke?

Comment 3

[Luke Wagner [:luke]]

Oops, sorry I missed this earlier.  Yes, it seems like it.  Testing with the almost-done patch for bug 692274 applied, I get a different assert:

  Unknown bytecode ba at #2:00035
  Assertion failure: array, at ../jsanalyze.h:1004
  Aborted

Any idea about what that means Brian?

Comment 4

[Brian Hackett [Laid off!]]

The unknown bytecode message shows up when new bytecodes are seen in the instruction stream which don't have a case in ScriptAnalysis::analyzeTypesBytecode.

Comment 5

[Luke Wagner [:luke]]

Oh duh; I didn't even read the message.  With that embarrassing omission fixed there is no assert.  I'll add the testcase in comment 0 to bug 692274.