Last Comment Bug 361552 - Crash [@ FindWatchPoint] involving new Script('') and GC
: Crash [@ FindWatchPoint] involving new Script('') and GC
Status: VERIFIED FIXED
[sg:critical?]
: crash, testcase, 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.9alpha1
Assigned To: Brendan Eich [:brendan]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: jsfunfuzz
  Show dependency treegraph
 
Reported: 2006-11-22 11:12 PST by Jesse Ruderman
Modified: 2011-06-13 10:01 PDT (History)
3 users (show)
dveditz: blocking1.8.1.1+
dveditz: blocking1.8.0.9+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (6.79 KB, patch)
2006-11-22 13:59 PST, Brendan Eich [:brendan]
no flags Details | Diff | Splinter Review
fix, v2 (6.93 KB, patch)
2006-11-23 09:13 PST, Brendan Eich [:brendan]
mrbkap: review+
Details | Diff | Splinter Review
1.8 branch version of fix (6.97 KB, patch)
2006-11-23 11:52 PST, Brendan Eich [:brendan]
brendan: review+
dveditz: approval1.8.1.1+
Details | Diff | Splinter Review
js1_5/Regress/regress-361552.js (2.12 KB, text/plain)
2006-11-24 16:11 PST, Bob Clary [:bc:]
no flags Details
1.8.0 branch version of patch (6.43 KB, patch)
2006-11-27 11:48 PST, Brendan Eich [:brendan]
mrbkap: review+
dveditz: approval1.8.0.9+
Details | Diff | Splinter Review

Description Jesse Ruderman 2006-11-22 11:12:29 PST
In a debug js shell:

js> this.__defineSetter__('x', gc); this.watch('x', new Script('')); x = 3;
Bus error

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00000000

Thread 0 Crashed:
0   js 	0x00023de0 FindWatchPoint + 72 (jsdbgapi.c:305)
1   js 	0x00023e6c js_FindWatchPoint + 48 (jsdbgapi.c:316)
2   js 	0x00072bb0 js_AddScopeProperty + 2424 (jsscope.c:1139)
3   js 	0x000731a4 js_ChangeScopePropertyAttrs + 840 (jsscope.c:1284)
4   js 	0x00048b24 js_ChangeNativePropertyAttrs + 124 (jsobj.c:2906)
5   js 	0x00024e9c JS_SetWatchPoint + 1864 (jsdbgapi.c:562)
6   js 	0x00043754 obj_watch + 308 (jsobj.c:1453)
7   js 	0x00095124 js_Invoke + 3912 (jsinterp.c:1396)
8   js 	0x000a8178 js_Interpret + 69376 (jsinterp.c:3948)
9   js 	0x00095d40 js_Execute + 960 (jsinterp.c:1643)
10  js 	0x00021564 JS_ExecuteScript + 64 (jsapi.c:4194)
...

Security-sensitive because some of the other FindWatchPoint crashes were trying to access addresses like 0xdbdbdbdb.
Comment 1 Brendan Eich [:brendan] 2006-11-22 13:59:46 PST
Created attachment 246340 [details] [diff] [review]
fix

obj_watch allows any callable, and so does JS_SetWatchPoint.

/be
Comment 2 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2006-11-22 23:02:05 PST
Comment on attachment 246340 [details] [diff] [review]
fix

>Index: jsdbgapi.c
>+                if (clasp == &js_FunctionClass)
>+                    fun = (JSFunction *) JS_GetPrivate(cx, closure);
>+                else if (clasp == &js_ScriptClass)
>+                    script = (JSScript *) JS_GetPrivate(cx, closure);

If we're dealing with a function, then we'll never set script.
Comment 3 Brendan Eich [:brendan] 2006-11-23 09:13:17 PST
Created attachment 246407 [details] [diff] [review]
fix, v2

Fixed, thanks.

/be
Comment 4 Brendan Eich [:brendan] 2006-11-23 09:18:16 PST
Reminder to land assertion fix from bug 361558 along with this bug's patch on the 1.8 branch.

/be
Comment 5 Blake Kaplan (:mrbkap) (PTO until Jan. 2, 2017) 2006-11-23 09:59:57 PST
Comment on attachment 246407 [details] [diff] [review]
fix, v2

Looks great.
Comment 6 Brendan Eich [:brendan] 2006-11-23 11:08:47 PST
Fixed on trunk:

Checking in jsdbgapi.c;
/cvsroot/mozilla/js/src/jsdbgapi.c,v  <--  jsdbgapi.c
new revision: 3.77; previous revision: 3.76
done
Checking in jsobj.c;
/cvsroot/mozilla/js/src/jsobj.c,v  <--  jsobj.c
new revision: 3.303; previous revision: 3.302
done

/be
Comment 7 Brendan Eich [:brendan] 2006-11-23 11:52:56 PST
Created attachment 246421 [details] [diff] [review]
1.8 branch version of fix

This syncs jsdbgapi.c on the 1.8 branch with the top trunk revision.

/be
Comment 8 Bob Clary [:bc:] 2006-11-24 16:11:46 PST
Created attachment 246499 [details]
js1_5/Regress/regress-361552.js

reliably crashes shell before fixes but not browser.
Comment 9 Bob Clary [:bc:] 2006-11-25 22:39:18 PST
verified fixed 20061125 1.9 windows/linux
Comment 10 Daniel Veditz [:dveditz] 2006-11-27 11:26:41 PST
Comment on attachment 246421 [details] [diff] [review]
1.8 branch version of fix

approved for 1.8 branch, a=dveditz for drivers
Do we need a separate 1.8.0 patch?
Comment 11 Brendan Eich [:brendan] 2006-11-27 11:46:06 PST
Fixed on the 1.8 branch:

Checking in jsdbgapi.c;
/cvsroot/mozilla/js/src/jsdbgapi.c,v  <--  jsdbgapi.c
new revision: 3.56.2.11; previous revision: 3.56.2.10
done
Checking in jsobj.c;
/cvsroot/mozilla/js/src/jsobj.c,v  <--  jsobj.c
new revision: 3.208.2.43; previous revision: 3.208.2.42
done

1.8.0 branch patch next.

/be
Comment 12 Brendan Eich [:brendan] 2006-11-27 11:48:11 PST
Created attachment 246699 [details] [diff] [review]
1.8.0 branch version of patch

Deserves a re-review by mrbkap, and testing by anyone who can help.  Blake, note that lacking JSOP_STOP on the old branch, I just made frame.pc point to the first bytecode.

/be
Comment 13 Daniel Veditz [:dveditz] 2006-11-27 16:00:45 PST
Comment on attachment 246699 [details] [diff] [review]
1.8.0 branch version of patch

approved for 1.8.0 branch, a=dveditz for drivers
Comment 14 Brendan Eich [:brendan] 2006-11-27 16:09:43 PST
Checked into the 1.8.0 branch:

Checking in jsdbgapi.c;
/cvsroot/mozilla/js/src/jsdbgapi.c,v  <--  jsdbgapi.c
new revision: 3.56.2.1.4.5; previous revision: 3.56.2.1.4.4
done
Checking in jsobj.c;
/cvsroot/mozilla/js/src/jsobj.c,v  <--  jsobj.c
new revision: 3.208.2.12.2.18; previous revision: 3.208.2.12.2.17
done

/be
Comment 15 Bob Clary [:bc:] 2006-11-29 04:36:13 PST
verified fixed 20061128 1.8.0.9 windows/linux/mac*, 1.8.1.1 windows/linux/mac*, 1.9 windows/linux
Comment 16 Bob Clary [:bc:] 2007-02-08 18:09:00 PST
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-361552.js,v  <--  regress-361552.js

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