Closed
Bug 312278
Opened 19 years ago
Closed 19 years ago
Access of GC-ed object in Array.prototype.toSource
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla1.8rc1
People
(Reporter: igor, Assigned: brendan)
Details
(Keywords: verified1.7.13, verified1.8, Whiteboard: [sg:critical?])
Attachments
(8 files, 3 obsolete files)
443 bytes,
text/html
|
Details | |
1.68 KB,
patch
|
Details | Diff | Splinter Review | |
5.56 KB,
patch
|
igor
:
review+
shaver
:
superreview+
dveditz
:
approval-aviary1.0.8+
dveditz
:
approval1.7.13+
asa
:
approval1.8rc1+
|
Details | Diff | Splinter Review |
3.22 KB,
patch
|
brendan
:
review+
dveditz
:
approval-aviary1.0.8+
dveditz
:
approval1.7.13+
brendan
:
approval1.8rc1+
|
Details | Diff | Splinter Review |
1.84 KB,
patch
|
brendan
:
review+
asa
:
approval1.8rc1+
|
Details | Diff | Splinter Review |
6.57 KB,
patch
|
igor
:
review+
shaver
:
superreview+
dveditz
:
approval-aviary1.0.8+
dveditz
:
approval1.7.13+
brendan
:
approval1.8rc1+
|
Details | Diff | Splinter Review |
15.18 KB,
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
2.53 KB,
text/plain
|
Details |
array_join_sub from jsarray.c uses unrooted jsval to hold a value of array element to apply recursive toSource to the element later in the code. A specially crafted script can take advantage of it to GC the element before it is passed as "this" variable to js_InternalCall. It would allow the script to replace on a 32 platform 8 bytes of JSObect that the unrooted jsval points to by 8 bytes from chosen double value. This is not as bad compared with other reported unrooted access bugs since this does not allow to subvert double reference to point to other objects to learn their addresses through double value bits. Still theoretically on platforms were the system heap is not randomized one can try to preallocate many strings where string body when interpreted as JSObjectMap would allow for JSObjectMap->ops->someOperation() to be successfully executed. Then one learn the address this strings will most likely occupy on a particular platform, use that address to build custom double value and replace via exploit JSObject->map to point to that expected address. But I have no idea how feasible it would be.
Reporter | ||
Comment 1•19 years ago
|
||
The test case proceeds as following: 1. Code defines array with getter for its single element. The getter would return a newly allocated specially crafted object. The object would be unrooted after array_join_sub calls JS_GetElement and stores its reference in jsval. 2. To construct that special object the code first creates an object "obj" with a getter for toSource property. The getter forces GC when called and return a function to be used as toSource implementation. Then code wraps "obj" into With constructor and return that. 3. array_join_sub tries to call toSource method on the unrooted With object. As a part of implementation the runtime calls OBJ_GET_PROPERTY that executes with_GetProperty. That method calls in turn OBJ_GET_PROPERTY on that object with toSource getter where GC is forced and With object is GC-ed. Then the getter return toSource implementation. 4. Code calls custom toSource passing GC-ed With there as this parameter. That crashes since JSObject for the original With is replaced by bits from Math.sqrt(2).
Reporter | ||
Updated•19 years ago
|
Attachment #199400 -
Attachment mime type: text/plain → text/html
Updated•19 years ago
|
Assignee: general → mrbkap
Flags: blocking1.9a1+
Flags: blocking1.8rc1+
Flags: blocking1.7.13?
Flags: blocking-aviary1.0.8?
Whiteboard: [sg:critical?]
Updated•19 years ago
|
Priority: -- → P1
Target Milestone: --- → mozilla1.8rc1
Assignee | ||
Comment 2•19 years ago
|
||
Related to or dup of bug 311161 ? /be
Comment 3•19 years ago
|
||
Updated•19 years ago
|
Flags: testcase+
Reporter | ||
Comment 4•19 years ago
|
||
(In reply to comment #2) > Related to or dup of bug 311161 ? I can not tell since I do not have access to it ;)
Reporter | ||
Comment 5•19 years ago
|
||
(In reply to comment #2) > Related to or dup of bug 311161 ? No, it is different one. That bug is another exposure of that "extra" problem from bug 311497. (Well, I would not bet a fortune on it, but at least I can bet 500$ ;) This bug is a reflection of the fact that OBJ_GET_PROPERTY/JS_GetElement etc. return unrooted jsval if "jsval* vp" argument is itself unrooted and the value is a result of getter call with no other references left. It looks like most of the places where OBJ_GET_PROPERTY stores its results in local C varaiables are not exploitable. Typically they are followed by js_ValueToNumber/js_ValueToString etc. which roots the object before triggering GC allocation and after the conversion the code does not access the original unrooted value. So with one-threaded JS embeddings there is no bug. The problem with Array.prototype.toSource (and with Object.prototype.toSource as well) is that via evil With there is a code path which does not root With instance before calling JS code. It allows to trigger controllable GC to wipe out the With instance. To fix the problem I suggest to always store results of js_InternalInvoke in a new rooted JSContext field. Then the code can continue to assume that OBJ_GET_PROPERTY returns a rooted jsval. And it would close the bug for multithreaded applications.
Reporter | ||
Comment 6•19 years ago
|
||
Here is a simple fix that stores results of js_InternalCall in an extra JSContext field. It solves the problem for the price of a memory leak since the field is always a part of GC root set until context is entered. Perhaps the field should be cleared when the last frame exits and cx->fp is set to NULL?
Updated•19 years ago
|
Attachment #199604 -
Flags: review?(brendan)
Assignee | ||
Comment 7•19 years ago
|
||
(In reply to comment #5) > This bug is a reflection of the fact that OBJ_GET_PROPERTY/JS_GetElement etc. > return unrooted jsval if "jsval* vp" argument is itself unrooted and the value > is a result of getter call with no other references left. But the GC model is not preemptive at arbitrary points -- requests (JS_Begin- and JS_EndRequest) keep the GC from running on another thread, so the only GC hazard is nesting a last-ditch GC on the current thread. It's hard to see all allocations in calls to subroutines, of course. This model is fragile. > It looks like most of the places where OBJ_GET_PROPERTY stores its results in > local C varaiables are not exploitable. Typically they are followed by > js_ValueToNumber/js_ValueToString etc. which roots the object before triggering > GC allocation and after the conversion the code does not access the original > unrooted value. So with one-threaded JS embeddings there is no bug. Right. > The problem with Array.prototype.toSource (and with Object.prototype.toSource as > well) is that via evil With there is a code path which does not root With > instance before calling JS code. It allows to trigger controllable GC to wipe > out the With instance. Could we not fix just this case? > To fix the problem I suggest to always store results of js_InternalInvoke in a > new rooted JSContext field. Then the code can continue to assume that > OBJ_GET_PROPERTY returns a rooted jsval. And it would close the bug for > multithreaded applications. The multithreaded case is handled by the request model. In that light, we might rather a more particular fix. The risk of your patch is entrainment of garbage, although your suggestion of clearing the lastInvokeResult root when popping the last frame from cx should help. But it's more code for a generalized solution that we might avoid with a focused fix. /be
Assignee | ||
Comment 8•19 years ago
|
||
This is the fix I was thinking of. It fixes the array testcase, but I haven't been able to construct an obvious variation on that case using an object instead of an array. Tomorrow... someone please beat me to it. /be
Attachment #199634 -
Flags: superreview?(shaver)
Attachment #199634 -
Flags: review?(igor.bukanov)
Reporter | ||
Comment 9•19 years ago
|
||
(In reply to comment #8) > Created an attachment (id=199634) [edit] > targeted fix > > This is the fix I was thinking of. It fixes the array testcase, but I haven't > been able to construct an obvious variation on that case using an object > instead of an array. Tomorrow... someone please beat me to it. I was wrong about Object.prototype.toSource case. It is safe from the problem as the exploit needs a call to a getter. But toSource here does not call the getter but rather converts it to a string for serialization. So unless there is way to have an object with a getter where where OBJ_IS_NATIVE(object) would give false, accessing unrooted value is safe there. But that was not the last fish in the pond. Error.ptototype.toSource has exactly the same problem as Array.prototype.toSource. That is, it stores results of JS_GetProperty in an unrooted local. Here is a trivial modification of that evil With example adopted to crash js shell: function customToSource() { return "customToSource "+this; } Error.prototype.__defineGetter__('message', function() { var obj = { toSource: "something" } obj.__defineGetter__('toSource', function() { gc(); return customToSource; }); return With(obj); }); print(Error.prototype.toSource());
Comment 10•19 years ago
|
||
Reporter | ||
Comment 11•19 years ago
|
||
Here is another low hanging fruit that exploits the fact that exn_toString from jsexn.c does not root name and message. So just couple of getters are sufficient for the demo: Error.prototype.__defineGetter__('name', function() { return String.fromCharCode(40); // 40 is code for '(' }); Error.prototype.__defineGetter__('message', function() { gc(); for (var i = 0; i != 40000; ++i) { var tmp = Math.sqrt(2); tmp = null; } return String.fromCharCode(45); // 45 is code for '-' }); print(Error.prototype.toString()); // should print "(: -" This prints "abs: -" instead of expected "(: -" where that "abs" comes from reading sqrt(2) as JSString. Note that here that idea with storing results of InternalInvoke would not prevent unrooted access since the code uses 2 unrooted values.
Comment 12•19 years ago
|
||
exn_toString is pretty simple to fix (as long as name is rooted, there's no chance for message to get collected before its copied over to the result string). It looks like an auditing of OBJ_GET_PROPERTY is necessary, *sigh*.
Comment 13•19 years ago
|
||
Igor, can you help determine the extent of this problem? We're really pushing for safe spot-fixes to get into 1.8. I can fix exn_to{String,Source}. Are there other places that need patching as well (I can help look).
Reporter | ||
Comment 14•19 years ago
|
||
(In reply to comment #13) > Igor, can you help determine the extent of this problem? What is "this problem" here? > Are there other places that need patching as well (I can help look). I wish I knew... My real concern is that when the bug would be known, people would start to look for the pattern. Since unrooted bugs can be quite bad and I honestly I do not see how one can be certain that all of them are spotted, I would prefer some proactive solution. On the other hand to fix proactively problems like Error.prototype.toString bug from comment 11 it would be necessary to add something like calls to js_Enter/LeaveLocalRootScope before any call to a native function in js_InternalInvoke. But that would probably lead to denial of service in trivial cases through rooted garbage. Another proactive solution is to add js_EnterLocalRootScope lazily when new object is created from JS code that was called from native code called which in turn was called via InternalInvoke from JS code. Then corresponding jsLeaveLocalRootScope would be executed in InternalInvoke if necessary. But that also may produce too much rooted garbage. Or would it?
Reporter | ||
Comment 15•19 years ago
|
||
(In reply to comment #13) > Are there other places that need patching as well (I can help look). I would suggest to create a patch with js_Enter/LeaveLocalRootScope around calls to native function in js_InternalInvoke and then try a browser with the patch against reported and reproducible crashes in bugzilla. If the patch would fix the crash, then one has unrooted access.
Reporter | ||
Comment 16•19 years ago
|
||
(In reply to comment #15) > (In reply to comment #13) > > Are there other places that need patching as well (I can help look). > > I would suggest to create a patch with js_Enter/LeaveLocalRootScope around calls > to native function in js_InternalInvoke and then try a browser with the patch > against reported and reproducible crashes in bugzilla. If the patch would fix > the crash, then one has unrooted access. Or add a runtime option to disable GC when JS code calls native functions or other potentially complex C code. In addition for providing extra debugging facility such option would allow to verify for a bug reporter the nature of potential bug without sending all the test cases and to prevent exploits if a bad bug would be found and announced before a fix is ready.
Comment 17•19 years ago
|
||
Attachment #200129 -
Flags: review?(brendan)
Assignee | ||
Comment 18•19 years ago
|
||
Comment on attachment 200129 [details] [diff] [review] fix exn_to{String,Source} r+a=me. /be
Attachment #200129 -
Flags: review?(brendan)
Attachment #200129 -
Flags: review+
Attachment #200129 -
Flags: approval1.8rc1+
Reporter | ||
Updated•19 years ago
|
Attachment #199634 -
Flags: review?(igor.bukanov) → review+
Reporter | ||
Comment 19•19 years ago
|
||
AFAICS there is one more problem with unrooted locals in jsexn.c. js_InitExceptionClasses there calls JS_NewStringCopyZ for messageStr and then for filenameStr without rooting messageStr. After that JS_DefineProperty is called for messageStr which could trigger GC during slot allocation before messageStr is rooted, right?
Assignee | ||
Comment 20•19 years ago
|
||
(In reply to comment #19) > AFAICS there is one more problem with unrooted locals in jsexn.c. > js_InitExceptionClasses there calls JS_NewStringCopyZ for messageStr and then > for filenameStr without rooting messageStr. You mean js_ErrorToException, not js_InitExceptionClasses, right? > After that JS_DefineProperty is called for messageStr which could trigger GC > during slot allocation before messageStr is rooted, right? Yes, good catch -- the way this code was written violates the existing model (http://www.mozilla.org/js/spidermonkey/gctips.html). Once the exnObject is rooted by cx->exception, each string should be constructed and defined in turn, rather than making two or more strings and then adding them as property values. /be
Comment 21•19 years ago
|
||
brendan, we're lost in patches here. What more needs to happen here for this to be resolved for 1.5?
Assignee | ||
Comment 22•19 years ago
|
||
1. shaver needs to review attachment 199634 [details] [diff] [review] ("targeted fix"), so we have two reviews; I'm checking into the trunk now with Igor's r+. 2. mrbkap needs to land attachment 200129 [details] [diff] [review] ("fix exn_to{String,Source}") on the 1.8 branch. 3. It falls on mrbkap, I bet, to fix the remaining problem Igor points out in comment 19 which I elaborate on in comment 20. This is all I think we should jam in this bug for 1.8/fx1.5. We may need another bug or two, depending on what more testing and code inspection turns up, but new problems should go in other bugs. That will simplify the testing and release managing by avoiding clouds of patches. /be
Assignee | ||
Comment 23•19 years ago
|
||
Comment on attachment 199634 [details] [diff] [review] targeted fix Checked into the trunk. Pre-nominating so shaver can review today and we can consider tomorrow based on expected zero-regression short bake cycle results on the trunk. /be
Attachment #199634 -
Flags: approval1.8rc1?
Comment on attachment 199634 [details] [diff] [review] targeted fix sr=shaver
Attachment #199634 -
Flags: superreview?(shaver) → superreview+
Comment 25•19 years ago
|
||
Attachment 200129 [details] [diff] is now checked in on trunk and branch. I'll fix the remaining
known problem in jsexn.
Comment 26•19 years ago
|
||
Local root scopes make everything OK.
Attachment #200293 -
Flags: review?(brendan)
Assignee | ||
Comment 27•19 years ago
|
||
Comment on attachment 200293 [details] [diff] [review] fix js_ErrorToException r=me. /be
Attachment #200293 -
Flags: review?(brendan)
Attachment #200293 -
Flags: review+
Attachment #200293 -
Flags: approval1.8rc1?
Updated•19 years ago
|
Attachment #199634 -
Flags: approval1.8rc1? → approval1.8rc1+
Assignee | ||
Comment 28•19 years ago
|
||
Comment on attachment 199634 [details] [diff] [review] targeted fix Fixed on the branch too. Are we done with this bug, and ready to give it the fixed1.8 keyword? /be
Updated•19 years ago
|
Attachment #200293 -
Flags: approval1.8rc1? → approval1.8rc1+
Comment 29•19 years ago
|
||
As far as I know, all fixes have been checked into to trunk and MOZILLA_1_8_BRANCH. Marking this bug as FIXED and fixed1.8 (also based on comment 28).
Reporter | ||
Comment 30•19 years ago
|
||
The attachment 200129 [details] [diff] [review] does not fix exn.toSource problem as it does not add rooting of v during GetProperty/toSource.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 31•19 years ago
|
||
second-review? mrbkap is welcome too. /be
Attachment #200320 -
Flags: superreview?(shaver)
Attachment #200320 -
Flags: review?(igor.bukanov)
Comment 32•19 years ago
|
||
Why wouldn't v be protected by newborn-ness through the ValueToSource calls?
Comment 33•19 years ago
|
||
Comment on attachment 200320 [details] [diff] [review] fix to several problems including the one Igor pointed out r=mrbkap, I think I see why...
Attachment #200320 -
Flags: review+
Reporter | ||
Comment 34•19 years ago
|
||
Comment on attachment 200320 [details] [diff] [review] fix to several problems including the one Igor pointed out This is not enough since getter may return unrooted value without creating new objects via var tmp = somePrviouslyInitializedReference; somePrviouslyInitializedReference = null; return tmp;
Attachment #200320 -
Flags: review?(igor.bukanov) → review-
Reporter | ||
Comment 35•19 years ago
|
||
(In reply to comment #32) > Why wouldn't v be protected by newborn-ness through the ValueToSource calls? Because you can create a getter that would create many objects overwriting all the newborn entities. Plus getter does not even need to create new objects to return unrooted jsval, it may simply forget all rooted references to existing one.
Assignee | ||
Comment 36•19 years ago
|
||
(In reply to comment #34) > (From update of attachment 200320 [details] [diff] [review] [edit]) > > This is not enough since getter may return unrooted value without creating new > objects via > > var tmp = somePrviouslyInitializedReference; > somePrviouslyInitializedReference = null; > return tmp; But the result value of the getter is stored in a local root (in exn_toSource; the other part of the patch uses a local root scope). What precisely is the GC hazard you see in the patch, at what line or lines? /be
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 37•19 years ago
|
||
(In reply to comment #36) > But the result value of the getter is stored in a local root (in exn_toSource; An explicit local root (rval, argv[argc], argv[argc + 1], argv[argc + 2]). So long as the GC can't run between the evil getter's return to JS_GetProperty and JS_GetProperty's return to exn_toSource, there is no GC hazard. /be
Reporter | ||
Comment 38•19 years ago
|
||
(In reply to comment #37) So > long as the GC can't run between the evil getter's return to JS_GetProperty and > JS_GetProperty's return to exn_toSource, there is no GC hazard. Sorry I did not paste the part of patch I was refering to. That was about the fix for js_ReportUncaughtException. The code accesses "message" and "filename" properties of the exception object. Since a script can define getter for both of them with the first returning unrooted value without using "new" and the second triggering GC, it is not enough to use js_EnterLocalRootScope there to protect message bytes from GC.
Assignee | ||
Comment 39•19 years ago
|
||
(In reply to comment #38) > The code accesses "message" and "filename" properties of the exception object. > Since a script can define getter for both of them with the first returning > unrooted value without using "new" and the second triggering GC, it is not > enough to use js_EnterLocalRootScope there to protect message bytes from GC. Local root scoping works across calls out of the C function that enters them. So this scope is open in subroutines called from js_ReportUncaughtException, and it protects all newborns until that function leaves the local root scope. Can you reverse your - on the patch? /be
Reporter | ||
Comment 40•19 years ago
|
||
(In reply to comment #39) > Local root scoping works across calls out of the C function that enters them. > So this scope is open in subroutines called from js_ReportUncaughtException, and > it protects all newborns until that function leaves the local root scope. But what about the following code that creates an object before local scope for js_ReportUncaughtException is entered during normal script execution: var error = new Error(); var prepared_message = String(Math.sqrt(2)); String(Math.sqrt(3)); // to remove JSString in prepared_message from newborn error.__defineGetter__('message', function() { var tmp = prepred_message; prepared_message = null; return tmp; }); error.__defineGetter__('filename', function() { for (var i = 0; i != 40000; ++i) { new Object(); } return "test"; }); throw error; Now I can not check it right now, but I do not see how the patch would prevent messageStr from GC.
Not accessible to reporter
Assignee | ||
Comment 41•19 years ago
|
||
Argh, so much for local root stack -- explicit local roots (rval, argv) handle already-born, otherwise unprotected things, but the local root stack saves only newborns born within a local root scope. New patch shortly. /be
Assignee | ||
Updated•19 years ago
|
Attachment #200320 -
Attachment is obsolete: true
Attachment #200320 -
Flags: superreview?(shaver)
Attachment #200320 -
Flags: review+
Assignee | ||
Comment 42•19 years ago
|
||
Relieving mrbkap of this one, since I'm already patching. Thanks to him and Igor for help so far. /be
Assignee: mrbkap → brendan
Status: ASSIGNED → NEW
Assignee | ||
Comment 43•19 years ago
|
||
When rooting several values at once and no explicit local roots (no argv passed to a native function), prefer to allocate scanned operand stack space. I hacked on the test-sketch a bit: var error = new Error(); var prepared_message = String(Math.sqrt(2)); String(Math.sqrt(3)); // to remove JSString in prepared_message from newborn var mjunk = 2; error.__defineGetter__('message', function() { if (--mjunk != 0) return "dumb"; print("blorp"); var tmp = prepared_message; prepared_message = null; return tmp; }); error.__defineGetter__('fileName', function() { print("glorp"); for (var i = 0; i != 4000000; ++i) { new Object(); } return "test"; }); throw error; (Changes: fileName, not filename; and mjunk is because message is got twice, once by the js_ValueToString(cx, exn), then in the (!reportp && exnObject && [error class matches])-conditioned block, where we want the get to cut all live object graph connections to the prepared_message string.) /be
Attachment #200392 -
Flags: superreview?(shaver)
Attachment #200392 -
Flags: review?(igor.bukanov)
Assignee | ||
Updated•19 years ago
|
Accessible to reporter
Reporter | ||
Updated•19 years ago
|
Attachment #200392 -
Flags: review?(igor.bukanov) → review+
Assignee | ||
Updated•19 years ago
|
Attachment #199604 -
Flags: review?(brendan)
Comment on attachment 200392 [details] [diff] [review] better patch sr=shaver
Attachment #200392 -
Flags: superreview?(shaver) → superreview+
Assignee | ||
Comment 45•19 years ago
|
||
Comment on attachment 200392 [details] [diff] [review] better patch This went in yesterday. I'll close the bug. /be
Attachment #200392 -
Flags: approval1.8rc1+
Assignee | ||
Comment 46•19 years ago
|
||
Oops, that jsexn.c patch was not checked in already (I meant to, since Igor's r+ suffices for the trunk, but forgot to). It's in now, and on the branch. /be
Status: NEW → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 47•19 years ago
|
||
This bug's patch caused bug 313565 -- investigating. /be
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 48•19 years ago
|
||
(In reply to comment #47) > This bug's patch caused bug 313565 -- investigating. As Jesse pointed out over IRC, it was the jsstr.c patch. /be
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 49•19 years ago
|
||
Sigh, my checkin message for jsstr.c fingered this bug by mistake. It should have identified bug 313276. /be
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 19 years ago
Resolution: --- → FIXED
Comment 50•19 years ago
|
||
winxp, 1.8b5_2005102412: pass js1_5/Error/regress-312278.js 1.9a1_2005102413: crash js1_5/Error/regress-312278.js
Comment 51•19 years ago
|
||
(In reply to comment #50) please ignore this comment.
Updated•19 years ago
|
Flags: blocking1.7.13?
Flags: blocking1.7.13+
Flags: blocking-aviary1.0.8?
Flags: blocking-aviary1.0.8+
Comment 52•18 years ago
|
||
Comment on attachment 200392 [details] [diff] [review] better patch aviary101/moz17 landing approval: a=dveditz for drivers. Please add the fixed1.7.13 and fixed-aviary1.0.8 keywords when landed.
Attachment #200392 -
Flags: approval1.7.13+
Attachment #200392 -
Flags: approval-aviary1.0.8+
Updated•18 years ago
|
Attachment #199634 -
Flags: approval1.7.13+
Attachment #199634 -
Flags: approval-aviary1.0.8+
Updated•18 years ago
|
Attachment #200129 -
Flags: approval1.7.13+
Attachment #200129 -
Flags: approval-aviary1.0.8+
Assignee | ||
Comment 53•18 years ago
|
||
Attachment #211341 -
Flags: review?(mrbkap)
Updated•18 years ago
|
Attachment #211341 -
Flags: review?(mrbkap) → review+
Comment 55•18 years ago
|
||
removing fixed-aviary1.0.8, until someone decides if this crash is related or not. Probably same issue on mozilla 1.7.13, but I haven't tested it yet. crash in shell 1.7.13/winxp from 2006-02-14 static JSBool ComputeThis(JSContext *cx, JSObject *thisp, JSStackFrame *fp) { JSObject *parent; => if (thisp && OBJ_GET_CLASS(cx, thisp) != &js_CallClass) { /* Some objects (e.g., With) delegate 'this' to another object. */ thisp = OBJ_THIS_OBJECT(cx, thisp); if (!thisp) return JS_FALSE; + cx 0x00037100 + fp 0x0013e3d4 + &js_CallClass 0x6108c710 + parent 0x0000000c + thisp 0x00039600 ComputeThis(JSContext * 0x00037100, JSObject * 0x00039600, JSStackFrame * 0x0013e3d4) line 600 + 20 bytes js_Invoke(JSContext * 0x00037100, unsigned int 0x00000000, unsigned int 0x00000002) line 860 + 23 bytes js_InternalInvoke(JSContext * 0x00037100, JSObject * 0x00039600, long 0x00039230, unsigned int 0x00000000, unsigned int 0x00000000, long * 0x00000000, long * 0x0013e544) line 1049 + 20 bytes js_TryMethod(JSContext * 0x00037100, JSObject * 0x00039600, JSAtom * 0x0003af90, unsigned int 0x00000000, long * 0x00000000, long * 0x0013e544) line 3827 + 31 bytes js_ValueToSource(JSContext * 0x00037100, long 0x00039600) line 2723 + 36 bytes exn_toSource(JSContext * 0x00037100, JSObject * 0x000394f8, unsigned int 0x00000000, long * 0x0042403c, long * 0x0013e664) line 691 + 44 bytes js_Invoke(JSContext * 0x00037100, unsigned int 0x00000000, unsigned int 0x00000000) line 955 + 23 bytes js_Interpret(JSContext * 0x00037100, long * 0x0013ee24) line 3020 + 15 bytes js_Execute(JSContext * 0x00037100, JSObject * 0x00038750, JSScript * 0x0041e070, JSStackFrame * 0x00000000, unsigned int 0x00000000, long * 0x0013fecc) line 1173 + 13 bytes JS_ExecuteScript(JSContext * 0x00037100, JSObject * 0x00038750, JSScript * 0x0041e070, long * 0x0013fecc) line 3545 + 25 bytes Process(JSContext * 0x00037100, JSObject * 0x00038750, char * 0x00034da5) line 351 + 22 bytes ProcessArgs(JSContext * 0x00037100, JSObject * 0x00038750, char * * 0x00034d3c, int 0x00000003) line 568 + 17 bytes main(int 0x00000003, char * * 0x00034d3c, char * * 0x00033320) line 2431 + 21 bytes JS! mainCRTStartup + 227 bytes KERNEL32! 7c816d4f() crash ff 1.0.8/winxp JS_FRIEND_API(jsval) js_GetSlotThreadSafe(JSContext *cx, JSObject *obj, uint32 slot) { jsval v; JSScope *scope; #ifndef NSPR_LOCK JSThinLock *tl; jsword me; #endif /* * We handle non-native objects via JSObjectOps.getRequiredSlot, treating * all slots starting from 0 as required slots. A property definition or * some prior arrangement must have allocated slot. * * Note once again (see jspubtd.h, before JSGetRequiredSlotOp's typedef) * the crucial distinction between a |required slot number| that's passed * to the get/setRequiredSlot JSObjectOps, and a |reserved slot index| * passed to the JS_Get/SetReservedSlot APIs. */ if (!OBJ_IS_NATIVE(obj)) => return OBJ_GET_REQUIRED_SLOT(cx, obj, slot); + cx 0x03baed70 + obj 0x03bae3b0 slot 0x00000002 00000180() js_GetSlotThreadSafe(JSContext * 0x03baed70, JSObject * 0x03bae3b0, unsigned long 0x00000002) line 554 + 37 bytes ComputeThis(JSContext * 0x03baed70, JSObject * 0x03bae3b0, JSStackFrame * 0x0012e664) line 600 + 241 bytes js_Invoke(JSContext * 0x03baed70, unsigned int 0x00000000, unsigned int 0x00000002) line 860 + 23 bytes js_InternalInvoke(JSContext * 0x03baed70, JSObject * 0x03bae3b0, long 0x03bae240, unsigned int 0x00000000, unsigned int 0x00000000, long * 0x00000000, long * 0x0012e7d4) line 1049 + 20 bytes js_TryMethod(JSContext * 0x03baed70, JSObject * 0x03bae3b0, JSAtom * 0x00ee1230, unsigned int 0x00000000, long * 0x00000000, long * 0x0012e7d4) line 3827 + 31 bytes js_ValueToSource(JSContext * 0x03baed70, long 0x03bae3b0) line 2723 + 36 bytes exn_toSource(JSContext * 0x03baed70, JSObject * 0x03bae2c0, unsigned int 0x00000000, long * 0x03d8f344, long * 0x0012e914) line 691 + 44 bytes js_Invoke(JSContext * 0x03baed70, unsigned int 0x00000000, unsigned int 0x00000000) line 955 + 23 bytes js_Interpret(JSContext * 0x03baed70, long * 0x0012f18c) line 3020 + 15 bytes js_Execute(JSContext * 0x03baed70, JSObject * 0x03bac520, JSScript * 0x03d97a38, JSStackFrame * 0x00000000, unsigned int 0x00000000, long * 0x0012f2a4) line 1173 + 13 bytes JS_EvaluateUCScriptForPrincipals(JSContext * 0x03baed70, JSObject * 0x03bac520, JSPrincipals * 0x02e29428, const unsigned short * 0x03db60e8, unsigned int 0x00000973, const char * 0x03dbc000, unsigned int 0x00000001, long * 0x0012f2a4) line 3654 + 25 bytes nsJSContext::EvaluateString(const nsAString & {...}, void * 0x03bac520, nsIPrincipal * 0x02e29420, const char * 0x03dbc000, unsigned int 0x00000001, const char * 0x100ba430, nsAString & {...}, int * 0x0012f2f0) line 946 + 67 bytes nsScriptLoader::EvaluateScript(nsScriptLoadRequest * 0x03db9750, const nsString & {...}) line 668 nsScriptLoader::ProcessRequest(nsScriptLoadRequest * 0x03db9750) line 581 + 22 bytes nsScriptLoader::OnStreamComplete(nsScriptLoader * const 0x03d442c4, nsIStreamLoader * 0x03dbc700, nsISupports * 0x03db9750, unsigned int 0x00000000, unsigned int 0xffffffff, const char * 0x03db19a4) line 905 nsStreamLoader::OnStopRequest(nsStreamLoader * const 0x03dbc704, nsIRequest * 0x03dbc0a0, nsISupports * 0x03db9750, unsigned int 0x00000000) line 144 nsStreamListenerTee::OnStopRequest(nsStreamListenerTee * const 0x03db9fd0, nsIRequest * 0x03dbc0a0, nsISupports * 0x03db9750, unsigned int 0x00000000) line 66 nsHttpChannel::OnStopRequest(nsHttpChannel * const 0x03dbc0a8, nsIRequest * 0x03dba180, nsISupports * 0x00000000, unsigned int 0x00000000) line 3739 nsInputStreamPump::OnStateStop() line 499 nsInputStreamPump::OnInputStreamReady(nsInputStreamPump * const 0x03dba184, nsIAsyncInputStream * 0x03dba05c) line 339 + 11 bytes nsInputStreamReadyEvent::EventHandler(PLEvent * 0x03dba264) line 119 PL_HandleEvent(PLEvent * 0x03dba264) line 673 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x00ef3d90) line 608 + 9 bytes _md_EventReceiverProc(HWND__ * 0x0011056a, unsigned int 0x0000c146, unsigned int 0x00000000, long 0x00ef3d90) line 1414 + 9 bytes USER32! 77d48734() USER32! 77d48816() USER32! 77d489cd() USER32! 77d48a10() nsAppShell::Run(nsAppShell * const 0x02e4c368) line 135 nsAppShellService::Run(nsAppShellService * const 0x02e4c2a8) line 495 xre_main(int 0x00000004, char * * 0x003e6e38, const nsXREAppData * 0x0041e01c kAppData) line 1907 + 35 bytes main(int 0x00000004, char * * 0x003e6e38) line 58 + 18 bytes mainCRTStartup() line 338 + 17 bytes KERNEL3
Keywords: fixed-aviary1.0.8
Comment 56•18 years ago
|
||
Neither Blake nor I crash with these testcases.
Keywords: fixed-aviary1.0.8
Comment 57•18 years ago
|
||
I do not crash using the testcase in the bug - Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.13) Gecko/20060214 Firefox/1.0.8.
Comment 58•18 years ago
|
||
verified with nightly builds. firefox/1.7.13 : 20060216 win/linux/mac firefox/1.8.0.1: 20060214 winxp/20060215 linux/20060216 mac firefox/1.8 : 20060216 winxp/linux/mac firefox/1.9a1 : 20060216 winxp/linux/mac
Status: RESOLVED → VERIFIED
Keywords: fixed-aviary1.0.8,
fixed1.7.13,
fixed1.8 → verified-aviary1.0.8,
verified1.7.13,
verified1.8
Comment 59•18 years ago
|
||
oops, forgot to mention verified on Mozilla 1.7.13/winxp/20060213 but not other platforms.
Comment 60•18 years ago
|
||
Ok, I don't know what to say except I am seeing failures on linux for ff 1.0.8 for the 20060217 nightly when run in the test framework. If you need help in trying to reproduce this, catch me on irc. Here are the full list of failures for the test run on this test. js1_5/Error/regress-312278.js: EXIT CRASHED 5 (0.580000 seconds) : (browser) : ./peachssh/2006-02-17-12-02-44-firefox-1.0-nightly-win32-1.7.5_20041111XX.log js1_5/Error/regress-312278.js: EXIT CRASHED 5 (0.592000 seconds) : (browser) : ./prunessh/2006-02-17-12-03-29-firefox-1.0-nightly-win32-1.7.5_20041111XX.log js1_5/Error/regress-312278.js: EXIT CRASHED 5 (0.780000 seconds) : (browser) : ./peachssh/2006-02-17-12-50-08-firefox-1.0.7-nightly-win32-1.7.12_20050915XX.log js1_5/Error/regress-312278.js: EXIT CRASHED 5 (0.791000 seconds) : (browser) : ./prunessh/2006-02-17-12-53-12-firefox-1.0.7-nightly-win32-1.7.12_20050915XX.log js1_5/Error/regress-312278.js: EXIT CRASHED signal 11 (1.390734 seconds) : (browser) : ./plumssh/2006-02-17-11-55-04-firefox-1.0-nightly-linux-1.7.5_20041107XX.log js1_5/Error/regress-312278.js: EXIT CRASHED signal 11 (1.416192 seconds) : (browser) : ./plumssh/2006-02-17-14-31-14-firefox-1.0.x-nightly-linux-1.7.13_20060214XX.log js1_5/Error/regress-312278.js: EXIT CRASHED signal 11 (1.435361 seconds) : (browser) : ./plumssh/2006-02-17-13-05-53-firefox-1.0.7-nightly-linux-1.7.12_20050915XX.log js1_5/Error/regress-312278.js: EXIT CRASHED signal 11 (1.437397 seconds) : (browser) : ./pearssh/2006-02-17-13-07-05-firefox-1.0.7-nightly-linux-1.7.12_20050915XX.log js1_5/Error/regress-312278.js: EXIT CRASHED signal 11 (1.445457 seconds) : (browser) : ./pearssh/2006-02-17-14-29-29-firefox-1.0.x-nightly-linux-1.7.13_20060214XX.log js1_5/Error/regress-312278.js: EXIT CRASHED signal 11 (1.446330 seconds) : (browser) : ./pearssh/2006-02-17-11-53-48-firefox-1.0-nightly-linux-1.7.5_20041107XX.log js1_5/Error/regress-312278.js: EXIT CRASHED signal 11 (1.607750 seconds) : (browser) : ./pineapplessh/2006-02-17-12-15-42-firefox-1.0.7-nightly-mac-1.7.12_20050915XX.log js1_5/Error/regress-312278.js: EXIT CRASHED signal 11 (1.639956 seconds) : (browser) : ./papayassh/2006-02-17-12-14-10-firefox-1.0.7-nightly-mac-1.7.12_20050915XX.log
Keywords: verified-aviary1.0.8 → fixed-aviary1.0.8
Comment 61•18 years ago
|
||
(In reply to comment #60) Note the linux builds are from 20060214 instead of 20060217. I filed bug 327704 on the linux builds not being updated.
Comment 62•18 years ago
|
||
v ff on 1.7.13, 1.8.0.1, 1.8, 1.9a1 on windows/linux/mac 20060221 builds.
Keywords: fixed-aviary1.0.8 → verified-aviary1.0.8
Reporter | ||
Comment 63•18 years ago
|
||
This is a replacement for the test case that does not use explicit With constructor. That extension was removed from SpiderMonkey, see bug 336784. As a replacement for With(obj) the new test case uses wrapInsideWith(obj) where the utlity function is defined as: function wrapInsideWith(obj) { var f; with (obj) { f = function() { } } return f.__parent__; } This uses yet another SpiderMonkey extension, namely __parent__ property, but that is not going to be removed yet. I also changed the test to crash js shell that uses uptached js sources.
Attachment #199507 -
Attachment is obsolete: true
Updated•18 years ago
|
Attachment #199766 -
Attachment is obsolete: true
Updated•18 years ago
|
Group: security
Comment 64•18 years ago
|
||
RCS file: /cvsroot/mozilla/js/tests/js1_5/GC/regress-312278.js,v done Checking in regress-312278.js; /cvsroot/mozilla/js/tests/js1_5/GC/regress-312278.js,v <-- regress-312278.js initial revision: 1.1 done
You need to log in
before you can comment on or make changes to this bug.
Description
•