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.
Created attachment 443379 [details] [diff] [review] Null-check exinfo 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.
Works on Linux, just need a green tree to land on.
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/a8ba788cac18
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.
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.
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
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.
Created attachment 443707 [details] [diff] [review] Rollup patch
Looks like we will be doing a build4 for bug 563847, so we'll want this ridealong as well. a=LegNeato for 220.127.116.11. Please land on mozilla-1.9.2 default and GECKO1924_20100413_RELBRANCH asap.
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)
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
I swear I have no idea how that happened. Sorry!
(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 18.104.22.168 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:22.214.171.124) Gecko/20100513 Firefox/3.6.4 (.NET CLR 3.5.30729) and compared against 126.96.36.199 with the STR above.