Closed
Bug 563662
Opened 14 years ago
Closed 14 years ago
FPU Exception filter crashes when we hit a pure virtual function call
Categories
(Toolkit :: Crash Reporting, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a5
People
(Reporter: ted, Assigned: ted)
References
()
Details
(Keywords: verified1.9.2, Whiteboard: [3.6.4 ridealong])
Attachments
(2 files)
6.15 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
85.89 KB,
patch
|
christian
:
approval1.9.2.4+
|
Details | Diff | Splinter Review |
I hit a local pure virtual call today which invoked Breakpad, which invoked the floating point exception filter. The app then crashed and produced a Microsoft crash dialog, because the EXCEPTION_POINTERS structure was NULL, and the filter unconditionally dereferences it. Should be trivially patchable, and I think I can even write a test for it.
Assignee | ||
Comment 1•14 years ago
|
||
Two-line fix, a bunch more lines of test changes + test harness changes. I need to test this on a non-Windows platform before landing, too.
Attachment #443379 -
Flags: review?(benjamin)
Updated•14 years ago
|
Attachment #443379 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 2•14 years ago
|
||
Works on Linux, just need a green tree to land on.
Assignee | ||
Comment 3•14 years ago
|
||
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/a8ba788cac18
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Assignee | ||
Comment 4•14 years ago
|
||
Comment on attachment 443379 [details] [diff] [review] Null-check exinfo This patch fixes a regression from the FPU exception filter in which we can crash again while handling certain types of crashes on Windows. The user will get the Microsoft crash dialog in those cases, and not our crash reporter, meaning we lose visibility into those crashes on Socorro. This patch contains a test for the change, as well.
Attachment #443379 -
Flags: approval1.9.2.5?
Assignee | ||
Updated•14 years ago
|
Flags: in-testsuite+
Updated•14 years ago
|
Attachment #443379 -
Flags: approval1.9.2.4?
Comment 5•14 years ago
|
||
Comment on attachment 443379 [details] [diff] [review] Null-check exinfo I'd take this with a 3.6.4 respin also, although there's currently not one planned.
Updated•14 years ago
|
Whiteboard: [rc-ridealong]
Assignee | ||
Comment 6•14 years ago
|
||
Bustage fix, had to disable the test on OS X (apparently we don't catch purecalls in Breakpad there!) http://hg.mozilla.org/mozilla-central/rev/6c5782d2377e
Comment 7•14 years ago
|
||
Can you put a roll-up patch up for approval with the bustage fix included? I agree that if we do a 3.6.4 build4 we'd want this, otherwise we'll take for 3.6.5.
blocking1.9.2: ? → .5+
status1.9.2:
--- → wanted
Updated•14 years ago
|
Whiteboard: [rc-ridealong] → [3.6.4 ridealong]
Assignee | ||
Comment 8•14 years ago
|
||
Attachment #443707 -
Flags: approval1.9.2.5?
Attachment #443707 -
Flags: approval1.9.2.4?
Assignee | ||
Updated•14 years ago
|
Attachment #443379 -
Flags: approval1.9.2.5?
Attachment #443379 -
Flags: approval1.9.2.4?
Looks like we will be doing a build4 for bug 563847, so we'll want this ridealong as well. a=LegNeato for 1.9.2.4. Please land on mozilla-1.9.2 default and GECKO1924_20100413_RELBRANCH asap.
Attachment #443707 -
Flags: approval1.9.2.5?
Attachment #443707 -
Flags: approval1.9.2.4?
Attachment #443707 -
Flags: approval1.9.2.4+
Assignee | ||
Comment 10•14 years ago
|
||
Pushed to 1.9.2: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/e15bd0e4e549 and to the relbranch: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/489e05095126 The relbranch landing doesn't include the tests because there was another patch that needed to be backported to branch for the tests to work, and I didn't want to backport it to the relbranch as well. (got a=LegNeato for that on IRC) I did land the tests on 1.9.2 proper. For verifying this bug (note it is Windows-only): Install "Crash me now Advanced": http://ted.mielczarek.org/mozilla/crashme-advanced.xpi Tools -> Crash Me! -> Pure virtual call! Expected results: Mozilla Crash Reporter (without this patch you would instead get the Windows error dialog)
Assignee | ||
Updated•14 years ago
|
Comment 11•14 years ago
|
||
I'm guessing Ted didn't really mean to backout a set of extension manager tests with this so I restored them with http://hg.mozilla.org/mozilla-central/rev/c69f1d2e14c5
Assignee | ||
Comment 12•14 years ago
|
||
I swear I have no idea how that happened. Sorry!
Comment 13•14 years ago
|
||
(In reply to comment #10) > For verifying this bug (note it is Windows-only): > Install "Crash me now Advanced": > http://ted.mielczarek.org/mozilla/crashme-advanced.xpi > Tools -> Crash Me! -> Pure virtual call! > > Expected results: Mozilla Crash Reporter > (without this patch you would instead get the Windows error dialog) Verified for 1.9.1.4 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.4) Gecko/20100513 Firefox/3.6.4 (.NET CLR 3.5.30729) and compared against 1.9.1.3 with the STR above.
Keywords: verified1.9.2
Updated•14 years ago
|
blocking1.9.2: .5+ → .4+
You need to log in
before you can comment on or make changes to this bug.
Description
•