Last Comment Bug 492675 - crash from MyWebSearch toolbar or WOT extension [@ nsStyleSet::FileRules(int (*)(nsIStyleRuleProcessor*, void*), RuleProcessorData*) ][@ nsStyleSet::FileRules(int (*)(nsIStyleRuleProcessor*, void*), RuleProcessorData*, nsRuleWalker*)]
: crash from MyWebSearch toolbar or WOT extension [@ nsStyleSet::FileRules(int ...
Status: RESOLVED FIXED
[crashkill][crashkill-fix][crashkill-...
: crash, topcrash, user-doc-complete
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 Windows XP
: P2 critical (vote)
: mozilla2.0
Assigned To: David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
:
Mentors:
: 319797 466024 565319 (view as bug list)
Depends on: 522595
Blocks: malware-attacks 503562 518287
  Show dependency treegraph
 
Reported: 2009-05-12 16:59 PDT by chris hofmann
Modified: 2011-12-06 15:11 PST (History)
22 users (show)
dbaron: blocking1.9.2-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.6+
.6-fixed


Attachments
patch 1: fix missing SetLevel call (808 bytes, patch)
2009-10-01 13:15 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
dveditz: approval1.9.1.6+
Details | Diff | Review
patch 2: use nsRuleWalker on the stack (22.45 KB, patch)
2009-10-01 13:16 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Review
patch 2 for 1.9.1 branch (21.94 KB, patch)
2009-10-02 16:03 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Review
patch 2 for 1.9.1 branch (21.93 KB, patch)
2009-10-02 17:40 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
dveditz: approval1.9.1.6+
Details | Diff | Review
patch queue containing relevant parts of bug 522595 and bug 536379 (65.96 KB, text/plain; charset=UTF-8)
2009-12-31 17:40 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
mbeltzner: approval1.9.2.2-
Details

Description chris hofmann 2009-05-12 16:59:47 PDT
#9 topcrash on firefox 3.5 b4.

I can't see where any of the code on the stack has changed in many months or years.

url's appear pretty randomly distributed with about 10% on startpages and then wide distribution of sites.

here is the stack for tracking until we figure out what to try and look at next.

0  	xul.dll  	nsStyleSet::FileRules  	layout/style/nsStyleSet.cpp:588
1 	xul.dll 	nsFrameManager::ReResolveStyleContext 	layout/base/nsFrameManager.cpp:1246
2 	xul.dll 	nsFrameManager::ReResolveStyleContext 	layout/base/nsFrameManager.cpp:1499
3 	xul.dll 	nsFrameManager::ReResolveStyleContext 	layout/base/nsFrameManager.cpp:1499
4 	xul.dll 	nsFrameManager::ComputeStyleChangeFor 	layout/base/nsFrameManager.cpp:1566
5 	xul.dll 	nsCSSFrameConstructor::RestyleElement 	layout/base/nsCSSFrameConstructor.cpp:9992
6 	xul.dll 	nsCSSFrameConstructor::ProcessOneRestyle 	layout/base/nsCSSFrameConstructor.cpp:13372
7 	xul.dll 	nsCSSFrameConstructor::ProcessPendingRestyles 	layout/base/nsCSSFrameConstructor.cpp:13480
8 	xul.dll 	PresShell::DoFlushPendingNotifications 	layout/base/nsPresShell.cpp:4686
9 	xul.dll 	PresShell::FlushPendingNotifications 	layout/base/nsPresShell.cpp:4644
10 	xul.dll 	nsCSSFrameConstructor::RestyleEvent::Run 	layout/base/nsCSSFrameConstructor.cpp:13564
11 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
12 	xul.dll 	nsBaseAppShell::Run 	widget/src/xpwidgets/nsBaseAppShell.cpp:170
13 	xul.dll 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:193
14 	nspr4.dll 	PR_GetEnv 	
15 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:110
16 	firefox.exe 	firefox.exe@0x2197 	
17 	kernel32.dll 	kernel32.dll@0x16fe6
Comment 1 chris hofmann 2009-05-12 17:31:27 PDT
beta 3 reports show it ranks #24

http://crash-stats.mozilla.com/query/query?do_query=1&product=Firefox&version=Firefox%3A3.1b3&date=&range_value=1&range_unit=weeks&query_search=signature&query_type=exact&query=

beta 2 reports show it ranked #38 

but we don't have a good way to tell if the crash slides down the ranking as users drop or if its climbing because of some regression in beta4
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-05-12 17:52:46 PDT
Most crashes seem to have NPMyWebS.dll in the module list; maybe duplicate of bug 466024?

I'd love to debug if somebody has a Windows vm that can be trashed with spyware and then rolled back / deleted.
Comment 3 [:Cww] 2009-07-10 12:00:28 PDT
can we get some eyes on this? QA?
Comment 4 Samuel Sidler (old account; do not CC) 2009-07-10 13:06:19 PDT
(In reply to comment #3)
> can we get some eyes on this? QA?

We can! But if it's caused by malware, it might be hard to reproduce...

Also, we should probably get a list of URLs this crashes on as it might be needed to reproduce, even with the .dll file.
Comment 5 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-08-07 16:13:03 PDT
From bug 466024 it sounds more like somewhat-aggressive toolbars, not total malware.

If somebody has a non-development environment where they can test this stuff I can also make a try server build of something that would crash at a more useful stack... if we can get crash report stacks for try server builds, that is.
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-09-23 11:44:49 PDT
http://dbaron.org/log/20090922-crashes shows:

  nsStyleSet::FileRules(int (*)(nsIStyleRuleProcessor*, void*), RuleProcessorData*) (79 crashes)
     52% (41/79) vs.   3% (243/7100) M3PLUGIN.DLL
     52% (41/79) vs.   4% (249/7100) NPMyWebS.dll
     51% (40/79) vs.   3% (246/7100) MWSBAR.DLL
     48% (38/79) vs.   4% (275/7100) MWSOESTB.DLL
     47% (37/79) vs.   3% (223/7100) F3HTMLMU.DLL
     29% (23/79) vs.   2% (175/7100) F3HKSTUB.DLL
Comment 7 chris hofmann 2009-09-23 15:12:19 PDT
spybuster and other malware removal sites says they find and remove most of the .dlls associated with MyTotalSearchBar that are on the list in comment 6.

http://www.spy-buster.com/spyware-database/MyTotalSearchBar-spyware.htm
http://www.bleepingcomputer.com/startups/M3PLUGIN.DLL-23933.html
Comment 8 Mike Beltzner [:beltzner, not reading bugmail] 2009-09-25 12:37:01 PDT
Need to fix this before we ship Firefox 3.6, either in our code or by blocklisting the toolbars causing it.
Comment 9 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-09-28 17:44:50 PDT
It's possible the patch in bug 513741 might fix this.
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-09-29 15:33:03 PDT
I installed the mywebsearch toolbar to try and test to see if that fixes this, but I haven't gotten it to crash yet.

I wonder if a list of URLs on which this crash was observed would help.
Comment 11 chris hofmann 2009-09-29 15:56:31 PDT
here are a few on youtube

  12 http://www.youtube.com
   2 http://www.youtube.com/
   1 http://www.youtube.com/watch?v=rNCPDLoPbd0&NR=1
   1 http://www.youtube.com/watch?v=egEdS2QGKCE&feature=related
   1 http://www.youtube.com/watch?v=aDTq3I8V0R0&feature=channel
   1 http://www.youtube.com/watch?v=WB1iJwIL6OI
   1 http://www.youtube.com/watch?v=LDYyv-iLmRY&feature=rec-HM-r2
   1 http://www.youtube.com/watch?v=Jdb--Uded5w
   1 http://www.youtube.com/watch?v=JHtwwJ4cKtg
   1 http://www.youtube.com/watch?gl=GB&feature=related&v=6kTTPir17fk
   1 http://www.youtube.com/watch?gl=BR&v=DOZM3L-bkzw
   1 http://www.youtube.com/videos?r=1

other crashes with this signature show facebook profile pages, and facebook apps.

here are the domains of sites
  89 //
  46 http://www.facebook.com
  44 http://www.google.com
  31 about:blank//
  31 \N//
  19 http://apps.facebook.com
  19 about:sessionrestore//
  18 http://mail.live.com
  18 http://login.live.com
  12 http://www.youtube.com
  12 http://www.google.co.uk
  11 https://www.google.com
  10 http://www.alot.com
   8 http://www.orkut.com.br
   8 http://www.microsoft.com
   8 http://www.google.pl
   8 http://www.google.com.br
[and a very long tail follows]

that pretty much maps to general browsing behavior, so I'm not sure we will find any particular site or pate that triggers the crash.

tomcat could builds with and without the patch, then grab a days worth of 900 urls associated with this signature and run them on automation if we think that might be a good approach to testing.
Comment 12 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-09-30 13:04:03 PDT
I've installed the MyWebSearch toolbar, and still couldn't get any of those URLs to crash, or get anything to crash by playing around with the toolbar's UI a bit.

Getting steps to reproduce here would be helpful.
Comment 13 Carsten Book [:Tomcat] 2009-09-30 13:15:03 PDT
(In reply to comment #11)
> 
> that pretty much maps to general browsing behavior, so I'm not sure we will
> find any particular site or pate that triggers the crash.
> 
> tomcat could builds with and without the patch, then grab a days worth of 900
> urls associated with this signature and run them on automation if we think that
> might be a good approach to testing.

will do !
Comment 14 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-01 13:15:03 PDT
Created attachment 404106 [details] [diff] [review]
patch 1: fix missing SetLevel call

This is pretty much unrelated to this bug, except the bug this fixes causes assertions once patch 2 is applied.
Comment 15 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-01 13:16:24 PDT
Created attachment 404107 [details] [diff] [review]
patch 2: use nsRuleWalker on the stack

This applies on top of bug 492675 and on top of patch 1, and I think should fix this bug, although I'm not sure, since I haven't been able to reproduce this bug.
Comment 16 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-01 13:16:53 PDT
er, I meant on top of bug 513741
Comment 17 Jesse Ruderman 2009-10-01 17:51:33 PDT
Does patch 2 at least fix *a* bug?  Hopefully something that we could create an automated test for?
Comment 18 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-01 18:03:51 PDT
It should be an alternate fix for bug 507457, such that we could back that fix out and it would still be fixed, but I haven't tested that yet.
Comment 19 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-01 22:48:06 PDT
(In reply to comment #18)
> It should be an alternate fix for bug 507457, such that we could back that fix
> out and it would still be fixed, but I haven't tested that yet.

It does, in fact, fix bug 507457 with that bug's patch removed.
Comment 20 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-02 14:08:32 PDT
http://hg.mozilla.org/mozilla-central/rev/3f5ac794879f
http://hg.mozilla.org/mozilla-central/rev/b080434f9dd4
Comment 21 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-02 15:53:40 PDT
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/4a4504e6581d
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/b5ff73121753

The latter patch had the warning fix that I threw in dropped (since it doesn't apply there), and was no longer on top of bug 513741 (by merging the two patches, since patch 2 here removed what was added there).
Comment 22 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-02 16:03:25 PDT
Created attachment 404355 [details] [diff] [review]
patch 2 for 1.9.1 branch

The patch as landed on 1.9.2, plus a little simple merging to deal with the differences from http://hg.mozilla.org/mozilla-central/rev/929ccf186103
Comment 23 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-02 17:40:45 PDT
Created attachment 404372 [details] [diff] [review]
patch 2 for 1.9.1 branch

Oops, made a simple mistake in the merging.
Comment 25 Daniel Veditz [:dveditz] 2009-10-05 14:46:11 PDT
This will not make 1.9.1.4. Do we need both patches for 1.9.1.5 or does the second include the first?
Comment 26 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-05 17:38:54 PDT
Both patches are needed.
Comment 28 Daniel Veditz [:dveditz] 2009-10-16 11:01:36 PDT
Comment on attachment 404106 [details] [diff] [review]
patch 1: fix missing SetLevel call

Approved for 1.9.1.5, a=dveditz for release-drivers
Comment 29 Daniel Veditz [:dveditz] 2009-10-16 11:03:12 PDT
Comment on attachment 404372 [details] [diff] [review]
patch 2 for 1.9.1 branch

Approved for 1.9.1.5, a=dveditz for release-drivers
Comment 30 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-16 15:53:00 PDT
Landed on 1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6d3cc605d44e
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/709168428034
Comment 31 Chris Ilias [:cilias] 2009-10-29 00:39:15 PDT
user-doc-complete
<https://support.mozilla.com/en-US/kb/Crash+signature+-+nsStyleSet+FileRules?bl=n>
Comment 32 Mike Shaver (:shaver -- probably not reading bugmail closely) 2009-10-30 09:40:06 PDT
This looks to be FIXED on trunk and 1.9.2, marking.
Comment 33 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-30 09:50:26 PDT
We have no idea whether it's fixed and won't know until we ship 3.5.5.
Comment 34 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-30 09:53:36 PDT
Then again, it's showing up as topcrash #59 for 3.6b1; I'm not sure if that means part of it is fixed and part isn't, or that none of it is fixed.
Comment 35 Damon Sicore (:damons) 2009-11-02 16:13:02 PST
Can we get a percentage of crashes that we think this will fix?
Comment 36 chris hofmann 2009-11-02 16:14:54 PST
re: comment 35

this will fix about 1000 crashes per day incoming on various releases.

distribution of all versions where the nsStyleSet::FileRules crash was found on 20091101-crashdata.csv
 834 Firefox 3.5.4
 193 Firefox 3.5.3
  23 Firefox 3.5.2
  14 Firefox 3.5
   5 Firefox 3.5.1
   4 Firefox 3.0.15
   2 Firefox 3.6b1
   2 Firefox 3.1b3
   1 Firefox 3.5b99
   1 Firefox 3.5b4
   1 Firefox 3.1b1
   1 Firefox 3.0.3
Comment 37 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-04 13:33:20 PST
(In reply to comment #34)
> Then again, it's showing up as topcrash #59 for 3.6b1; I'm not sure if that
> means part of it is fixed and part isn't, or that none of it is fixed.

Though there have been very few crashes since we actually shipped 3.6b1, though.  There were more on the early builds than on the build we actually shipped as beta:

http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.5.4&query_search=signature&query_type=exact&query=&date=&range_value=1&range_unit=weeks&do_query=1&signature=nsStyleSet%3A%3AFileRules%28int%20%28*%29%28nsIStyleRuleProcessor*%2C%20void*%29%2C%20RuleProcessorData*%29
Comment 39 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-07 23:40:51 PST
Notes from debugging the minidump:
http://crash-stats.mozilla.com/report/index/e964d5e5-efc3-4a37-b99e-c2a0e2091107

It looks like the disassembly is really for the guts of AddImportantRules... actually GetImportantRule inlined within AddImportantRules (with a check for whether the vtable entry matches the expected CSSStyleRuleImpl::GetImportantRule).

The 0x1c offset is the mImportantData of a null nsCSSDeclaration.  The CSSStyleRuleImpl is 0x53480e0, which seems pretty likely to be a good pointer given that it just passed a test against one of its vtable entries.
Comment 40 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-07 23:42:28 PST
(In reply to comment #39)
> The 0x1c offset is the mImportantData of a null nsCSSDeclaration.  The
> CSSStyleRuleImpl is 0x53480e0, which seems pretty likely to be a good pointer
> given that it just passed a test against one of its vtable entries.

To be clear, mDeclaration->mImportantData is being loaded as part of the inlined HasImportantData call in CSSStyleRuleImpl::GetImportantRule; the fast-path is for when it's null.  (However, the crash is because mDeclaration is null.)
Comment 42 chris hofmann 2009-11-08 08:20:23 PST
(In reply to comment #37)
> (In reply to comment #34)
> > Then again, it's showing up as topcrash #59 for 3.6b1; I'm not sure if that
> > means part of it is fixed and part isn't, or that none of it is fixed.
> 
> Though there have been very few crashes since we actually shipped 3.6b1,
> though.  There were more on the early builds than on the build we actually
> shipped as beta:
> 

We might have hit some threshold of 

-general 3.6b1 usage, 
-or some pocket of users joining the beta, 
-or some new outbreak of the externally driven crash problem 

that changed the beta volume on this shortly after you made  comment 37.   we have gone from 1-3 3.6b1 crash per day to 12-31 crashes per day the last few days.

3.6b1
active
daily
users                         #crash  #tl-sigs    #nsStyleSet::FileRules crashes

89,724	20091101-crashdata.csv	 1,468	  500     2
123,763	20091102-crashdata.csv	 1,939	  601     3
158,496	20091103-crashdata.csv	 2,177	  645     3
180,255	20091104-crashdata.csv	 2,321	  729     1
195,912	20091105-crashdata.csv	 5,962	1,364    12
213,475	20091106-crashdata.csv	14,917	2,537    24
198,588	20091107-crashdata.csv 	15,187	2,685    31
Comment 43 chris hofmann 2009-11-08 08:25:53 PST
there is some wrapping on that last comment.  
left column is adu's on the beta.  
right column is  #nsStyleSet::FileRules crashes
Comment 44 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-11-08 18:54:05 PST
Should we keep blocking 1.9.2 on this? It seems like we don't have a good idea of what could be causing the remaining crashes.
Comment 46 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-09 08:46:24 PST
The extension correlations for this crash that look likely to be statistically significant to me, from last night's run at http://people.mozilla.com/crash_analysis/ are:

Firefox 3.6b1:

  nsStyleSet::FileRules(int (*)(nsIStyleRuleProcessor*, void*), RuleProcessorData*, nsRuleWalker*)|EXCEPTION_ACCESS_VIOLATION (34 crashes)
     26% (9/34) vs.   3% (331/13234) {a0d7ccb3-214d-498b-b4aa-0e8fda9a7bf7} (WOT, https://addons.mozilla.org/addon/3456)
     29% (10/34) vs.  10% (1310/13234) {ABDE892B-13A8-4d1b-88E6-365A6E755758}

Firefox 3.5.5:

  nsStyleSet::FileRules(int (*)(nsIStyleRuleProcessor*, void*), RuleProcessorData*)|EXCEPTION_ACCESS_VIOLATION (808 crashes)
     18% (148/808) vs.   2% (1592/79833) {a0d7ccb3-214d-498b-b4aa-0e8fda9a7bf7} (WOT, https://addons.mozilla.org/addon/3456)
     23% (186/808) vs.  13% (10240/79833) {635abd67-4fe9-1b23-4f01-e679fa7484c1} (Yahoo! Toolbar, https://addons.mozilla.org/addon/2032)
     14% (110/808) vs.   5% (4064/79833) avg@igeared
     17% (140/808) vs.   9% (7109/79833) {3f963a5b-e555-4543-90e2-c3908898db71}
     14% (110/808) vs.   6% (4947/79833) {E2883E8F-472F-4fb0-9522-AC9BF37916A7} (Adobe Flash Extension, https://addons.mozilla.org/addon/13845)
      9% (73/808) vs.   2% (1534/79833) browserhighlighter@ebay.com (The Browser Highlighter, https://addons.mozilla.org/addon/11808)
     16% (132/808) vs.  10% (8006/79833) {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d} (Adblock Plus, https://addons.mozilla.org/addon/1865)
     13% (101/808) vs.   7% (5375/79833) {B13721C7-F507-4982-B2E5-502A71474FED} (Skype Toolbar for Firefox, https://addons.mozilla.org/addon/4938)


https://addons.mozilla.org/addon/3456 is probably the most clearly related.
Comment 47 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-09 09:08:55 PST
And the URL list for this crash looks pretty much like a "most visited pages on the Web" list.
Comment 48 chris hofmann 2009-11-09 09:37:15 PST
anyone know a contact for the web of trust folks?
Comment 49 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-09 09:56:42 PST
How would a contact help?
Comment 50 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-09 11:15:01 PST
The WOT extension also seems to be causing crashes in:
  RuleHash_ClassTable_GetKey
  ContentEnumFunc
  RuleHash::EnumerateAllRules(int, nsIAtom*, nsIAtom*, nsAttrValue const*, void (*)(nsICSSStyleRule*, nsCSSSelector*, void*), void*)

Those might be related.
Comment 51 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-09 11:20:30 PST
(In reply to comment #46)
> https://addons.mozilla.org/addon/3456 is probably the most clearly related.

That said, the MyWebSearch toolbar is responsible for a larger portion of the crashes (probably 2/3 on 3.6b1 and 1/2 on 3.5.5), but it only shows up in the module correlations and not the addons list.

NPMyWebS.dll also shows up in the correlations (I'm looking at 3.5.5) for the three crashes above (though not quite at those percentages).
Comment 52 Damon Sicore (:damons) 2009-11-09 12:53:16 PST
(In reply to comment #49)
> How would a contact help?

David, are you saying that this crash cannot be fixed by the addons mentioned here?
Comment 53 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-09 13:14:35 PST
Looking at 6 3.5.5 RuleHash_ClassTable_GetKey crashes:

The disassembly is really simple:
  RuleHashTableEntry::mRules is at offset 0x4
  RuleValue::mSelector is at offset 0x4
  nsCSSSelector::mClassList is at offset 0xc
  nsAtomList::mAtom is at offset 0x0

http://crash-stats.mozilla.com/report/index/7d8624df-a602-438e-be00-5c43f2091108

The crash is trying to dereference ->mAtom (the dereference at offset 0x0), so
mClassList is a bad pointer (0x0f100c11).

http://crash-stats.mozilla.com/report/index/509b0d0c-da2e-4162-a9fd-3458d2091108

The crash is trying to dereference ->mAtom (the dereference at offset 0x0), so
mClassList is a bad pointer (0x006c0061).  (That's "al" in UTF-16.)

http://crash-stats.mozilla.com/report/index/643fecb1-b5de-4245-b880-12d262091108

The crash is trying to dereference ->mAtom (the dereference at offset 0x0);
this time mClassList is null.  And in the FileRules frame we're doing
eDocSheet rule processors.

http://crash-stats.mozilla.com/report/index/a8b5d21f-2812-467e-ac71-10a802091108

Exactly like the previous one.

http://crash-stats.mozilla.com/report/index/c5c5a7ce-3f28-48d5-8371-78adb2091108

Exactly like the previous two.

http://crash-stats.mozilla.com/report/index/cc18f999-95f3-4af0-9f2b-ca5fd2091108

The crash is trying to dereference ->mAtom (the dereference at offset 0x0), so
mClassList is a bad pointer (0x70697263).  (That's "crip" in UTF-8/ASCII.)



Now, I'll look at six 3.5.5 ContentEnumFunc crashes:

http://crash-stats.mozilla.com/report/index/40a9f15b-4bef-480e-81db-781502091108

Claims to be crashing on the line data->mRuleWalker->Forward(aRule).
Presumably that's because nsRuleWalker::Forward and nsRuleNode::Transition
are inlined.  In particular, it's in a chunk of code representing this bit
at the end of nsRuleNode::Transition:

    // Create the new entry in our list.
    next = new (mPresContext)
      nsRuleNode(mPresContext, this, aRule, aLevel, aIsImportantRule);
    if (!next) {
      return nsnull;
    }
    next->mNextSibling = ChildrenList();
    SetChildrenList(next);

With nsRuleNode::nsRuleNode inlined into this.

And, in particular, the crash is on the line (EIP 0x10074134):
  NS_IF_ADDREF(aRule)
in nsRuleNode::nsRuleNode.  aRule (ECX) is 0x0200fc80.  aRule's vtable
(EDX), however, is null.  And, for the record, |this| in
nsRuleNode::nsRuleNode (ESI) is 0x022da37c.  And mDependentBits was
just set from (EDI) to 0x40000000 (i.e., eDocSheet level, not important).
And mParent was set to EAX, 0x024bce5c.

http://crash-stats.mozilla.com/report/index/72acd167-2dc5-4a36-b8b5-918352091108

In this case, it's SelectorMatchesTree inlined into ContentEnumFunc,
and it's the variable |content| (EAX, 0x64b7e80) that has a bad vtable
(EDX, 0x400).  Probably not related to the crash in this bug.

http://crash-stats.mozilla.com/report/index/88bb6665-0eba-4115-a791-071cf2091108

Looks like a bad rule node in the list near the start of
nsRuleNode::Transition, in this loop:
    while (curr && curr->GetKey() != key) {
      curr = curr->mNextSibling;
      ++numKids;
    }
Again, not related.

http://crash-stats.mozilla.com/report/index/a1da412a-f82c-4954-8fa3-852512091108

Like the one before the previous:  a bad vtable on |content|, I think.
(0x00001000).

http://crash-stats.mozilla.com/report/index/ad97299e-9b71-4be5-9332-6c5da2091108

At a quick glance, this looks like a bad rule node inside the inlined
nsRuleNode::Transition (i.e., nsRuleWalker::mCurrentNode ended up bad).

http://crash-stats.mozilla.com/report/index/cfa3e704-5245-4d47-9c15-e300d2091108

This one looks like the first of the six (the only other one related
to this bug).  Like that, the crash is on the line (EIP 0x10074134):
  NS_IF_ADDREF(aRule)
in nsRuleNode::nsRuleNode.  aRule (ECX) is 0x02f8ace0.  aRule's vtable
(EDX), however, is null.  And, for the record, |this| in
nsRuleNode::nsRuleNode (ESI) is 0x05b1edd0.  And mDependentBits was
just set from (EDI) to 0x40000000 (i.e., eDocSheet level, not important).
And mParent was set to EAX, 0x0547b350.




(In reply to comment #52)
> (In reply to comment #49)
> > How would a contact help?
> 
> David, are you saying that this crash cannot be fixed by the addons mentioned
> here?

It's probably related to something that those addons are doing, but we don't know what.

So, if we had a contact, what would we tell them?  I don't currently have anything to say, but if somebody else does, then it could be useful.
Comment 55 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-09 15:57:28 PST
(In reply to comment #49)
> How would a contact help?

Two things we could ask:

 (1) Do they have steps to reproduce the crash?

 (2) Do they have any ideas of what their code might be doing related to CSS rules that might be causing the crash?
Comment 56 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-11-09 16:34:22 PST
Can we ask for a look at (parts of) their source code?
Comment 57 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-09 16:50:12 PST
I just took a look at WOT:  it's GPL, has no binary components, and does some stuff with the CSS object model (pretty trivial, except they're holding their own cache of style rules).
Comment 58 Boris Zbarsky [:bz] 2009-11-09 18:32:07 PST
So the reason I asked about extensions is because I wanted to see whether any of them were using the inspector APIs for messing with style rules.  WOT doesn't seem to be doing that...  Their css.js code should be pretty safe.  They hold DOM rules, but if the underlying style rule dies the DOM rule should just end up with a null mRule and handle that safely; in such cases the nsIStyleRule shouldn't be reachable via rulenodes anyway...
Comment 61 Henrik Skupin (:whimboo) 2009-11-10 08:45:36 PST
So what I can see after running both of those extensions a bit is that WOT overlays the content area with a warning all the time you search for something inside the mywebsearch toolbar or click a button. All those referenced web pages have a poor reputation. Could that be related?
Comment 62 Henrik Skupin (:whimboo) 2009-11-15 15:03:57 PST
Boris or David, was that information helpful for you?
Comment 63 Boris Zbarsky [:bz] 2009-11-15 17:07:08 PST
I don't know.  Nothing in the comment 59 file jumped out at me...  :(
Comment 64 Damon Sicore (:damons) 2009-11-23 16:05:00 PST
Not blocking.  We can't repro.
Comment 65 chris hofmann 2009-11-23 20:20:47 PST
The RuleProcessor singature seems spread across 3.0* 3.5* 3.6* .  The RuleWalker signature seems exclusive to 3.6*

chofmann$ grep 'nsStyleSet::FileRules' 20091122*  | grep RuleProcessorData | awk -F\t '{print $8}' | sort | uniq -c | sort -nr
 915 3.5.5
  52 3.5.3
  43 3.6b3
  19 3.5.4
  16 3.5.2
  12 3.5
  11 3.6b1
   9 3.1b3
   8 3.5.1
   5 3.5b4
   4 3.6b2
   3 3.0.15
   2 3.1b2
   1 3.7a1pre
   1 3.5b99
   1 3.0.4

chofmann$ grep 'nsStyleSet::FileRules' 20091122*  | grep RuleWalker| awk -F\t '{print $8}' | sort | uniq -c | sort -nr
  43 3.6b3
  11 3.6b1
   4 3.6b2
   1 3.7a1pre
Comment 66 chris hofmann 2009-11-23 22:11:50 PST
so if I read things right the RuleWalker crashes are only 3.6x 

 and

the RuleWalker crashes are the ones strongly correlated to MyWebSearch toolbar

     58% (22/38) vs.   2% (244/11946) NPMyWebS.dll
     58% (22/38) vs.   2% (244/11946) M3PLUGIN.DLL
     58% (22/38) vs.   2% (236/11946) MWSBAR.DLL
     55% (21/38) vs.   3% (375/11946) MWSOESTB.DLL
     55% (21/38) vs.   2% (229/11946) F3HTMLMU.DLL
     42% (16/38) vs.   2% (280/11946) F3HKSTUB.DLL

Then blocking these might be an option to reduce the 3.6x side of these crashes.

Sounds like spybuster and maybe other anti-virus packages might be disabling or removing.
http://www.spy-buster.com/spyware-database/MyTotalSearchBar-spyware.htm

http://www.dslreports.com/forum/remark,15313228 says NPMyWebS.dll gets put in plugins

Program Files\Mozilla Firefox\plugins\NPMyWebS.dll

Some more research is probably needed to figure out the exact blocking strategy and test and confirm it.  maybe that could get sorted out by an addition to the blocklist and another 36beta.

Sounds like Henrik as an installation of the toolbar set up to test a block entry.

renom to see if we want to try the MyWebSearch toolbar block to help 3.6 to reduce at least a few of these crashes; although this wouldn't address other  bugs that might be getting tickled by addons/plugins poking at CSS rule code.
Comment 67 chris hofmann 2009-11-23 22:13:08 PST
or maybe we should just spin of another bug for the suggestion in comment 66
Comment 68 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-23 22:25:07 PST
Yes, please file a separate bug for extension blocks.
Comment 69 Jesse Ruderman 2009-11-23 22:27:31 PST
We'd rather not block these extensions, since we've looked at their code and determined they're not doing anything wrong.  We could suggest alternative ways for these extensions to work (as a workaround), but blocking wouldn't be nice.
Comment 70 Mike Beltzner [:beltzner, not reading bugmail] 2009-11-24 08:28:35 PST
Blocking based on the fact that this was fixed for 1.9.1.6 - can we get it ported to 1.9.2 and central?
Comment 71 Samuel Sidler (old account; do not CC) 2009-11-24 11:26:20 PST
This landed on m-c and 1.9.2 (comment 20 and comment 21).

See also comment #34:
> Then again, it's showing up as topcrash #59 for 3.6b1; I'm not sure if that
> means part of it is fixed and part isn't, or that none of it is fixed.
Comment 72 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-11-24 11:47:24 PST
The patch in question landed on all relevant branches, but didn't fix the bug.

I think the patches in bug 522595 might either fix this or move the crash to a different location that might tell us more information.

But I don't think this should block.
Comment 73 chris hofmann 2009-12-08 10:48:41 PST
the signature 

nsStyleSet::FileRules(int (*)(nsIStyleRuleProcessor*, void*), RuleProcessorData*, nsRuleWalker*) 

is showing up as new/morfed inearly beta testing of firefox 3.5.6

http://people.mozilla.com/~jst/new-crashes/Firefox/latest/3.5.6-3.5.html
Comment 74 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-12-11 11:54:18 PST
(In reply to comment #72)
> I think the patches in bug 522595 might either fix this or move the crash to a
> different location that might tell us more information.

These patches landed on mozilla-central today.
Comment 75 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-12-22 07:10:06 PST
There haven't been any nsStyleSet::FileRules crashes from mozilla-central builds since those patches landed, or, for that matter, any crashes in any nsStyleSet methods.

The histogram prior to landing was:

0  Dec 2
2  Dec 3
0  Dec 4
2  Dec 5
0  Dec 6
1  Dec 7
1  Dec 8    (though that crash was recorded a week after the build date)
0  Dec 9
0  Dec 10
Comment 76 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-12-28 08:34:11 PST
Once bug 536379 has baked on trunk, I think we should consider taking bug 522595 and bug 536379 for a 1.9.2.* release.
Comment 77 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-12-31 13:04:11 PST
I have a merge of both of those bugs to 1.9.2 in my 1.9.2 patch queue at http://hg.mozilla.org/users/dbaron_mozilla.com/patches-1.9.2/

(I'm currently waiting on try server results from that merge.)
Comment 78 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-12-31 17:40:26 PST
Created attachment 419722 [details]
patch queue containing relevant parts of bug 522595 and bug 536379

This is the output of "hg outgoing -p" with the patch queue for what I think we'd need to land to fix this.  It passes tests on try (modulo random orange in make check on Windows).
Comment 79 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-01-08 10:10:10 PST
*** Bug 466024 has been marked as a duplicate of this bug. ***
Comment 80 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2010-01-08 10:24:23 PST
*** Bug 319797 has been marked as a duplicate of this bug. ***
Comment 81 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-10 13:24:23 PST
Comment on attachment 419722 [details]
patch queue containing relevant parts of bug 522595 and bug 536379

This is in the top 100 crashes, but not the top 20. Do we still fell it's worth the risk?

The reason I ask is that this is a pretty big patch, and that usually causes driver concern.
Comment 82 timeless 2010-05-12 21:54:48 PDT
*** Bug 565319 has been marked as a duplicate of this bug. ***
Comment 83 Sheila Mooney 2011-12-06 14:59:45 PST
All of these appear to be on 3.0, 3.5 and 3.6. Resolving as works for me.

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