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
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?
Is there a way to swap rval into the root protecting rtmp, once rtmp is protected by something else? If not, more stack. /be
trying to get owners for all sg:bugs
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.
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.
Fix checked into trunk.
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
This affects the 1.8.0 and 1.8 branches, too.
Comment on attachment 265554 [details] [diff] [review] Proposed fix This applies to the 1.8 and 1.8.0 branches as is.
Comment on attachment 265554 [details] [diff] [review] Proposed fix approved for 22.214.171.124 and 126.96.36.199, a=dveditz
Fixed on the 1.8 branches.
verified fixed 1.8.1, 1.9.0 windows/linux/macppc opt/debug browser/shell 7/16
/cvsroot/mozilla/js/tests/js1_5/GetSet/regress-375976.js,v <-- regress-375976.js initial revision: 1.1