584607 - JM: "Assertion failure: v.isObject()"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

(function() { var x = {}; x--; })();

With a 64-bit moo build and -m, I get:

Assertion failure: v.isObject(), at ../jsnum.cpp:1178

Comment 1

[Jesse Ruderman]

sstangl, dvander said I should cc you on JM64 bugs.

Comment 2

[Sean Stangl [:sstangl]]

The bug affects 32-bit as well. The value seen by the interpreter upon jsop_localinc() entry is a JSVAL_TYPE_OBJECT, whereas the value seen by methodjit on either platform is a JSVAL_TYPE_UNDEFINED.

On x86_64, the Value.isUndefined() check does a 64-bit comparison, but the payload is nonzero. So the Value is not undefined, and isn't anything else, so it trips an assert.

Note that we optimize away payload stores for known Undefined types -- which breaks this comparison logic. We may have to remove this optimization, or set the payload to zero for known Undefined values before a stub call.

Heading home; will debug further.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: fix

LOCALINC used to be right, then at some point we started being fast and eliding stores, and it became very wrong.

Most of the time we decompose INC ops into something like ; GET ; DUP ; ADD ; 1 etc, but we can do much better for locals. Our optimized version was passing in a vp of the local slot, to ::VpInc, without syncing it.

The new patch does all work on the stack.

Comment 6

[Sean Stangl [:sstangl]]

Comment on attachment 463731 [details] [diff] [review]
fix

Looks good. If only we had some way to assert the validity of the second (currently ignored) argument to frame.merge()..

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/787e35063545