Last Comment Bug 744727 - firefox crash on mouse move
: firefox crash on mouse move
Status: VERIFIED FIXED
[qa+]
: crash, regression, reproducible
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 11 Branch
: x86_64 Windows 7
: -- critical (vote)
: mozilla15
Assigned To: Bill McCloskey (:billm)
:
Mentors:
http://web.nethium.pl/move_menu/
: 744728 (view as bug list)
Depends on: 731398
Blocks: SadJägerMonkey 641027
  Show dependency treegraph
 
Reported: 2012-04-12 02:48 PDT by bugzilla33
Modified: 2012-06-07 05:27 PDT (History)
12 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
+
verified
+
verified
verified


Attachments
patch for beta (2.63 KB, patch)
2012-04-16 17:19 PDT, Bill McCloskey (:billm)
bhackett1024: review+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Review
fix for m-c (2.23 KB, patch)
2012-04-17 19:05 PDT, Bill McCloskey (:billm)
bhackett1024: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Review

Description bugzilla33 2012-04-12 02:48:21 PDT
User Agent: Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; WOW64; Trident/5.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E)

Steps to reproduce:

http://web.nethium.pl/move_menu/

move fast mouse cursor over menu - left, right, left, right, ect ...


Actual results:

firefox 12 crashes


Expected results:

firefox 11 no crash
Comment 1 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-04-12 03:04:49 PDT
Confirmed on FF12/Win.
It seems specific to Windows, though, as I cannot reproduce on FF12/Mac.
Comment 2 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-04-12 03:18:27 PDT
Companion crash at http://crash-stats.mozilla.com/report/index/bp-7d3eabc6-6f4a-4748-8821-a51702120412
Comment 3 Alice0775 White 2012-04-12 05:35:28 PDT
Regression window(m-c)
Not crash:
http://hg.mozilla.org/mozilla-central/rev/1374294a6119
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111111 Firefox/11.0a1 ID:20111111031514
Crashes:
http://hg.mozilla.org/mozilla-central/rev/50c1bcb49c76
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111111 Firefox/11.0a1 ID:20111111021131
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1374294a6119&tochange=50c1bcb49c76


Regression window(m-i)
Not crash:
http://hg.mozilla.org/integration/mozilla-inbound/rev/0b200d3bd408
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111109 Firefox/11.0a1 ID:20111110084129
Crashes:
http://hg.mozilla.org/integration/mozilla-inbound/rev/33d34da275ed
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111110 Firefox/11.0a1 ID:20111110112929
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0b200d3bd408&tochange=33d34da275ed

Triggered by:
d4bd0f9bece8	Bill McCloskey — Bug 641027 - Add snapshot-at-the-beginning write barriers for incremental GC (r=luke,bhackett)
Comment 4 Alice0775 White 2012-04-12 06:00:13 PDT
Fixed window(m-c)
Crashes
http://hg.mozilla.org/mozilla-central/rev/5c13fce74f83
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120322 Firefox/14.0a1 ID:20120322031220
Not crashes:
http://hg.mozilla.org/mozilla-central/rev/b622d692b8ec
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120322 Firefox/14.0a1 ID:20120322050919
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5c13fce74f83&tochange=b622d692b8ec

Fixed window(m-i)
Crashes
http://hg.mozilla.org/integration/mozilla-inbound/rev/d9491b6074a4
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120321 Firefox/14.0a1 ID:20120321055741
Not crashes:
http://hg.mozilla.org/integration/mozilla-inbound/rev/149eff9b7b92
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120321 Firefox/14.0a1 ID:20120321063842
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d9491b6074a4&tochange=149eff9b7b92

Fixed by:
149eff9b7b92	Brian Hackett — Use singleton types for global object initializers, bug 731398. r=dvander
Comment 5 bugzilla33 2012-04-12 06:26:15 PDT
*** Bug 744728 has been marked as a duplicate of this bug. ***
Comment 6 Brian Hackett (:bhackett) 2012-04-12 06:36:49 PDT
I wouldn't count on the underlying issue having been fixed, as bug 731398 is just an optimization (though maybe one of the IGC fixes that has since gone in fixed the actual issue).
Comment 7 Fernando Hartmann 2012-04-12 09:39:13 PDT
It happens to me too.
Windows 7 32 bits FF12 b4

My crash https://crash-stats.mozilla.com/report/index/bp-c2bf8f50-284a-4bc4-89de-b17fd2120412
Comment 8 Bill McCloskey (:billm) 2012-04-16 17:19:39 PDT
Created attachment 615554 [details] [diff] [review]
patch for beta

I haven't actually figured out what the bug is. However, this patch is fairly small and it makes the crash go away. The difference between crashing and not crashing is whether we do the tempRegForData call after the SetName stub or before.

The patch just moves the tempRegForData after the SetName stub when write barriers are disabled (which will be always in FF12). This is where it was before the write barriers patch landed.
Comment 9 Bill McCloskey (:billm) 2012-04-16 17:40:34 PDT
Comment on attachment 615554 [details] [diff] [review]
patch for beta

[Approval Request Comment]
Regression caused by (bug #): bug 641027
User impact if declined: Methodjit crashes
Testing completed (on m-c, etc.): The patch avoids the crash. Otherwise none.
Risk to taking this patch (and alternatives if risky): The patch reverts to previous behavior before write barriers landed, so the risk is low.
String changes made by this patch: None
Comment 10 Bill McCloskey (:billm) 2012-04-16 20:14:48 PDT
Comment on attachment 615554 [details] [diff] [review]
patch for beta

Landed this for beta. It basically reverts back to pre-write barrier behavior. I got a=akeybl over email. I'll investigate more thoroughly tomorrow.

https://hg.mozilla.org/releases/mozilla-beta/rev/0ba53003ae26
Comment 11 Lukas Blakk [:lsblakk] use ?needinfo 2012-04-17 11:25:26 PDT
Comment on attachment 615554 [details] [diff] [review]
patch for beta

a=akeybl over email, but setting the + here to get it out of our tracking queries.
Comment 12 Bill McCloskey (:billm) 2012-04-17 19:05:48 PDT
Created attachment 615986 [details] [diff] [review]
fix for m-c

Here's the code in question:

RegisterID reg = frame.tempRegForData(lhs);
bool isObject = lhs->isTypeKnown();
MaybeJump notObject;
if (!isObject)
    notObject = frame.testObject(Assembler::NotEqual, lhs);

The call to tempRegForData put the data for lhs in eax. Then testObject called tempRegForType on lhs. It evicted the data of lhs from eax and stored the type there instead. So then the code trying to use reg below was broken. The fix is just to call pinReg(reg) right after tempRegForData(lhs).

It occurs to me that these sort of bugs (which I seem to have introduced several times while adding the write barriers) are similar to GC hazards. It's probably not worth it, but we could track them down in a way similar to how we do GC zeal. That is, every time we call a function that might evict something, we instead have it evict everything possible and fill those registers with garbage.
Comment 13 Alex Keybl [:akeybl] 2012-04-24 10:17:42 PDT
Firefox 13 still appears to be affected by this bug - should we prepare a patch similar to https://bugzilla.mozilla.org/attachment.cgi?id=615554 for that version?
Comment 15 Bill McCloskey (:billm) 2012-04-24 13:42:58 PDT
Comment on attachment 615986 [details] [diff] [review]
fix for m-c

Sorry, I forgot to land this on 14 before it went to aurora. The right thing to do now is to land this patch (not the first one, which was kind of a shot in the dark that happened to kind of work) on aurora and beta.

[Approval Request Comment]
Regression caused by (bug #): bug 641027
User impact if declined: methodjit crashes
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Low, the patch is just some code motion.
String changes made by this patch: None.
Comment 16 :Ehsan Akhgari (out sick) 2012-04-25 07:22:42 PDT
https://hg.mozilla.org/mozilla-central/rev/c81818b53c37
Comment 17 Alex Keybl [:akeybl] 2012-04-30 10:11:52 PDT
Comment on attachment 615986 [details] [diff] [review]
fix for m-c

[Triage Comment]
Approved for Aurora 14 and Beta 13 since this is low risk and early in the cycle.
Comment 19 Paul Silaghi, QA [:pauly] 2012-05-09 05:08:16 PDT
Verified fixed on FF 12 release on Win 7 32/64, Ubuntu 12.04 and Mac OS X 10.6.

Still an issue on FF 13b2: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0
Comment 20 Paul Silaghi, QA [:pauly] 2012-05-09 07:18:57 PDT
Sorry, the crash was on FF 13b1. Everything looks ok on FF 13b2. Verified fixed
Comment 21 Paul Silaghi, QA [:pauly] 2012-06-07 05:27:25 PDT
Verified fixed on FF 14b6:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20100101 Firefox/14.0
Mozilla/5.0 (X11; Linux i686; rv:14.0) Gecko/20100101 Firefox/14.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:14.0) Gecko/20100101 Firefox/14.0

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