Closed Bug 657418 Opened 13 years ago Closed 13 years ago

Firefox Crash [@ nsJSPrincipalsSubsume ]


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox6 + fixed


(Reporter: marcia, Unassigned)



(Keywords: crash, regression)

Crash Data


(1 file)

Seen while reviewing trunk new crashes, but spans 3.6.x and 4.0.x as well. to the crashes across all versions.

Crashes on the trunk started showing using the 2011051300 build. Possible pushlog regression range:

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsJSPrincipalsSubsume 	caps/src/nsJSPrincipals.cpp:73
1 	mozjs.dll 	EvalKernel 	
2 	mozjs.dll 	js::PropertyCache::fill 	js/src/jspropertycache.cpp:154
3 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:4673
4 		@0x0
Line 154 in jspropertycache.cpp there is:

if (!pobj->generic() && shape->hasDefaultGetter() && pobj->containsSlot(shape->slot)) {

which means that frames 0 and 1 are bogus-ish (as in, we jumped into lala-land in fill()).
Hmm, of those report I looked into from there seem to be a number of different stacks that lead to a crash in nsJSPrincipalsSubsume. The 3.6 and 4 ones are very different, but the ones from 6.0a1 for Windows have the top frames from comment #0 mostly (with js::PropertyCache::fill in there), but most from Mac and Linux as well as bp-b2394f9b-40a3-4402-ae5b-832f42110516 have those top frames (Mac and Linux are missing the one being #3 here but all have #0, #1, #2 the same, the #4 here is #3 for the others, I also found cases of #2 "missing"):

0 	xul.dll 	nsJSPrincipalsSubsume 	caps/src/nsJSPrincipals.cpp:73
1 	mozjs.dll 	EvalKernel 	
2 	mozjs.dll 	js::DirectEval 	js/src/jsobj.cpp:1303
3 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:4673
4 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:4580

Here a Mac example from bp-7a0aa18d-6f43-4a13-a531-e4bc72110515

0 	XUL 	nsJSPrincipalsSubsume 	caps/src/nsJSPrincipals.cpp:73
1 	XUL 	EvalKernel 	js/src/jsobj.cpp:990
2 	XUL 	js::DirectEval 	js/src/jsobj.cpp:1303
3 	XUL 	js::Interpret 	js/src/jsinterp.cpp:4580

And a Linux one from bp-e53f9a90-0886-465a-a47d-a183e2110515

0 	nsJSPrincipalsSubsume 	caps/src/nsJSPrincipals.cpp:73
1 	EvalKernel 	js/src/jsobj.cpp:995
2 	js::DirectEval 	js/src/jsobj.cpp:1303
3 	js::Interpret 	js/src/jsinterp.cpp:4580

All are coming from EvalKernel to nsJSPrincipalsSubsume, all are going through the same line in js::Interpret before, but the way there seems different.
See the linked list for more examples.

We had zero crashes with that signature for a week on 6 until this came up on 2011-05-14 with ~30 crashes daily since then. All those seem to be on startup, so due to retries there are a number of those marked as "duplicates".
The first nightly I can consistently reproduce this in is 05-13 from the Tracemonkey branch (5ff15fe83e16). I saw a crash once testing 05-12 nightly, but didn't get a signature and can't reproduce it. As quite a lot of my users are reporting this, I can probably provide a method to reproduce it consistently.
To reproduce this crash with a nighly build simply use the example extension attached to bug 644856 (attachment 521690 [details]).
Crash Signature: [@ nsJSPrincipalsSubsume ]
It is #3 top crasher in 6.0 and happens almost exclusively at startup.
Maybe related to incompatible add-ons.
I don't think it's an issue with incompatible add-ons. In my case, it happens when evalInSandbox is called while a JSM is loading. I suspect it's only becoming more common in 6.0 because more people are using it with a wider variety of add-ons now that it's in beta.
Crashes are at 0x18 on 32-bit and 0x30 64-bit.  In both cases, this is offsetof(nsJPrincipals, nsIPrincipalPtr) so I think this is simply null 'jsprin'.  The only way to call 'subsume' from EvalKernel is through EvalCacheLookup, so hopefully this is just a simple fix.
Attached patch test nullSplinter Review
(Ideally, we'd just do bug 668558, but this is beta, so quickfix/safe for now.)
Attachment #545194 - Flags: review?(mrbkap)
For some reason this signature has risen in FF6 but appeared in FF5. We are going to track this with the assumption we will approve the null check for beta. We can verify the crashes go down.
Attachment #545194 - Flags: review?(mrbkap) → review+
Attachment #545194 - Flags: approval-mozilla-beta?
Attachment #545194 - Flags: approval-mozilla-aurora?
Attachment #545194 - Flags: approval-mozilla-beta?
Attachment #545194 - Flags: approval-mozilla-beta+
Attachment #545194 - Flags: approval-mozilla-aurora?
Attachment #545194 - Flags: approval-mozilla-aurora+
The latest crashes on 7.0a2 (resp. 8.0a1) were with the build ID 20110712042004 (resp. 20110712030758), so it's verified that it's fixed.
Closed: 13 years ago
Resolution: --- → FIXED
I think this caused a massive, presumably shell-only regression on ss-date-format-tofte (visible on the awfy-regress page on TI, as the TM branch doesn't have this cset yet).  The principals are (I think?) always NULL in the shell, so we never use the eval cache when eval'ing the same scripts several hundred times.  Is there a way to modify the tests so that shell performance reflects browser performance better?
Ah, thanks.  File bug 672283.
You need to log in before you can comment on or make changes to this bug.