Status

()

P1
critical
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: gkw, Assigned: mrbkap)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
x86
Mac OS X
hang, testcase, verified1.9.0.7, verified1.9.1
Points:
---
Bug Flags:
blocking1.9.1 +
wanted1.9.0.x +
wanted1.8.1.x -
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] fixed-in-tracemonkey, post 1.8-branch)

Attachments

(3 attachments)

(Reporter)

Description

10 years ago
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.
Flags: blocking1.9.1?
Flags: blocking1.9.0.7?
You should be able to change zeal dynamically using a pref now in opt builds.
Flags: blocking1.9.0.7? → wanted1.9.0.x+
I was wrong, javascript.options.gczeal only works if JS_GC_ZEAL is defined at compile time
(Reporter)

Comment 3

10 years ago
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.
Created attachment 356280 [details] [diff] [review]
Fix

Easy fix.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #356280 - Flags: review?(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
Attachment #356280 - Flags: review?(brendan) → review+
Comment on attachment 356280 [details] [diff] [review]
Fix

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

/be
Attachment #356280 - Flags: approval1.9.1?
(Reporter)

Comment 8

10 years ago
Created attachment 356294 [details] [diff] [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.
Attachment #356294 - Flags: review?(mrbkap)
Attachment #356294 - Flags: approval1.9.0.7?
Attachment #356294 - Flags: review?(mrbkap) → review+
(Reporter)

Comment 9

10 years ago
(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

10 years ago
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).

Updated

10 years ago
Whiteboard: fixed-in-tracemonkey → [sg:critical?] fixed-in-tracemonkey

Updated

10 years ago
Flags: blocking1.9.1? → blocking1.9.1+

Comment 11

10 years ago
http://hg.mozilla.org/mozilla-central/rev/411b2f1e19de
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical?] fixed-in-tracemonkey → [sg:critical?][needs 1.9.1 baking] fixed-in-tracemonkey
(Reporter)

Updated

10 years ago
Duplicate of this bug: 474319

Comment 13

10 years ago
Created attachment 358058 [details]
js1_5/extensions/regress-472787.js

Updated

10 years ago
Flags: in-testsuite+
Flags: in-litmus-
Whiteboard: [sg:critical?][needs 1.9.1 baking] fixed-in-tracemonkey → [sg:critical?][needs 1.9.1 landing] fixed-in-tracemonkey
Comment on attachment 356280 [details] [diff] [review]
Fix

No longer needs approval (is a blocker).
Attachment #356280 - Flags: approval1.9.1?
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
Priority: -- → P1

Comment 16

10 years ago
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ce3a8a21675f
Keywords: fixed1.9.1
Whiteboard: [sg:critical?][needs 1.9.1 landing] fixed-in-tracemonkey → [sg:critical?] fixed-in-tracemonkey
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.
Attachment #356294 - Flags: approval1.9.0.7? → approval1.9.0.7+
I checked this into the 1.9.0 branch.
Keywords: fixed1.9.0.7

Comment 19

10 years ago
v 1.9.0, 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.0.7, fixed1.9.1 → 4xp, verified1.9.0.7, verified1.9.1

Updated

10 years ago
Keywords: 4xp
Does not seem to affect the 1.8 branch
Flags: wanted1.8.1.x-
Whiteboard: [sg:critical?] fixed-in-tracemonkey → [sg:critical?] fixed-in-tracemonkey, post 1.8-branch
Group: core-security

Comment 21

10 years ago
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
You need to log in before you can comment on or make changes to this bug.