Closed Bug 519244 Opened 12 years ago Closed 12 years ago

Assertion failure: JSVAL_IS_INT(v), at ../jsapi.h:242 - js1_8/extensions/regress-452476.js

Categories

(Core :: JavaScript Engine, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta5-fixed

People

(Reporter: bc, Assigned: dvander)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

js1_8/extensions/regress-452476.js
Assertion failure: JSVAL_IS_INT(v), at ../jsapi.h:242

crashes opt.

regression changeset: 33135:0dfaa6e65031 user: David Anderson <danderson@mozilla.com> date: Fri Sep 25 18:20:06 2009 -0700 summary: Fix bogus return values with SETELEM without a POP when deep bailing (bug 518492, r=gal)
Flags: in-testsuite+
Summary: Assertion failure: JSVAL_IS_INT(v) - Assertion failure: JSVAL_IS_INT(v), at ../jsapi.h:242 → Assertion failure: JSVAL_IS_INT(v), at ../jsapi.h:242 - js1_8/extensions/regress-452476.js
Flags: wanted1.9.2?
Assignee: general → dvander
Attached patch fixSplinter Review
|cs| has to reflect the JSOP_POP, not the JSOP_SETELEM. You can't rebind references though, so I've re-ordered the logic.
Attachment #408040 - Flags: review?(gal)
Why fix this in LeaveTree? Can't we make sure we record this right?
(In reply to comment #2)
> Why fix this in LeaveTree? Can't we make sure we record this right?

see bug 518492

The snapshot needs to be taken in post-state (i.e. next opcode), and the interpreter doesn't execute the JSOP_POP because it's coalesced. The problem is either that pendingGuardCondition is kind of a hack, or that JSOP_SETELEM should really be two opcodes (JSOP_SETELEMPOP and JSOP_SETELEM) because coalescing is really irritating.
Comment on attachment 408040 [details] [diff] [review]
fix

Nit drive-by, and a comment.

>diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp
>--- a/js/src/jstracer.cpp
>+++ b/js/src/jstracer.cpp
>@@ -6571,30 +6571,31 @@ LeaveTree(InterpState& state, VMSideExit
>             JSOp op = (JSOp) *regs->pc;
>             JS_ASSERT(op == JSOP_CALL || op == JSOP_APPLY || op == JSOP_NEW ||
>                       op == JSOP_GETPROP || op == JSOP_GETTHISPROP || op == JSOP_GETARGPROP ||
>                       op == JSOP_GETLOCALPROP || op == JSOP_LENGTH ||
>                       op == JSOP_GETELEM || op == JSOP_CALLELEM ||
>                       op == JSOP_SETPROP || op == JSOP_SETNAME || op == JSOP_SETMETHOD ||
>                       op == JSOP_SETELEM || op == JSOP_INITELEM ||
>                       op == JSOP_INSTANCEOF);

Blank line here.

>+            /*
>+             * JSOP_SETELEM can be coalesced with a JSOP_POP in the interpeter.
>+             * Since this doesn't re-enter the recorder, the post-state snapshot
>+             * is invalid. Fix it up here.
>+             */
>+            if (op == JSOP_SETELEM && JSOp(*(regs->pc + JSOP_SETELEM_LENGTH) == JSOP_POP)) {

It's more "honest" to do *(ptr + index) but I prefer ptr[index] here, i.e. regs->pc[JSOP_SETELEM_LENGTH], to cut down on the noise. Must be some old Pascal nerve twitching because (* starts a comment.

>+                regs->sp -= js_CodeSpec[JSOP_SETELEM].nuses;
>+                regs->sp += js_CodeSpec[JSOP_SETELEM].ndefs;
>+                regs->pc += JSOP_SETELEM_LENGTH;
>+                op = JSOP_POP;
>+            }

Blank line here too.

>             const JSCodeSpec& cs = js_CodeSpec[op];

To get rid of coalesced op dispatch would mean more ops, and some duplicated code one way or another, which we have today due to coalescing (the smaller trailing op, JSOP_POP or JSOP_IFEQ/IFNE, is coalesced by a macro such as TRY_BRANCH_AFTER_COND).

Let's say the dup. code is a draw. More ops could be done but we need to get rid of the E4X ops (see bug 441416), and it seems strictly more complex to have more ops when coalescing is about as cheap (modulo JIT maintenance :-/).

/be
Attachment #408040 - Flags: review?(gal) → review+
pushed with nits as:

http://hg.mozilla.org/tracemonkey/rev/ee7f090e12ff
Whiteboard: fixed-in-tracemonkey
Should block 1.9.2 because bug 518492 landed there.
Flags: blocking1.9.2?
this is now asserting on 32bit linux Assertion failure: ((jsval) obj & JSVAL_TAGMASK) == JSVAL_OBJECT, at js/src/jsapi.h:204 on mozilla-central. Since the mc->tm merge this morning, I assume it is happening there too.
http://hg.mozilla.org/mozilla-central/rev/ee7f090e12ff
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Flags: blocking1.9.2? → blocking1.9.2+
These bugs landed after b4 was cut. Moving flag out.
Flags: wanted1.9.2?
You need to log in before you can comment on or make changes to this bug.