Last Comment Bug 361964 - Crash [@ MarkGCThingChildren] involving watch and setter
: Crash [@ MarkGCThingChildren] involving watch and setter
Status: VERIFIED FIXED
[sg:critical?]
: crash, regression, verified1.8.0.9, verified1.8.1.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P1 critical (vote)
: mozilla1.8.1
Assigned To: Blake Kaplan (:mrbkap) (please use needinfo!)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-11-27 08:05 PST by moz_bug_r_a4
Modified: 2011-06-13 10:01 PDT (History)
4 users (show)
jaymoz: blocking1.9?
jaymoz: blocking1.8.1.1+
jaymoz: blocking1.8.0.9+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.29 KB, text/html)
2006-11-27 08:06 PST, moz_bug_r_a4
no flags Details
backtrace from debug build (11.11 KB, text/plain)
2006-11-27 08:07 PST, moz_bug_r_a4
no flags Details
Possible fix (2.94 KB, patch)
2006-11-27 10:37 PST, Blake Kaplan (:mrbkap) (please use needinfo!)
no flags Details | Diff | Review
Updated (3.05 KB, patch)
2006-11-27 17:37 PST, Blake Kaplan (:mrbkap) (please use needinfo!)
brendan: review+
jaymoz: approval1.8.1.1+
Details | Diff | Review
Patch for 1.8.0 (3.01 KB, patch)
2006-11-30 14:27 PST, Blake Kaplan (:mrbkap) (please use needinfo!)
jaymoz: approval1.8.0.9+
Details | Diff | Review
js1_5/Regress/regress-361964.js (2.69 KB, text/plain)
2006-12-01 08:25 PST, Bob Clary [:bc:]
no flags Details
D'oh (1.40 KB, patch)
2006-12-02 11:37 PST, Blake Kaplan (:mrbkap) (please use needinfo!)
brendan: review+
jaymoz: approval1.8.0.9+
jaymoz: approval1.8.1.1+
Details | Diff | Review

Description moz_bug_r_a4 2006-11-27 08:05:28 PST
Crash:
  document.watch("title", function(a,b,c,d) {
    return { toString : function() { alert(1); } };
  });
  document.title = "xxx";

No crash:
  document.watch("title", function() {
    return { toString : function() { alert(1); } };
  });
  document.title = "xxx";

Regression range on trunk is from 2006-11-06-04 to 2006-11-07-04.
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2006-11-06+04&maxdate=2006-11-07+04

Regression range on 1.8.0 branch is from 2006-11-24-07 to 2006-11-25-08
http://bonsai.mozilla.org/cvsquery.cgi?module=AviarySuiteBranchTinderbox&branch=MOZILLA_1_8_0_BRANCH&date=explicit&mindate=2006-11-24+07&maxdate=2006-11-25+08

Regressed from bug 354978?

I'm marking this security sensitive for now, since I'm not sure if this could
be an exploitable crash or not.

Steps to reproduce:
0. Load a testcase.
1. Click a button.
2. An alert dialog appears.
3. Don't close the alert dialog.
4. After a few seconds, crash.
Comment 1 moz_bug_r_a4 2006-11-27 08:06:43 PST
Created attachment 246677 [details]
testcase
Comment 2 moz_bug_r_a4 2006-11-27 08:07:59 PST
Created attachment 246678 [details]
backtrace from debug build
Comment 3 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-11-27 10:37:29 PST
Created attachment 246687 [details] [diff] [review]
Possible fix

I haven't actually tested this, but I think the problem is that we're trying to mark fun->nargs + fun->extra slots, when we're only providing two memory spaces.
Comment 4 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-11-27 10:39:47 PST
Comment on attachment 246687 [details] [diff] [review]
Possible fix

>+                frame.scopeChain = OBJ_GET_PARENT(cx, closure);

This line is not actually part of this patch.
Comment 5 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-11-27 17:37:36 PST
Created attachment 246733 [details] [diff] [review]
Updated
Comment 6 Brendan Eich [:brendan] 2006-11-27 17:45:37 PST
Comment on attachment 246733 [details] [diff] [review]
Updated

r=me, thanks.

/be
Comment 7 Brendan Eich [:brendan] 2006-11-29 13:26:59 PST
Need this on the trunk when it opens, and on the branches sooner.

/be
Comment 8 Jay Patel [:jay] 2006-11-30 11:41:33 PST
mrbkap/brendan:  Is the patch good as-is for the branches?  If so, please ask for approval for 1.8.0.9 and 1.8.1.1.  If we need separate patches, please attach and ask for separate approvals.  Thanks!
Comment 9 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-11-30 13:44:12 PST
Fixed on trunk.
Comment 10 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-11-30 14:15:00 PST
Comment on attachment 246733 [details] [diff] [review]
Updated

This patch applies cleanly to the 1.8 branch.
Comment 11 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-11-30 14:20:33 PST
Comment on attachment 246733 [details] [diff] [review]
Updated

This patch applies cleanly to the 1.8 branch.
Comment 12 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-11-30 14:27:57 PST
Created attachment 247101 [details] [diff] [review]
Patch for 1.8.0

The only changes are:
- fun->u.n.extra -> fun->extra (since the unionizing of JSFunction didn't happen on the 1.8.0 branch).
- JS_ARRAY_LENGTH doesn't exist on the 1.8.0 branch, so I hand-expanded it.

I don't think this needs another review.
Comment 13 Jay Patel [:jay] 2006-11-30 14:28:32 PST
Comment on attachment 246733 [details] [diff] [review]
Updated

Approved for 1.8 branch, a=jay for drivers.
Comment 14 Jay Patel [:jay] 2006-11-30 14:29:07 PST
Comment on attachment 247101 [details] [diff] [review]
Patch for 1.8.0

Approved for 1.8.0 branch, a=jay for drivers.
Comment 15 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-11-30 14:35:38 PST
Fixed on the 1.8 branches.
Comment 16 Bob Clary [:bc:] 2006-12-01 08:25:28 PST
Created attachment 247184 [details]
js1_5/Regress/regress-361964.js
Comment 17 moz_bug_r_a4 2006-12-02 01:00:29 PST
I can still reproduce crash.

This seems to be due to missing |* sizeof(jsval)|.

- argv = JS_malloc(cx, nslots);
+ argv = JS_malloc(cx, nslots * sizeof(jsval));

- memset(argv + 2, 0, nslots - 2);
+ memset(argv + 2, 0, (nslots - 2) * sizeof(jsval));
Comment 18 Bob Clary [:bc:] 2006-12-02 05:57:10 PST
In trunk winxp debug I get MSVC CRT Debug error: Heap Corruption Detected... detected that the application wrote to memory after the end of the heap buffer. 

In 1.8.0.9|1.8.1.1 winxp debug I get MSCC CRT Debug error: DAMAGE after normal block

This should block the Firefox 1.5.0.9, Firefox 2.0.0.1 and Gran Paradiso releases.

Comment 19 Jay Patel [:jay] 2006-12-02 11:28:48 PST
Removing "fixed1.8.x.x" keywords since this doesn't appear to be fixed.  

Blake/Brendan:  We are past our code freeze date, but really need to get this fixed, so please let us know if either of you can take a look at this.  It would be nice to have a patch together by Monday morning, since we were hoping to spin RC1 then.
Comment 20 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-12-02 11:37:45 PST
Created attachment 247282 [details] [diff] [review]
D'oh

I blame all of the Java code I've had to write lately.
Comment 21 Brendan Eich [:brendan] 2006-12-02 15:14:46 PST
Comment on attachment 247282 [details] [diff] [review]
D'oh

I have no Java-on-the-brainn excuse :-(.

This needs prompt approval for the branches.

/be
Comment 22 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-12-02 16:16:25 PST
Fixed (again) on trunk.
Comment 23 Jay Patel [:jay] 2006-12-02 17:58:58 PST
Comment on attachment 247282 [details] [diff] [review]
D'oh

Approved for both branches, a=jay for drivers.  Thanks for the quick patch patch mrbkap. ;-)
Comment 24 Blake Kaplan (:mrbkap) (please use needinfo!) 2006-12-02 18:39:49 PST
Fixed, again.

Thanks for the heads-up moz_bug.
Comment 25 Bob Clary [:bc:] 2006-12-03 10:58:37 PST
verified fixed 20061203 1.8.0.9 windows/linux, 1.8.1.1 windows/linux, 1.9 windows/linux
Comment 26 Bob Clary [:bc:] 2007-02-08 22:35:56 PST
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-361964.js,v  <--  regress-361964.js

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