Last Comment Bug 710516 - Printing JS stacks hits fatal asserts
: Printing JS stacks hits fatal asserts
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Mac OS X
: -- critical (vote)
: mozilla12
Assigned To: Brian Hackett (:bhackett)
:
Mentors:
Depends on:
Blocks: 684505
  Show dependency treegraph
 
Reported: 2011-12-13 19:49 PST by Boris Zbarsky [:bz]
Modified: 2012-03-01 13:48 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
wontfix


Attachments
rm JSObject::containsSlot (12.01 KB, patch)
2011-12-14 07:36 PST, Brian Hackett (:bhackett)
luke: review+
Details | Diff | Splinter Review
Rebased patch (6.68 KB, patch)
2011-12-20 12:06 PST, Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
no flags Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2011-12-13 19:49:01 PST
Stack looks like this:

#0  0x011b7698 in CrashInJS () at ../../../mozilla/js/src/jsutil.cpp:92
#1  0x011b7701 in JS_Assert (s=0x1380d58 "hasSlot() && !hasMissingSlot()", file=0x1380c5c "../../../mozilla/js/src/jsscope.h", ln=762) at ../../../mozilla/js/src/jsutil.cpp:103
#2  0x010d0642 in js::Shape::slot (this=0x224ad430) at jsscope.h:762
#3  0x01077fd5 in JS_GetPropertyDesc (cx=0x1db010, obj=0x224b0940, sprop=0x224ad430, pd=0x64cd590) at ../../../mozilla/js/src/jsdbgapi.cpp:864

Line 864 of jsdbgapi.cpp is:

    if (obj->containsSlot(shape->slot())) {

and in this case |shape->hasSlot()| is false.

It looks like there shoud be an explicit hasSlot check here, perhaps?  Is the containsSlot() check still needed, then?

This is preventing use of DumpJSStack, which is a serious problem for browser debugging....
Comment 1 Boris Zbarsky [:bz] 2011-12-13 19:51:07 PST
(Note that there seem to be other uses of containsSlot(shape->slot()) that aren't guarded by hasSlot(); I'm not sure why they're safe.)
Comment 2 Brian Hackett (:bhackett) 2011-12-14 07:36:45 PST
Created attachment 581635 [details] [diff] [review]
rm JSObject::containsSlot

Remove obj->containsSlot() entirely.  There isn't any situation where it is correct to use containsSlot instead of shape->hasSlot().  This also fixes some broken logic in js_PrintObjectSlotName.
Comment 3 Boris Zbarsky [:bz] 2011-12-14 10:16:06 PST
Comment on attachment 581635 [details] [diff] [review]
rm JSObject::containsSlot

Is the jsdbgapi code not needed anymore?
Comment 4 Brian Hackett (:bhackett) 2011-12-15 08:08:06 PST
The jsdbgapi code is looking for a second shape which has the same slot as the first shape.  It used to be possible that two properties on an object could alias each other in this way, but this is no longer the case.

https://hg.mozilla.org/integration/mozilla-inbound/rev/36fa9b176a91
Comment 5 Brian Hackett (:bhackett) 2011-12-15 09:07:48 PST
Backed out due to M1 orange:

https://hg.mozilla.org/integration/mozilla-inbound/rev/c5ebfdbe12d9
Comment 6 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-12-20 12:06:01 PST
Created attachment 583244 [details] [diff] [review]
Rebased patch

I rebased the patch to be able to print js stacks while debugging something else.
Comment 7 Brian Hackett (:bhackett) 2011-12-21 06:34:12 PST
Relanding.  Some method barrier code gets invoked during property deletion, and this was happening after calling a hook which could remove the shape from the object.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c7cd0b499ba
Comment 8 Ed Morley [:emorley] 2011-12-21 12:53:47 PST
https://hg.mozilla.org/mozilla-central/rev/2c7cd0b499ba
Comment 9 Alex Keybl [:akeybl] 2012-01-09 11:41:31 PST
](In reply to Brian Hackett (:bhackett) from comment #4)
> The jsdbgapi code is looking for a second shape which has the same slot as
> the first shape.  It used to be possible that two properties on an object
> could alias each other in this way, but this is no longer the case.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/36fa9b176a91

If this will significantly affect dev debugging for 11, please nominate for approval on Aurora 11 as soon as possible.

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