Closed Bug 352873 Opened 13 years ago Closed 13 years ago

Assertion "JSVAL_IS_OBJECT(rval)" or "(jsval *)mark >= sp" involving finally{return} in "with"

Categories

(Core :: JavaScript Engine, defect, P1, critical)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8.1

People

(Reporter: jruderman, Assigned: brendan)

References

(Blocks 1 open bug)

Details

(5 keywords)

Attachments

(1 file, 1 obsolete file)

js> (function() { try { with({}) { try { } finally { return; }  }  } finally {  } })()
Assertion failure: JSVAL_IS_OBJECT(rval), at jsinterp.c:2361

In an opt js shell, this looks like a null deref crash.
Different assertion with a similar testcase but with "with" in a different place.

js> (function(){ try { try { } finally { return; } } finally { with({}) { } } })()
Assertion failure: (jsval *)mark >= sp, at jsinterp.c:1292

In an opt js shell, this doesn't crash or halt execution.
Summary: "Assertion failure: JSVAL_IS_OBJECT(rval)" involving finally{return} in "with" → Assertion "JSVAL_IS_OBJECT(rval)" or "(jsval *)mark >= sp" involving finally{return} in "with"
Attached patch fix (obsolete) — Splinter Review
This is a regression from bug 350312 (which had a followup bug 350837).  It's a one character fix, not including the comment.  Without it the stack during non-local jumps such as return, break, or continue from finally may be too full by one value per finally block being jumped out of.  In the testcase this is fairly benign because the non-local jump is a return.  Not so for non-return transfers.

/be
Assignee: general → brendan
Status: NEW → ASSIGNED
Attachment #238813 - Flags: review?(igor.bukanov)
Attachment #238813 - Flags: approval1.8.1?
Attachment #238813 - Flags: approval1.8.0.8?
Flags: blocking1.8.1?
Flags: blocking1.8.0.8?
OS: Mac OS X 10.4 → All
Priority: -- → P1
Hardware: Macintosh → All
Target Milestone: --- → mozilla1.8.1
Attachment #238813 - Flags: review?(mrbkap)
Depends on: 350312
Attachment #238813 - Attachment is obsolete: true
Attachment #238821 - Flags: review?(igor.bukanov)
Attachment #238821 - Flags: approval1.8.1?
Attachment #238821 - Flags: approval1.8.0.8?
Attachment #238813 - Flags: review?(mrbkap)
Attachment #238813 - Flags: review?(igor.bukanov)
Attachment #238813 - Flags: approval1.8.1?
Attachment #238813 - Flags: approval1.8.0.8?
Attachment #238821 - Flags: review?(mrbkap)
Attachment #238821 - Flags: review?(igor.bukanov) → review+
Comment on attachment 238821 [details] [diff] [review]
fix with better comment

I bet this fixes that let assertion bug I was looking at earlier.
Attachment #238821 - Flags: review?(mrbkap) → review+
Comment on attachment 238821 [details] [diff] [review]
fix with better comment

a=schrep
Attachment #238821 - Flags: approval1.8.1? → approval1.8.1+
Flags: blocking1.8.1? → blocking1.8.1+
Fixed on trunk:

Checking in jsemit.c;
/cvsroot/mozilla/js/src/jsemit.c,v  <--  jsemit.c
new revision: 3.212; previous revision: 3.211
done

/be
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Fixed on the 1.8 branch.

/be
Keywords: fixed1.8.1
*** Bug 352644 has been marked as a duplicate of this bug. ***
Checking in regress-352873-01.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-352873-01.js,v  <--  regress-352873-01.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/js/tests/js1_5/Regress/regress-352873-02.js,v
done
Checking in regress-352873-02.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-352873-02.js,v  <--  regress-352873-02.js
initial revision: 1.1
done
Flags: in-testsuite+
verified fixed 1.8 1.9 20060919 windows/mac*/linux
Status: RESOLVED → VERIFIED
restoring lost blocking flag
Flags: blocking1.8.0.8?
Flags: blocking1.8.0.8? → blocking1.8.0.8+
Keywords: regression
Comment on attachment 238821 [details] [diff] [review]
fix with better comment

approved for 1.8.0 branch, a=dveditz for drivers
Attachment #238821 - Flags: approval1.8.0.9? → approval1.8.0.8+
Fixed on the 1.8.0 branch:

Checking in jsemit.c;
/cvsroot/mozilla/js/src/jsemit.c,v  <--  jsemit.c
new revision: 3.128.2.3.2.11; previous revision: 3.128.2.3.2.10
done

/be
Keywords: fixed1.8.0.8
1.8.0.8 20060927

Assertion failure: strcmp(rval, retsub_pc_cookie) == 0, at /work/mozilla/builds/ff/1.5.0/mozilla/js/src/jsopcode.c:1237

Whiteboard: [notfixed1.8.0.8]
There are lots of decompiler bugs not fixed on the 1.8.0 branch.  You can verify that this fixes the assertion failure shown in comment 0, even if the other bugs still bite.

/be
(In reply to comment #15)

verified fixed 1.8.0.8 20061003 windows/mac*/linux assuming the new assert doesn't hide the older assert.
Whiteboard: [notfixed1.8.0.8]
Note that on 1.8.0.9 winxp the assert is now in PopOff

    /* ss->top points to the next free slot; be paranoid about underflow. */
    top = ss->top;
    JS_ASSERT(top != 0);
No longer blocks: 349611
Blocks: 349611
You need to log in before you can comment on or make changes to this bug.