Closed
Bug 744727
Opened 13 years ago
Closed 13 years ago
firefox crash on mouse move
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla15
People
(Reporter: bugzilla33, Assigned: billm)
References
()
Details
(Keywords: crash, regression, reproducible, Whiteboard: [qa+])
Crash Data
Attachments
(2 files)
(deleted),
patch
|
bhackett1024
:
review+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bhackett1024
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Updated•13 years ago
|
Severity: normal → critical
Component: Untriaged → General
Reporter | ||
Updated•13 years ago
|
Comment 1•13 years ago
|
||
Confirmed on FF12/Win.
It seems specific to Windows, though, as I cannot reproduce on FF12/Mac.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•13 years ago
|
||
Comment 3•13 years ago
|
||
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•13 years ago
|
||
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 6•13 years ago
|
||
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).
Updated•13 years ago
|
Blocks: SadJägerMonkey
Crash Signature: [@ js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool)]
status-firefox11:
--- → affected
status-firefox12:
--- → affected
status-firefox13:
--- → affected
status-firefox14:
--- → fixed
Version: 12 Branch → 11 Branch
Comment 7•13 years ago
|
||
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
Updated•13 years ago
|
tracking-firefox12:
--- → ?
tracking-firefox13:
--- → ?
Updated•13 years ago
|
Assignee | ||
Comment 8•13 years ago
|
||
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.
Attachment #615554 -
Flags: review?(bhackett1024)
Updated•13 years ago
|
Attachment #615554 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 9•13 years ago
|
||
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
Attachment #615554 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 10•13 years ago
|
||
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
Updated•13 years ago
|
Crash Signature: [@ js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool)] → [@ js::mjit::EnterMethodJIT(JSContext*, js::StackFrame*, void*, JS::Value*, bool)]
[@ js::Shape::search(JSContext*, js::Shape*, int, js::Shape***, bool)]
Comment 11•13 years ago
|
||
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.
Attachment #615554 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•13 years ago
|
Assignee: general → wmccloskey
Assignee | ||
Comment 12•13 years ago
|
||
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.
Attachment #615986 -
Flags: review?(bhackett1024)
Updated•13 years ago
|
Attachment #615986 -
Flags: review?(bhackett1024) → review+
Comment 13•13 years ago
|
||
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?
Assignee | ||
Comment 14•13 years ago
|
||
Target Milestone: --- → mozilla15
Assignee | ||
Comment 15•13 years ago
|
||
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.
Attachment #615986 -
Flags: approval-mozilla-beta?
Attachment #615986 -
Flags: approval-mozilla-aurora?
Comment 16•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 17•13 years ago
|
||
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.
Attachment #615986 -
Flags: approval-mozilla-beta?
Attachment #615986 -
Flags: approval-mozilla-beta+
Attachment #615986 -
Flags: approval-mozilla-aurora?
Attachment #615986 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 18•13 years ago
|
||
Comment 19•13 years ago
|
||
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•13 years ago
|
||
Sorry, the crash was on FF 13b1. Everything looks ok on FF 13b2. Verified fixed
Comment 21•12 years ago
|
||
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
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•