361552 - Crash [@ FindWatchPoint] involving new Script('') and GC

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

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]]

Attachment: fix

obj_watch allows any callable, and so does JS_SetWatchPoint.

/be

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

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]]

Attachment: fix, v2

Fixed, thanks.

/be

Comment 4

[Brendan Eich [:brendan]]

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

Comment on attachment 246407 [details] [diff] [review]
fix, v2

Looks great.

Comment 6

[Brendan Eich [:brendan]]

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]]

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

Attachment: js1_5/Regress/regress-361552.js

reliably crashes shell before fixes but not browser.

Comment 9

[Bob Clary [:bc] (inactive)]

verified fixed 20061125 1.9 windows/linux

Comment 10

[Daniel Veditz [:dveditz]]

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]]

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]]

Attachment: 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]]

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]]

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

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

/cvsroot/mozilla/js/tests/js1_5/extensions/regress-361552.js,v  <--  regress-361552.js