Last Comment Bug 472787 - Hang involving gczeal
: Hang involving gczeal
Status: VERIFIED FIXED
: [sg:critical?] fixed-in-tracemonkey, ...
: hang, testcase, verified1.9.0.7, verified1.9.1
Product: Core
Classification: Components
Component: JavaScript Engine
: Trunk
: x86 Mac OS X
: P1 critical (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
: general
:
:
: jsfunfuzz
  Show dependency treegraph
 
Reported: 2009-01-08 21:31 PST by Gary Kwong [:nth10sd]
Modified: 2009-03-05 17:49 PST (History)
11 users (show)
sayrer: blocking1.9.1+
dveditz: wanted1.9.0.x+
dveditz: wanted1.8.1.x-
bclary: in‑testsuite+
bclary: in‑litmus-
See Also:
Crash Signature:


Attachments
Fix (980 bytes, patch)
2009-01-09 17:04 PST, Blake Kaplan (:mrbkap)
brendan: review+
Details | Diff | Splinter Review
branch backport to 1.9.0.x (1.05 KB, patch)
2009-01-09 19:59 PST, Gary Kwong [:nth10sd]
mrbkap: review+
dveditz: approval1.9.0.7+
Details | Diff | Splinter Review
js1_5/extensions/regress-472787.js (2.19 KB, text/plain)
2009-01-21 14:57 PST, Bob Clary [:bc:]
no flags Details

Summon comment box

Description Gary Kwong [:nth10sd] 2009-01-08 21:31:05 PST
this.__defineSetter__("x", Math.sin)
this.watch("x", '' .concat)
gczeal(2)
x = 1

This hangs 1.9.0.x debug and trunk debug (with and without -j). Doesn't seem to affect opt because opt by default doesn't compile with the gczeal function.

Security-sensitive because of the gczeal function. Thanks Jesse for helping with the process of reduction.
Comment 1 Daniel Veditz 2009-01-09 11:16:41 PST
You should be able to change zeal dynamically using a pref now in opt builds.
Comment 2 Daniel Veditz 2009-01-09 11:52:57 PST
I was wrong, javascript.options.gczeal only works if JS_GC_ZEAL is defined at compile time
Comment 3 Gary Kwong [:nth10sd] 2009-01-09 12:25:21 PST
Eventually I removed the #ifdef DEBUG surrounding http://mxr.mozilla.org/mozilla-central/source/js/src/jsapi.h#2694 in my js source tree and recompiled opt.

The testcase in comment #0 hangs opt trunk compiled with gczeal (with and without -j) as well.
Comment 4 Blake Kaplan (:mrbkap) 2009-01-09 17:04:11 PST
Created attachment 356280 [details] [review]
Fix

Easy fix.
Comment 5 Brendan Eich [:brendan] 2009-01-09 17:50:11 PST
Comment on attachment 356280 [details] [review]
Fix

Might be worth commenting that early *vp setting is only for the argc == 0 case without extra silly argc testing logic.

/be
Comment 6 Brendan Eich [:brendan] 2009-01-09 17:50:54 PST
Comment on attachment 356280 [details] [review]
Fix

gczeal is our bestest testing friend on all active repos/branches.

/be
Comment 7 Blake Kaplan (:mrbkap) 2009-01-09 18:33:18 PST
http://hg.mozilla.org/tracemonkey/rev/411b2f1e19de
Comment 8 Gary Kwong [:nth10sd] 2009-01-09 19:59:04 PST
Created attachment 356294 [details] [review]
branch backport to 1.9.0.x

I backported mrbkap's patch to the 1.9.0.x branch and it does fix the hang.

e.g. for debug 1.9.0.x js shell,

$ ./js
js> this.__defineSetter__("x", Math.sin)
js> this.watch("x", '' .concat)
js> gczeal(2)
js> x = 1
NaN
js> 

"NaN" is now immediately shown.
Comment 9 Gary Kwong [:nth10sd] 2009-01-11 01:04:48 PST
(In reply to comment #1)
> You should be able to change zeal dynamically using a pref now in opt builds.

(In reply to comment #2)
> I was wrong, javascript.options.gczeal only works if JS_GC_ZEAL is defined at
> compile time

Bug 472877 is now fixed so most probably one can execute:

./configure --enable-optimize --disable-debug --enable-gczeal

to build an opt js shell with gczeal.
Comment 10 Jeff Walden (remove +bmo to email) 2009-01-12 14:27:13 PST
Just a note for anyone watching, but the reason 190x would result in NaN instead of 1 (as on trunk) is that the result of an assignment expression changed on trunk.  It used to be the value returned by the setter, but now it's the RHS of the expression (as ECMA-262 requires).
Comment 12 Gary Kwong [:nth10sd] 2009-01-19 09:50:17 PST
*** Bug 474319 has been marked as a duplicate of this bug. ***
Comment 13 Bob Clary [:bc:] 2009-01-21 14:57:11 PST
Created attachment 358058 [details]
js1_5/extensions/regress-472787.js
Comment 14 Gavin Sharp 2009-01-22 11:06:20 PST
Comment on attachment 356280 [details] [review]
Fix

No longer needs approval (is a blocker).
Comment 15 Daniel Veditz 2009-01-23 11:49:17 PST
Guessing P1 if this is going to land on 1.9.1 in time to bake for landing in 1.9.0.7 as requested
Comment 17 Daniel Veditz 2009-01-26 11:38:41 PST
Comment on attachment 356294 [details] [review]
branch backport to 1.9.0.x

Approved for 1.9.0.7, a=dveditz for release-drivers.
Comment 18 Blake Kaplan (:mrbkap) 2009-01-27 15:43:19 PST
I checked this into the 1.9.0 branch.
Comment 19 Bob Clary [:bc:] 2009-02-06 15:17:29 PST
v 1.9.0, 1.9.1, 1.9.2
Comment 20 Daniel Veditz 2009-02-12 10:43:41 PST
Does not seem to affect the 1.8 branch
Comment 21 Bob Clary [:bc:] 2009-03-05 17:49:39 PST
http://hg.mozilla.org/tracemonkey/rev/e373ca8fa578
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-472787.js,v  <--  regress-472787.js
initial revision: 1.1

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