621121 - Assert isObject() with function increment

Status: RESOLVED DUPLICATE of bug 618007

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following code asserts in shell on mozilla-central-trunk:

var foo;

testFoo(0);
testFoo(0);

function testFoo(i)
{
  foo = getFoo(i);
}

function getFoo(i)
{
  getFoo++;
}


Assertion failure: isObject(), at jsvalue.h:602

Comment 1

[Andrew Drake [:adrake]]

Bisecting on tracemonkey yields:

changeset:   52243:db403b4edf3e
parent:      52220:9889018ba3cf
parent:      52242:68f7019ffed5
user:        David Anderson <[e-mail]>
date:        Mon Jul 05 18:07:39 2010 -0700
summary:     [JAEGER] Merge from fatval & TM.

changeset:   52244:0ea22d1ff97e
user:        David Anderson <[e-mail]>
date:        Mon Jul 05 20:14:42 2010 -0700
summary:     [JAEGER] Unbroke build, merge merge merge.

Comment 2

[Andrew Drake [:adrake]]

After further digging behind that merge, the actual first bad revision is:

changeset:   51981:56ac5ae1825d
user:        David Anderson <[e-mail]>
date:        Thu Jun 10 23:43:14 2010 -0700
summary:     [JAEGER] Statically bind some global functions (bug 562729, r=brendan).

Comment 3

[Nicholas Nethercote [inactive]]

To clarify:  it asserts with 'js' and 'js -j'.  It runs correctly with 'js -m' or any other invocation involving '-m'.

It's a regression.  I bisected it to the JM merge :(

Comment 4

[Nicholas Nethercote [inactive]]

Looks like adrake beat me to it and did a better job!

Comment 5

[Nicholas Nethercote [inactive]]

Nominating as blocking2.0 because it's a regression.

Comment 6

[Andrew Drake [:adrake]]

The problem is caused by INCGLOBAL/DECGLOBAL/GLOBALINC/GLOBALDEC not properly invalidating the shape if incrementing something other than an integer. I copied the logic to do this from SETGLOBAL/FORGLOBAL, and it appears to have fixed the regression.

Patch against tip forthcoming.

Comment 7

[Andrew Drake [:adrake]]

Attachment: Patch v0

Patch against tip. Doesn't regress any jstests and fixes given test case.

Comment 8

[Andrew Drake [:adrake]]

Attachment: Patch v1

With regression test for test suite.

Comment 9

[Andrew Drake [:adrake]]

Bug 618007 removes the INCGLOBAL/DECGLOBAL/etc ops entirely, rendering this fix obsolete when that one lands.