472787 - Hang involving gczeal

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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 [:dveditz]]

You should be able to change zeal dynamically using a pref now in opt builds.

Comment 2

[Daniel Veditz [:dveditz]]

I was wrong, javascript.options.gczeal only works if JS_GC_ZEAL is defined at compile time

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

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) (inactive)]

Attachment: Fix

Easy fix.

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 356280 [details] [diff] [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]]

Comment on attachment 356280 [details] [diff] [review]
Fix

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

/be

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tracemonkey/rev/411b2f1e19de

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: 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 [:gkw] [:nth10sd] (NOT official MoCo now)]

(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 [:Waldo]]

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 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/411b2f1e19de

Comment 13

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/extensions/regress-472787.js

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 356280 [details] [diff] [review]
Fix

No longer needs approval (is a blocker).

Comment 15

[Daniel Veditz [:dveditz]]

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 16

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ce3a8a21675f

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 356294 [details] [diff] [review]
branch backport to 1.9.0.x

Approved for 1.9.0.7, a=dveditz for release-drivers.

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

I checked this into the 1.9.0 branch.

Comment 19

[Bob Clary [:bc] (inactive)]

v 1.9.0, 1.9.1, 1.9.2

Comment 20

[Daniel Veditz [:dveditz]]

Does not seem to affect the 1.8 branch

Comment 21

[Bob Clary [:bc] (inactive)]

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