This might be useful in some cases, e.g. bug 1296630.

Would you just add it to the CPU description, or have a separate field?

Separate field maybe? So we have more flexibility when using SuperSearch.

The CPU description is generated by Breakpad from some structs in the minidump itself (which we can't change because they're defined by Microsoft), so I think it'd be better to put it in a separate field.

I was hoping we'd be able to extend Breakpad to show this extra information, but sounds like that is not an option.

Most of the code got reviewed in https://bugzilla.mozilla.org/attachment.cgi?id=8790490&action=edit, but I want to make sure this is the correct location for this kind of a functionality.

Comment on attachment 8795455 [details] Bug 1305120: CPU microcode as a separate field in the crash report. .mielczarek https://reviewboard.mozilla.org/r/81496/#review81400 ::: toolkit/xre/nsAppRunner.cpp:3451 (Diff revision 2) > + &len) == ERROR_SUCCESS && > + vtype == REG_BINARY && len == sizeof(updateRevision)) { > + > + // The first word is unused > + cpuUpdateRevision = static_cast<int>(updateRevision[1]); > + } else if (RegQueryValueExW(key, L"Update Revision", Seems like you'd be better served writing this as a loop and breaking on the first successful read. ::: toolkit/xre/nsAppRunner.cpp:3463 (Diff revision 2) > + cpuUpdateRevision = static_cast<int>(updateRevision[1]); > + } > + } > + > + if (cpuUpdateRevision > 0) { > + CrashReporter::AnnotateCrashReport(NS_LITERAL_CSTRING("Microcode"), Can we give this a slightly more informative name, like maybe "CPU_Microcode_Revision"?

Milan, did you want to ask for review again or carry r+ and land the patch?

(In reply to Marco Castelluccio [:marco] from comment #10) > Milan, did you want to ask for review again or carry r+ and land the patch? Once it passes try, I will carry r+ and land it.

Pushed by msreckovic@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d382e432aa08 CPU microcode as a separate field in the crash report. r=ted.mielczarek

I'm a little worried that it's not showing up in the Metadata tab of any of today's crash reports: https://crash-stats.mozilla.com/search/?build_id=20161007030207&release_channel=nightly&product=Firefox&platform=Windows&date=%3E%3D2016-10-07T00%3A00%3A00.000Z&date=%3C2016-10-07T16%3A19%3A00.000Z&_sort=-date&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=process_type#crash-reports Should I be expecting it to be there? If this is expected until bug 1305888 happens, though, could you request uplift to aurora and beta for debugging of bug 1296630?

Comment on attachment 8795455 [details] Bug 1305120: CPU microcode as a separate field in the crash report. .mielczarek Approval Request Comment Need the extra information for bug 1296630. Because we need the edit made in bug 1308385, I will attach a aurora/beta rolled up patch here.

(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #16) > I'm a little worried that it's not showing up in the Metadata tab of any of > today's crash reports: > https://crash-stats.mozilla.com/search/ > ?build_id=20161007030207&release_channel=nightly&product=Firefox&platform=Win > dows&date=%3E%3D2016-10-07T00%3A00%3A00.000Z&date=%3C2016-10-07T16%3A19%3A00. > 000Z&_sort=- > date&_facets=signature&_columns=date&_columns=signature&_columns=product&_col > umns=version&_columns=build_id&_columns=platform&_columns=process_type#crash- > reports > > Should I be expecting it to be there? > > > If this is expected until bug 1305888 happens, though, could you request > uplift to aurora and beta for debugging of bug 1296630? It is showing up for me, e.g. https://crash-stats.mozilla.com/report/index/2c4bc882-96b2-4456-95ea-61d8a2161007#tab-metadata has CPUMicrocodeVersion: 0x1b. Perhaps you need to be logged-in in order to see it.

(In reply to Marco Castelluccio [:marco] from comment #19) > Perhaps you need to be logged-in in order to see it. Yes, that was the problem.

Comment on attachment 8795455 [details] Bug 1305120: CPU microcode as a separate field in the crash report. .mielczarek We need this to fix the Intel crashes, Aurora51+, Beta50+