586048 - Report graphics card and graphics driver version in crash dumps

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Ted, what is the best way to get data included in crash reports?

Comment 1

[Benjamin Smedberg]

http://mxr.mozilla.org/mozilla-central/source/xpcom/system/nsICrashReporter.idl#90

Comment 2

[Benjamin Smedberg]

If you want to be able to search over the data in crash-stats, you will also need to file a bug to collect the data into the database and crash UI for it. If you just want it to be human-readable, use appendAppNotesToCrashReport.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #2)
> If you want to be able to search over the data in crash-stats, you will also
> need to file a bug to collect the data into the database and crash UI for it.

Can we use AnnotateCrashReport and update the database later or do they need to be done in lock-step?

Also, it seems like AnnotateCrashReport would be called during startup. I'm worried about negatively impacting startup. What are the options if we want to do the detection at crash time?

Comment 4

[Damon Sicore (:damons)]

I'm going to mark this as a blocker, just to make sure it's on people's radar.  I think we'll need this info in advance to make sure we consider all sources of crashes once we enable hardware acceleration.  Blocking betaN+.

Comment 5

[(not currently active) Ted Mielczarek]

(In reply to comment #3)
> (In reply to comment #2)
> > If you want to be able to search over the data in crash-stats, you will also
> > need to file a bug to collect the data into the database and crash UI for it.
> 
> Can we use AnnotateCrashReport and update the database later or do they need to
> be done in lock-step?

You can send any data you want, crash-stats will simply ignore unknown keys.

> Also, it seems like AnnotateCrashReport would be called during startup. I'm
> worried about negatively impacting startup. What are the options if we want to
> do the detection at crash time?

We annotate a bunch of data during startup right now. Are you worried that actually gathering the data will be slow? I would recommend against doing anything in the exception handler, since it's unsafe to do almost anything at that point. If you only want to send this data for browser crashes (and not plugin or content crashes), then you could feasibly do it in the crash reporter client instead. We don't currently do anything like that, but there's no real reason you couldn't.

Comment 6

[Benjamin Smedberg]

Why does this block testpilot? It should be possible to design testpilot collection of this data using ctypes, and skip crash reporter altogether, if you wanted to get something together quickly.

Comment 7

[Benjamin Smedberg]

If this is possibly slow, I would fork a thread to collect the data and annotate once you've collected it. Unless you also think that crashes are likely right at startup.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add graphics card vendor and device ids to crash reports

The vendor and device ids should be enough for us to get a sense of what graphics hardware is out there. We can already get driver version from the module list provided D3D has been loaded. We can add more information as the need arises.

Comment 10

[Benjamin Smedberg]

Comment on attachment 467845 [details] [diff] [review]
Add graphics card vendor and device ids to crash reports

You haven't arranged for anyone to call this method? Maybe missed a file in qref?

With AnnotateCrashReport, you'll need socorro database changes in order to see the results, which is not going to be quick. Assuming you want this data right away, I think you should use the comment method.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Attachment: New version with some comments addressed

Comment 12

[Jeff Muizelaar [:jrmuizel]]

Attachment: Actually annotate at startup

I assumed services were started at startup instead of at the first use. This patch initializes GfxInfo during gfxPlatform initialization.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 470146 [details] [diff] [review]
Actually annotate at startup

>diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp

>+    /* Initialize the GfxInfo service */
>+    nsCOMPtr<nsIGfxInfo> gfxInfo;
>+    gfxInfo = do_GetService("@mozilla.org/gfx/info;1");
>+    if (gfxInfo)
>+        gfxInfo->AddCrashReportAnnotations();

Maybe you should put a comment about not calling GetD2DEnabled and GetDwriteEnabled here (and in AddCrashReportAnnotations?) just in case. And reference the followup.

>diff --git a/widget/src/windows/GfxInfo.cpp b/widget/src/windows/GfxInfo.cpp

>+/* GetD2DEnabled and GetDwriteEnabled shouldn't be called until after gfxPlatform initialization
>+ * has occured because they depend on it for information */

occurred

>@@ -232,9 +234,6 @@ void GfxInfo::Init()

>-  AddCrashReportAnnotations();

I think I would prefer leaving this here and just commenting about the fact that instantiation causes annotations to be added next to the getService().

Comment 14

[Jeff Muizelaar [:jrmuizel]]

Attachment: Actually annotate at startup v2

Address review comments.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 470149 [details] [diff] [review]
Actually annotate at startup v2

>diff --git a/widget/public/Makefile.in b/widget/public/Makefile.in

>+		nsIGfxInfo.idl \

>-		nsIGfxInfo.idl \

This doesn't seem worth doing until we actually have other implementations.

>diff --git a/widget/src/windows/GfxInfo.cpp b/widget/src/windows/GfxInfo.cpp

>@@ -232,9 +234,6 @@ void GfxInfo::Init()

>-  AddCrashReportAnnotations();

Need to revert this change too.

Comment 16

[Jeff Muizelaar [:jrmuizel]]

Attachment: Actually annotate at startup v3

Finished version

Comment 17

[Philip Chee]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1283011877.1283012858.6022.gz
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1283011877.1283012651.5031.gz

make[7]: Entering directory `/e/builds/slave/comm-central-trunk-win32/build/objdir/mozilla/widget/src/build'
link -NOLOGO -DLL -OUT:gkwidget.dll -PDB:gkwidget.pdb -SUBSYSTEM:WINDOWS  nsWinWidgetFactory.obj    widget.res -NXCOMPAT -DYNAMICBASE -SAFESEH  -DEBUG -DEBUGTYPE:CV -MANIFEST:NO -LIBPATH:"e:/builds/slave/comm-central-trunk-win32/build/objdir/mozilla/memory/jemalloc/crtsrc/build/intel" -NODEFAULTLIB:msvcrt -NODEFAULTLIB:msvcrtd -NODEFAULTLIB:msvcprt -NODEFAULTLIB:msvcprtd -DEFAULTLIB:mozcrt19 -DEFAULTLIB:mozcpp19 -DEBUG -OPT:REF   -IMPLIB:fake.lib  @../windows/widget_windows.lib.fake @../xpwidgets/xpwidgets_s.lib.fake   ../../../dist/lib/gkgfx.lib ../../../dist/lib/thebes.lib e:/builds/slave/comm-central-trunk-win32/build/objdir/mozilla/dist/lib/xpcom.lib e:/builds/slave/comm-central-trunk-win32/build/objdir/mozilla/dist/lib/xpcom_core.lib e:/builds/slave/comm-central-trunk-win32/build/objdir/mozilla/dist/lib/mozalloc.lib e:/builds/slave/comm-central-trunk-win32/build/objdir/mozilla/dist/lib/nspr4.lib e:/builds/slave/comm-central-trunk-win32/build/objdir/mozilla/dist/lib/plc4.lib e:/builds/slave/comm-central-trunk-win32/build/objdir/mozilla/dist/lib/plds4.lib  e:/builds/slave/comm-central-trunk-win32/build/objdir/mozilla/dist/lib/unicharutil_s.lib ../../../gfx/qcms/mozqcms.lib   kernel32.lib user32.lib gdi32.lib winmm.lib wsock32.lib advapi32.lib uuid.lib ole32.lib oleaut32.lib comctl32.lib comdlg32.lib shell32.lib imm32.lib shlwapi.lib winspool.lib msimg32.lib  
   Creating library fake.lib and object fake.exp
GfxInfo.obj : error LNK2019: unresolved external symbol "unsigned int __cdecl CrashReporter::AppendAppNotesToCrashReport(class nsACString_internal const &)" (?AppendAppNotesToCrashReport@CrashReporter@@YAIABVnsACString_internal@@@Z) referenced in function "private: void __thiscall mozilla::widget::GfxInfo::AddCrashReportAnnotations(void)" (?AddCrashReportAnnotations@GfxInfo@widget@mozilla@@AAEXXZ)
GfxInfo.obj : error LNK2019: unresolved external symbol "unsigned int __cdecl CrashReporter::AnnotateCrashReport(class nsACString_internal const &,class nsACString_internal const &)" (?AnnotateCrashReport@CrashReporter@@YAIABVnsACString_internal@@0@Z) referenced in function "private: void __thiscall mozilla::widget::GfxInfo::AddCrashReportAnnotations(void)" (?AddCrashReportAnnotations@GfxInfo@widget@mozilla@@AAEXXZ)
gkwidget.dll : fatal error LNK1120: 2 unresolved externals

Comment 18

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #17)

Can you try out the patch in #591576?

Comment 19

[Jeff Muizelaar [:jrmuizel]]

Attachment: Fix reporting of device ID

I was checking the crash reporter results and noticed they seemed a little funny. It turns out we were printing the addresses of the ids instead of the values. This patch fixes that and improves the formatting of the app notes a little bit.

Comment 20

[Benjamin Smedberg]

Comment on attachment 470366 [details] [diff] [review]
Fix reporting of device ID

Since we're here, can you call AppendAppNotesToCrashReport just once, and also add a newline at the end so that other notes don't collide?

Comment 21

[Jeff Muizelaar [:jrmuizel]]

http://hg.mozilla.org/mozilla-central/rev/c8aa01bfccb2

Comment 22

[chris hofmann]

just wanted to reference another crashed bug that spiked in beta5 when testpilot tried to gather graphics card info.   

https://bugzilla.mozilla.org/show_bug.cgi?id=594608#c14

As a precaution we should be on the look out for any similar crashes or tickling of any similar bugs that test pilot might have stumbled upon.   Are there any test cases associated with this check-in, or any good ways to test?

Comment 23

[Jeff Muizelaar [:jrmuizel]]

Having a test that loads about:support would probably help.