Closed
Bug 577486
Opened 14 years ago
Closed 14 years ago
Check-in for Breakpad 64-bit on OS X dropped Windows 2000 support
Categories
(Toolkit :: Crash Reporting, defect)
Tracking
()
VERIFIED
FIXED
mozilla2.0b3
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: aja+bugzilla, Assigned: tnikkel)
References
Details
(Keywords: regression, Whiteboard: [pm])
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
ted
:
review-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
https://bugzilla.mozilla.org/show_bug.cgi?id=567424#c14
John P Baker 2010-07-08 06:05:50 PDT
This seems to have broken on Windows 2000 (yes I know!) with
RtlCaptureContext could not be loaded in dynamic link library KERNEL32.dll
on attempted start.
Reporter | ||
Updated•14 years ago
|
Assignee | ||
Comment 1•14 years ago
|
||
This causes major problems for someone like me who needs to dogfood his major changes before landing them for example.
Comment 2•14 years ago
|
||
This is the upstream change that broke this, FWIW:
http://code.google.com/p/google-breakpad/source/detail?r=557&path=/trunk/src/client/windows/handler/exception_handler.cc
Reporter | ||
Updated•14 years ago
|
Keywords: regression
Comment 4•14 years ago
|
||
I thought Firefox 4 would drop support for Win2k, no?
Bug 563318 will break the support anyway because VC10-built binaries will not even start on Win2k.
Updated•14 years ago
|
Whiteboard: [pm]
Comment 6•14 years ago
|
||
I'm not aware of any decision to drop support for Win2K, though I'd be willing to entertain arguments for or against.
Fixing this regression seems important until such a decision is made, but I am unaware of the cost of that fix. Perhaps that has something to do with the relative desire to drop Win2K?
Comment 8•14 years ago
|
||
Just a side note, Microsoft has dropped all support for Windows 200 (http://arstechnica.com/microsoft/news/2010/07/support-for-windows-2000-and-windows-xp-service-pack-2-comes-to-an-end.ars). Not that this necessarily means we should, but it may come into consideration.
Comment 9•14 years ago
|
||
Keeping Windows 2000 support is important for enterprises and enterprise support despite Microsoft's official lack of support for it, since many large organizations both non-profit and for-profit still use it. Ideally we should provide more warning for this (e.g. stating now that Firefox 4.not-zero or 5.0 will drop Windows 2000 support officially).
Also, if memory serves, Breakpad is already broken on Windows 2000 from Firefox 3.5 or 3.6, so would it be possible to ship Windows 2000 builds without Breakpad? Some solution like this would prevent further problems with Breakpad on Windows 2000 and likely allow a longer support window for it, since Breakpad is its only recurrent problem that I'm aware of.
Comment 10•14 years ago
|
||
Building a separate build for Windows 2000 would be way more work than just fixing the specific bustage here. I think that's way out of scope.
(And yes, Breakpad doesn't actually work out of the box on Win2k.)
Comment 11•14 years ago
|
||
FWIW, a year ago Mike Connor have proposed to drop support for Windows 2000/Windows XP SP0/SP1 in Gecko 1.9.2, see discussions in http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/67ddcaa5c897a58b and http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/ef0a210ed56df40e/3e4dfa5dfc1a2be9
It's not clear if any decision has been made on this issue.
Comment 12•14 years ago
|
||
(In reply to comment #11)
> FWIW, a year ago Mike Connor have proposed to drop support for Windows
> 2000/Windows XP SP0/SP1 in Gecko 1.9.2, see discussions in
> http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/67ddcaa5c897a58b
> and
> http://groups.google.com/group/mozilla.dev.planning/browse_thread/thread/ef0a210ed56df40e/3e4dfa5dfc1a2be9
> It's not clear if any decision has been made on this issue.
The decision was made, but we also agreed that we would not do anything that breaks 2K unless absolutely necessary.
I think we could fix this by loading the RtlCaptureContext entry point manually.
Assignee | ||
Comment 13•14 years ago
|
||
FWIW, I'm running fine on win2k by just commenting out the RtlCaptureContext call, so I think that would work.
Comment 14•14 years ago
|
||
Feel free to submit a patch upstream and we can land it here and there. I don't think it's a very big patch, I just don't really care about Win2k (and clearly Google doesn't either).
Comment 15•14 years ago
|
||
I filed bug 579078 to capture the decision about Windows 2000, but that shouldn't stop this work, as per comment 12.
Updated•14 years ago
|
Summary: Bug 567424 checkin breaks win 2000 → Check-in for Breakpad 64-bit on OS X dropped Windows 2000 support
Assignee | ||
Comment 17•14 years ago
|
||
(In reply to comment #12)
> I think we could fix this by loading the RtlCaptureContext entry point
> manually.
How would one go about doing that? This is really causing me significant pain in trying to push bug 130078 to completion.
Comment 18•14 years ago
|
||
Comment 19•14 years ago
|
||
Comment on attachment 459139 [details] [diff] [review]
patch
Ted, are you the right reviewer for something like this?
Attachment #459139 -
Flags: review?(ted.mielczarek)
Comment 20•14 years ago
|
||
(In reply to comment #17)
> (In reply to comment #12)
> > I think we could fix this by loading the RtlCaptureContext entry point
> > manually.
>
> How would one go about doing that? This is really causing me significant pain
> in trying to push bug 130078 to completion.
Curious, why would win2k support in the crash reporter be an issue with testing 130078?
Comment 21•14 years ago
|
||
in theory we should be able to do this:
RtlCaptureContextPtr fnRtlCaptureContext = (RtlCaptureContextPtr)
GetProcAddress(GetModuleHandleW(L"kernel32"), "RtlCaptureContext");
if (fnRtlCaptureContext) {
fnRtlCaptureContext(&exception_context);
} else {
#ifdef _WIN64
return;
#elif !defined _MSC_VER
return;
#else
memset(&exception_context, 0, sizeof(exception_context));
exception_context.ContextFlags = CONTEXT_CONTROL;
exception_context.ContextFlags = CONTEXT_CONTROL;
__asm call x
__asm x: pop eax
__asm mov exception_context.Eip, eax
__asm mov exception_context.Ebp, ebp
__asm mov exception_context.Esp, esp
#endif
}
this is based on http://groups.google.com/group/v8-dev/browse_thread/thread/7e9d9f848da4d2a3?pli=1
Assignee | ||
Comment 22•14 years ago
|
||
(In reply to comment #20)
> Curious, why would win2k support in the crash reporter be an issue with testing
> 130078?
Because I dogfood on win2k, and 130078 needs much dogfooding. It's not win2k support in the crashreporter, its having builds that work on win2k. I need to compare to trunk builds to see if an issue is specific to my changes or not. I don't want to be pushing a win2kdiff to try everyday just to get a vanilla trunk build.
Comment 23•14 years ago
|
||
(In reply to comment #21)
> in theory we should be able to do this:
> RtlCaptureContextPtr fnRtlCaptureContext = (RtlCaptureContextPtr)
> GetProcAddress(GetModuleHandleW(L"kernel32"), "RtlCaptureContext");
> if (fnRtlCaptureContext) {
> fnRtlCaptureContext(&exception_context);
> } else {
> #ifdef _WIN64
> return;
> #elif !defined _MSC_VER
> return;
> #else
> memset(&exception_context, 0, sizeof(exception_context));
> exception_context.ContextFlags = CONTEXT_CONTROL;
> exception_context.ContextFlags = CONTEXT_CONTROL;
This seems to work fine when I trigger it on win7.
Although do we want this? From the discussion in the 2k bug, I think we were planning on not collecting data from unsupported platforms. I suppose we might be able to drop the reports on the server side, but really what's the point of sending them in the first place?
Comment 24•14 years ago
|
||
we're working to accept reports from linux distros, i don't see why we shouldn't be willing to accept reports from w2k users.
i spend a lot of time dealing w/ end users who report crashes, i much prefer to get crashes from crash-stats than talking them through windbg.
tn: can you see if this works for you too? :)
Comment 25•14 years ago
|
||
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #459194 -
Flags: review?(ted.mielczarek)
Comment 26•14 years ago
|
||
(In reply to comment #24)
> we're working to accept reports from linux distros, i don't see why we
> shouldn't be willing to accept reports from w2k users. ...
(In reply to comment #9)
> ...
> Also, if memory serves, Breakpad is already broken on Windows 2000 from Firefox
> 3.5 or 3.6, ...
At least on my box this seems to be correct so I am not sure that the extra code means that any reports will actually be sent.
Assignee | ||
Comment 27•14 years ago
|
||
(In reply to comment #26)
> (In reply to comment #9)
> > Also, if memory serves, Breakpad is already broken on Windows 2000 from Firefox
> > 3.5 or 3.6, ...
>
> At least on my box this seems to be correct so I am not sure that the extra
> code means that any reports will actually be sent.
You need to have a dbghelp.dll, but otherwise it works fine.
Updated•14 years ago
|
Attachment #459139 -
Flags: review?(ted.mielczarek)
Comment 28•14 years ago
|
||
Comment on attachment 459194 [details] [diff] [review]
patch which supports w2k x86
There's no way we'd take this much complexity upstream just to support Win2k. If we can just pass NULL as the exception pointers or something on Win2k that'd be fine.
Attachment #459194 -
Flags: review?(ted.mielczarek) → review-
Comment 32•14 years ago
|
||
Since this currently breaks on Win2k, Someone might want to update the 'system requirements' page so the bug doesn't keep getting reported and you get fewer disgruntled users.
The link from the beta currently points to the system requirements for 3.6
Comment 33•14 years ago
|
||
Moreover, 4.0b1 runs on W2k, and the update to 4.0b2 is proposed. Until today, only users of nightly faced this error, it will now affect beta testers too.
Comment 34•14 years ago
|
||
Rick: that's some other bug.
Chris: please don't spam bugs with unrelated garbage. it makes things much less likely to be fixed.
https://bugzilla.mozilla.org/page.cgi?id=etiquette.html
Comment 36•14 years ago
|
||
Comment on attachment 459139 [details] [diff] [review]
patch
I would take this patch (and I think we could take it upstream), except I think you could make it work a little better. Instead of just returning if you don't have that method, just pass NULL to WriteMinidump{WithException,OnHandlerThread}, which is what the behavior used to be before the upstream patch:
http://hg.mozilla.org/mozilla-central/annotate/e8b52ed02f99/toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.cc#l571
Attachment #459139 -
Flags: review-
Assignee | ||
Comment 37•14 years ago
|
||
Addressed Ted's comment. This is still jimm's patch, I'm just trying to push this forward.
Attachment #459139 -
Attachment is obsolete: true
Attachment #460985 -
Flags: review?(ted.mielczarek)
Comment 38•14 years ago
|
||
I hope this gets fixed - otherwise I can't start FF on Win2k.
Comment 39•14 years ago
|
||
Comment on attachment 460985 [details] [diff] [review]
patch v2
This looks ok, but can you put all the pieces where the exception_record structure is filled out inside the if (fnRtlCaptureContext) ? It seems more logical to not bother filling out the struct if we don't have the data we need.
r=me with that change.
Attachment #460985 -
Flags: review?(ted.mielczarek) → review+
Comment 40•14 years ago
|
||
As a reminder to people who are affected by this bug, Win2k-compatible builds can be generated with --disable-crashreporter until this issue is fixed. Just remember to remove it again once trunk works on Win2k without it :)
Assignee | ||
Comment 41•14 years ago
|
||
Made those changes and landed
http://hg.mozilla.org/mozilla-central/rev/779fc3ba8329
What is the procedure for getting this upstream?
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 42•14 years ago
|
||
This says it is fixed. I opened my computer this morning and FireFox asked if I wanted to restart it to accept Beta 2. I did, I got the error message "The procedure entry point RtlCaptureContext could not be located in the dynamic link library KERNEL32.Dll" I have W2K - do I go back to FF 3.68 (which is what I am using for this)?
Comment 43•14 years ago
|
||
(In reply to comment #42)
> This says it is fixed. I opened my computer this morning and FireFox asked if I
> wanted to restart it to accept Beta 2. I did, I got the error message "The
> procedure entry point RtlCaptureContext could not be located in the dynamic
> link library KERNEL32.Dll" I have W2K - do I go back to FF 3.68 (which is what
> I am using for this)?
It'll be in beta 3, which we're code freezing for early this week. In the mean time you could run a nightly or go back to 3.6.
Comment 44•14 years ago
|
||
I'll land it upstream for you when I get a chance.
Comment 45•14 years ago
|
||
Landed upstream:
http://code.google.com/p/google-breakpad/source/detail?r=637
Comment 46•14 years ago
|
||
Verified fixed with Mozilla/5.0 (Windows; Windows NT 5.0; rv:2.0b3pre) Gecko/20100802 Minefield/4.0b3pre
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla2.0b3
You need to log in
before you can comment on or make changes to this bug.
Description
•