Last Comment Bug 375976 - "Assertion failure: *flagp != GCF_FINAL" with post-increment
: "Assertion failure: *flagp != GCF_FINAL" with post-increment
Status: VERIFIED FIXED
[sg:critical?]
: assertion, crash, fixed1.8.0.13, testcase, verified1.8.1.5
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Mac OS X
: -- critical (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: jsfunfuzz
  Show dependency treegraph
 
Reported: 2007-03-30 04:28 PDT by Jesse Ruderman
Modified: 2007-09-21 07:26 PDT (History)
8 users (show)
dveditz: blocking1.8.1.5+
dveditz: wanted1.8.1.x+
dveditz: blocking1.8.0.13+
dveditz: wanted1.8.0.x+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed fix (1.81 KB, patch)
2007-05-21 15:07 PDT, Blake Kaplan (:mrbkap)
crowderbt: review+
brendan: review+
dveditz: approval1.8.1.5+
dveditz: approval1.8.0.13+
Details | Diff | Splinter Review
js1_5/GetSet/regress-375976.js (2.40 KB, text/plain)
2007-05-27 16:40 PDT, Bob Clary [:bc:]
no flags Details

Description Jesse Ruderman 2007-03-30 04:28:15 PDT
To reproduce: paste this into jsshell

this.__defineSetter__('x', gc);
this.__defineGetter__('x', Math.sin);
x = x++;


Debug:

Assertion failure: *flagp != GCF_FINAL, at jsgc.c:2499


Opt:

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00023011

Thread 0 Crashed:
0   js 	0x00023011 js_EmitTree + 11342
1   js 	0x0002de89 js_MarkStackFrame + 325
2   js 	0x0002e3eb js_GC + 816
3   js 	0x000058c9 JS_GC + 66
4   js 	0x0000279e GC + 34
5   js 	0x0003bb73 js_Invoke + 1843
6   js 	0x0003bfac js_InternalInvoke + 181
7   js 	0x0003c136 js_InternalGetOrSet + 261
8   js 	0x00041765 js_NativeSet + 165
9   js 	0x00045b61 js_SetProperty + 1084
10  js 	0x00031dfc js_Interpret + 6390
11  js 	0x0003b37a js_Execute + 513
12  js 	0x000079c3 JS_ExecuteScript + 56
13  js 	0x00002095 Process + 1105
14  js 	0x00004df9 main + 2270
15  js 	0x00001c26 _start + 216
16  js 	0x00001b4d start + 41
Comment 1 Blake Kaplan (:mrbkap) 2007-04-18 08:13:54 PDT
So, the post-increment code goes through some effort to root rtmp (the old value) but no attempt to root rval, which is the new value. So, I think that we call OBJ_SET_PROPERTY with an unrooted rval, which gets collected. I'm not really sure what the cleanest way to fix this is.

We could make JSOP_INC* take another stack slot and push rval too, or is there a cleaner way?
Comment 2 Brendan Eich [:brendan] 2007-05-02 13:15:38 PDT
Is there a way to swap rval into the root protecting rtmp, once rtmp is protected by something else? If not, more stack.

/be
Comment 3 chris hofmann 2007-05-03 13:56:11 PDT
trying to get owners for all sg:bugs
Comment 4 Blake Kaplan (:mrbkap) 2007-05-21 14:59:57 PDT
My initial diagnosis was not correct. The parameter to OBJ_SET_PROPERTY is, in fact, safe from GC (first, because it's a parameter to the setter, then because it's stored in a GC-safe object). The problem is actually that the old value needs to be protected across the OBJ_SET_PROPERTY, not just across the js_NewNumberValue.
Comment 5 Blake Kaplan (:mrbkap) 2007-05-21 15:07:33 PDT
Created attachment 265554 [details] [diff] [review]
Proposed fix

With this patch, the testcase runs cleanly for me, since rtmp is now protected over both the js_NewNumberValue and the OBJ_SET_PROPERTY.
Comment 6 Blake Kaplan (:mrbkap) 2007-05-21 16:45:55 PDT
Fix checked into trunk.
Comment 7 Brendan Eich [:brendan] 2007-05-22 01:19:24 PDT
Comment on attachment 265554 [details] [diff] [review]
Proposed fix

Whew -- I *thought* we did not need another root -- a relief to know that's true still.

/be
Comment 8 Bob Clary [:bc:] 2007-05-27 16:40:40 PDT
Created attachment 266303 [details]
js1_5/GetSet/regress-375976.js
Comment 9 Daniel Veditz [:dveditz] 2007-06-28 01:39:26 PDT
This affects the 1.8.0 and 1.8 branches, too.
Comment 10 Blake Kaplan (:mrbkap) 2007-07-10 17:55:12 PDT
Comment on attachment 265554 [details] [diff] [review]
Proposed fix

This applies to the 1.8 and 1.8.0 branches as is.
Comment 11 Daniel Veditz [:dveditz] 2007-07-10 18:11:00 PDT
Comment on attachment 265554 [details] [diff] [review]
Proposed fix

approved for 1.8.1.5 and 1.8.0.13, a=dveditz
Comment 12 Blake Kaplan (:mrbkap) 2007-07-10 18:32:06 PDT
Fixed on the 1.8 branches.
Comment 13 Bob Clary [:bc:] 2007-07-17 10:45:10 PDT
verified fixed 1.8.1, 1.9.0 windows/linux/macppc opt/debug browser/shell 7/16
Comment 14 Bob Clary [:bc:] 2007-09-21 07:26:57 PDT
/cvsroot/mozilla/js/tests/js1_5/GetSet/regress-375976.js,v  <--  regress-375976.js
initial revision: 1.1

Note You need to log in before you can comment on or make changes to this bug.