977360 - Crash in AtomTableGetHash EXCEPTION_ACCESS_VIOLATION_READ various addresses

Status: RESOLVED INCOMPLETE

(Core :: XPCOM, defect)

Description

[key-mozillabugzilla2939-contact]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20140212131424

Steps to reproduce:

Normal use view/enter data into websites (e.g. Wikipedia). Happened 4 times in less than 24 hours.

I don't know if the problem was introduced in 27.0, but is is _much_ more prevalent in 27.0 and 27.0.1.


Actual results:

Firefox crash.  Crash shows attempt to read address 0x0 in AtomTableGetHash.  Obviously, generates a EXCEPTION_ACCESS_VIOLATION_READ.
Stack trace shows coming from:
3x: mozilla::dom::EventTargetBinding::removeEventListener
1x: nsFormFillController::RemoveKeyListener()


Expected results:

Not a crash. Fail soft. Potentially results from attempt to remove non-existent listener (first guess).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Can you link to the incident reports or attach the stack here?

Comment 2

[key-mozillabugzilla2939-contact]

I have changed the address in the summary to "various" as looking at a query of crash reports filtered for the signature of "AtomTableGetHash" shows a much higher prevalence with a variety of addresses.

A query of crash reports filtered for FF27.0.1 and a signature of "AtomTableGetHash" shows 226 crash reports spread across multiple Windows versions, and one on Linux over the last 2 weeks.
Link to query:
https://crash-stats.mozilla.com/query/?product=Firefox&version=Firefox%3A27.0.1&range_value=2&range_unit=weeks&date=02%2F27%2F2014+19%3A00%3A00&query_search=signature&query_type=contains&query=AtomTableGetHash&reason=&release_channels=&build_id=&process_type=any&hang_type=crash

Link to a crash with this signature that occurred, ironically, while I was writing/researching this comment:
https://crash-stats.mozilla.com/report/index/fa79fb30-c3ef-4526-8650-d6bbd2140227

NOTE: I have not been able to verify the crash report linked above is definitely this bug as the crash reporting system is not displaying _any_ results at this time (overloaded?). However, it does show up in the above query and contains the comment I made when submitting a crash report while researching this comment.

Some of the comments in the filtered crash reports indicate that affected users are experiencing a high rate of crashes (e.g. "Please fix this crash it happens over and over it is never ending").  Obviously, someone could be experiencing crashes from multiple causes.  I know that for me switching to 27.0 then 27.0.1 from 26.0 resulted in a _very_ noticeable increase in crash frequency (enough such that I reverted to 26.0 after only a short time of using 27.0*).  However, only some of them are from this specific cause.

Assuming the access violation was the result of trying to read from an invalid pointer either contained in the hash table, or a pointer to an invalid hash table location: While the optimal solution would be to track down and fix why a pointer was pointing to an invalid address, it looks like the preventive measure of sanity checking that the pointers used are to a valid area of memory prior to accessing the contents would be beneficial in this location (i.e. prevent a significant number of crashes).  [Note: I really should go back and take a look at the source prior to including this paragraph, but the crash reporting system is not functioning well at the moment.  I am writing this paragraph based on my memory of glancing at the code linked while viewing crash reports prior to writing the bug report initially. Thus, this paragraph may be inaccurate.]  Effectively this is a suggestion to include sanity checking the pointers in this function prior to using them as a measure to prevent crashes. Fixing why they are pointing to invalid locations is something that should also be done.  Based on looking at a query report not filtered for only FF27.0.1 it appears that crashing in this function is something that occurs over a wide range of FF versions, and may(?) have multiple root causes (which should all be fixed, but finding and fixing all(?) of them may be difficult). Thus, the suggestion of a sanity check.

I am clearing the needinfo request based on the assumption that the link to the query (with many attached crash reports) and single crash report are sufficient.  I will probably revisit looking at my crash reports at a later time, but at the moment my attempting to get more information is useless due to getting no information out of the crash database in response to requests.  I may provide further links at such time without prompting. However, if more information is actually desired/needed from me please feel free to set the needinfo flag again.

Comment 3

[Benjamin Smedberg]

Reporter, do you have the link from about:crashes to the crash you experienced yourself? Also do you have any Firefox addons (extensions) installed?

Comment 4

[key-mozillabugzilla2939-contact]

As I mentioned in my previous comment, the one link to a crash report which I did provide:
https://crash-stats.mozilla.com/report/index/fa79fb30-c3ef-4526-8650-d6bbd2140227
is mine.  The link from my about:crashes:
https://crash-stats.mozilla.com/report/index/bp-fa79fb30-c3ef-4526-8650-d6bbd2140227

There are several more, but I can not filter the ones from my about:crashes for this bug specifically because, at the moment, _every_ page I attempt to get from crash-stats.mozilla.com which should contain some data either results in an "Internal Server Error", or "Crash Not Found". I am assuming that at least the "Internal Server Error" issues are temporary.  The "Crash Not Found" pages are displayed when I attempt to show crashes in the range of those I looked at initially to file this bug. The "Crash Not Found" results imply that those crashes have been deleted.  I have _never_ intentionally deleted _any_ of my crash reports. I hope that response is related to the other obvious "Internal Server Error" problems as they appeared to start occurring at about the same time. I will continue to attempt to access them, but that data may be gone.  Fortunately, there is at least one, linked above, which is _probably_ this bug.  I do not know for sure that it is this bug because from the point when that crash happened, I have not been able to get _any_ data out of crash-stats.mozilla.com. Thus, I have not been able to view that crash itself.  However, it did show up in the query I linked in my Comment 2.



If you can get data from the query I previously posted (I only get an "Internal Server Error" page for all queries at the moment), then sort for those crashes that have a 0x0 address.  From what I recall, all, or almost all, of the ones that are at address 0x0 are mine.

If you want, I can just post links to all of the crashes listed in this time frame from my about:crashes.  However, the majority in that list I know to not be this bug.

Basically, I am willing to provide more data, but can not obtain any crash information at the moment from crash-stats.mozilla.com due to what appear to be problems on that end. I will make at least one additional attempt again later today. Actually, given how often 27.0.1 crashes for me, I will probably look multiple additional times.

As to extensions: Yes, I have multiple extensions installed.  In an effort to reduce the frequency which 27.0.1 crashes for me, I have reduced the number of extensions enabled to those I routinely use.  Firefox is much more useful with extensions.  In fact, I would not desire to use FF without extensions.  It is certainly possible, even likely, that the initiating "removeEventListener" call is from an extension.  However, from an API design standpoint, it should not be _possible_ for _any_ combination of arguments passed to that function from an extension to cause FF to actually crash. Throw an exception to the extension, sure, but not actually crash FF. I'm not saying that an extension is not making the initiating call to removeEventListener, just that the design should be such that a crash is not possible as a result of a JS call to that API function.  That is also a function that is quite commonly used in JS supplied as a part of page content.  Thus, it is also possible the initiating removeEventListener call was made by JS on a page I had loaded.  However, it is certainly not localized to any one page, or even one website.  As mentioned, the crash in the one link provided above occurred while looking through crash reports to acquire data for Comment 2.

I had looked at all of the crashes I reported over the last couple/few days which were filed prior to filing this bug.  For the subset upon which I based this bug report _all_ had the crashing thread initiated from "removeEventListener" and crashed in AtomTableGetHash with a EXCEPTION_ACCESS_VIOLATION_READ attempting to read address 0x0.

I had multiple other crashes, but this subset (5, or so, crashes) was distinctive in that it had the signature in the crashing thread of: removeEventListener -> (don't remember) -> AtomTableGetHash attempt to read address 0x0 : crash EXCEPTION_ACCESS_VIOLATION_READ.

Comment 5

[key-mozillabugzilla2939-contact]

I have a 6 crashes with this signature in 27.0.1 (including the one previously linked):
https://crash-stats.mozilla.com/report/index/77b9aafc-5785-4ba9-be3d-1bce32140226   removeEventListener
https://crash-stats.mozilla.com/report/index/1de5700f-6569-4439-a9b6-1624e2140226 RemoveKeyListener()
https://crash-stats.mozilla.com/report/index/5cd2fb3c-25b4-44d5-8bdd-ca9b82140226   removeEventListener
https://crash-stats.mozilla.com/report/index/cec8fb57-4f9a-4094-8f5c-87f712140226   removeEventListener
https://crash-stats.mozilla.com/report/index/e52af61c-3d34-470c-93af-f7ff92140227   removeEventListener
https://crash-stats.mozilla.com/report/index/fa79fb30-c3ef-4526-8650-d6bbd2140227   removeEventListener

and 3 crashes with this signature in 27.0 (including the one previously linked):
https://crash-stats.mozilla.com/report/index/0ffc2d10-600a-4a72-b9b6-87e422140214 :Close(nsISHEntry *)
https://crash-stats.mozilla.com/report/index/37ad2fee-ca61-4261-84a4-b4e212140214   removeEventListener
https://crash-stats.mozilla.com/report/index/f1dfa63f-d3f7-4d80-a96f-c933a2140215 addEventListener

Out of a total of 29 reported crashes in 27.0 and 27.0.1 over about 4 days of using those versions. Not surprisingly, I reverted to 26.0 for after only a relatively short time on 27.0.  After waiting a while I have now been trying 27.0.1 n for a bit.

I am clearing the needinfo flag as there are not any requests which I am aware of to which I have not replied.

Comment 6

[Benjamin Smedberg]

cras-stats config was busted yesterday afternoon, should be fixed now.

Here's what I've got so far.

Slightly better stack out of MSVC:

 	xul.dll!AtomTableGetHash(table=..., key=...)  Line 174	C++
 	xul.dll!PL_DHashTableOperate(table=..., key=..., op=PL_DHASH_ADD)  Line 542	C++
 	xul.dll!NS_NewAtom(aUTF16String={...})  Line 626	C++
>	xul.dll!nsEventListenerManager::RemoveEventListenerByType(aListener={...}, aType={...}, aFlags={...})  Line 550	C++
 	xul.dll!mozilla::dom::EventTarget::RemoveEventListener(aType={...}, aListener=0x120f45f0, aUseCapture=false, aRv={...})  Line 21	C++

At http://hg.mozilla.org/releases/mozilla-release/annotate/0414e679f2ab/xpcom/ds/nsAtomTable.cpp#l174 k->mUTF16String is null.

This value comes from http://hg.mozilla.org/releases/mozilla-release/annotate/0414e679f2ab/content/events/src/nsEventListenerManager.cpp#l550 NS_LITERAL_STRING("on") + aType. Which should produce a result which can never be null.

The compiled code for RemoveEventListenerByType is fragmented into hot and cold sections and there is a bunch of inlining so it's very difficult to follow, but from what I've pieced together I don't think the local control flow is miscompiled, and there is a check for the correct string *length* which leads to a NS_ABORT_OOM. However we see that the length is correct (aType is length 7 and the string in AtomTableGetHash is listed as length 9). So possibly we're failing to allocate a buffer and not noticing?

I read through nsTSubstring_CharT::Assign( const substring_tuple_type& tuple ) and I don't see any obvious logical holes in the code there.

Reporter, the primary purpose of asking about addons is merely to create reliable steps to reproduce the crash. If we can isolate one particular addon which, if disabled, causes the crash to go away, then we can investigate that addon code to see how it is using removeEventListener calls and whether it has any binaries. You have roboform active, which has been known to cause crashes in the past, and so I'd be interested to see if disabling roboform is sufficient to make these crashes go away.

Comment 7

[key-mozillabugzilla2939-contact]

Reliably reproducing bugs is, the vast majority of the time, crucial to being able to fix the problem and/or verify that the fix attempted has actually solved the problem.  I apologize if the tone of my prior response was less than cordial.

I have disabled RoboForm and will attempt running for a while with RoboForm disabled.  There are, fortunately, non-automatic methods for using the data contained in RF.  Thus, disabling the extension is just significantly less convenient rather than crippling.  I should reitterate that I have already disabled several extensions which I do not use as often.  Thus, there is no guarantee that disabling RF will demonstrate a definitive cause and effect relationship. In other words, more testing beyond just disabling it and not seeing crashes would be needed to know that there is a definite association between the lack of crashes and the use of RF.

Unfortunately, I do not already have a reliable method of reproducing this crash. No specific activity appeared related to the crashes.  In part, this is could be due to the number of crashes and that I have seen, many of which are not this specific bug.  In other words, there is too much data without it being immediately associated with each crash signature.  Thus, I will merely have to run for a while and see what happens.

Speaking of the above, it would be helpful if there was a way to display the crash signature at the time the crash occurs (i.e. in the crash reporting dialog).  It could, of course, be optional.  Seeing this information at that time would permit better association by the user of crashes to specific signatures.  Humans are good at pattern recognition. If we have the ability to immediately associate signatures with crashes at the time of occurrence this would permit better discrimination between crash signatures and potentially better association between specific crash causes and activity at the time of the crash.  It would be able to change user impressions from "FF is crashing a lot" to FF is crashing a lot due to X and Y, but I've only seen this Z thing one time so I am not going to lump it together with X and Y.

Comment 8

[key-mozillabugzilla2939-contact]

Here is another crash report while RoboForm was disabled:
https://crash-stats.mozilla.com/report/index/9bc98ecb-25d9-496f-937a-c05d32140301

So, not limited to RoboForm.  I have re-enabled that extension.

Comment 9

[key-mozillabugzilla2939-contact]

Additional crash:
https://crash-stats.mozilla.com/report/index/e2185940-438a-4619-a95b-f0fb02140301

Similar, but not exactly the same sequence of calls. Still a crash with EXCEPTION_ACCESS_VIOLATION_READ of address 0x0 in AtomTableGetHash.

This one happened when FF was restarting from a different crash (which had occurred overnight while the computer was unattended). The following is the slightly edited comment I wrote when submitting the crash:
Crashed upon restarting from a overnight crash. Window had come up, session manager had asked which session to restore, and I had selected the most recent session backup. FF had started to restore the session.

A subsequent restart, including loading the same session from session manager, resulted in normal operation.

Comment 10

[key-mozillabugzilla2939-contact]

And another; very similar to the last from a user perspective.  It was again restoring the session from a crash and FF crashed again prior to completely restoring the previous session.

https://crash-stats.mozilla.com/report/index/353a3354-3fc6-48ad-bef3-d625a2140301

A subsequent restart, including loading the same session from session manager, resulted in normal operation.

I am getting frustrated with the _much_ higher frequency of crashes in 27.0.1 than 26.0.  I may choose to yet again revert to 26.0.  If so, I am willing to move back to 27.0.1 for testing, but I'd like to be able to use FF without

Comment 11

[Wayne Mery (:wsmwk)]

Reporter,
Can you still reproduce this crash and bug 980869?

Comment 12

[Phoenix]

Resolved per whiteboard