• R/O
  • HTTP
  • SSH
  • HTTPS

thirdparty-breakpad: Commit

Breakpad, a crash reporter, from Google.

Original home: https://chromium.googlesource.com/breakpad/breakpad/


Commit MetaInfo

Revision70914b2d380d893364ad0110b8af18ba1ed5aaa3 (tree)
Time2017-11-14 23:31:22
AuthorMike Wittman <wittman@chro...>
CommiterMark Mentovai

Log Message

Make identical-code-folded symbol output more consistent between runs

Consistently output the "least" symbol by decorated name when
multiple symbols share an address.

Testing with chrome.dll.pdb the diffs between the new and old output
look sensible, and this is actually ~20% faster than the existing
implementation.

Bug: 749
Change-Id: Ie638559b63f0eb2dcb80b1ebb579228d62c63bb2
Reviewed-on: https://chromium-review.googlesource.com/758885
Reviewed-by: Mark Mentovai <mark@chromium.org>

Change Summary

Incremental Difference

--- a/src/common/windows/pdb_source_line_writer.cc
+++ b/src/common/windows/pdb_source_line_writer.cc
@@ -37,8 +37,11 @@
3737 #include <ImageHlp.h>
3838 #include <stdio.h>
3939
40+#include <algorithm>
4041 #include <limits>
42+#include <map>
4143 #include <set>
44+#include <utility>
4245
4346 #include "common/windows/dia_util.h"
4447 #include "common/windows/guid_string.h"
@@ -106,6 +109,8 @@ namespace {
106109
107110 using std::vector;
108111
112+typedef std::multimap<DWORD, CComPtr<IDiaSymbol>> SymbolMultimap;
113+
109114 // A helper class to scope a PLOADED_IMAGE.
110115 class AutoImage {
111116 public:
@@ -166,6 +171,16 @@ bool CreateDiaDataSourceInstance(CComPtr<IDiaDataSource> &data_source) {
166171 return false;
167172 }
168173
174+// Computing undecorated names for all symbols is expensive, so we compare
175+// decorated names.
176+bool CompareSymbols(const SymbolMultimap::value_type& a,
177+ const SymbolMultimap::value_type& b) {
178+ BSTR a_name, b_name;
179+ a.second->get_name(&a_name);
180+ b.second->get_name(&b_name);
181+ return wcscmp(a_name, b_name) < 0;
182+}
183+
169184 } // namespace
170185
171186 PDBSourceLineWriter::PDBSourceLineWriter() : output_(NULL) {
@@ -400,7 +415,7 @@ bool PDBSourceLineWriter::PrintFunctions() {
400415 CComPtr<IDiaEnumSymbols> symbols = NULL;
401416
402417 // Find all function symbols first.
403- std::set<DWORD> rvas;
418+ SymbolMultimap rva_symbols;
404419 hr = global->findChildren(SymTagFunction, NULL, nsNone, &symbols);
405420
406421 if (SUCCEEDED(hr)) {
@@ -408,9 +423,10 @@ bool PDBSourceLineWriter::PrintFunctions() {
408423
409424 while (SUCCEEDED(symbols->Next(1, &symbol, &count)) && count == 1) {
410425 if (SUCCEEDED(symbol->get_relativeVirtualAddress(&rva))) {
411- // To maintain existing behavior of one symbol per address, place the
412- // rva onto a set here to uniquify them.
413- rvas.insert(rva);
426+ // Place the symbols into a multimap indexed by rva, so we can choose
427+ // the apropriate symbol name to use when multiple symbols share an
428+ // address.
429+ rva_symbols.insert(std::make_pair(rva, symbol));
414430 } else {
415431 fprintf(stderr, "get_relativeVirtualAddress failed on the symbol\n");
416432 return false;
@@ -432,8 +448,9 @@ bool PDBSourceLineWriter::PrintFunctions() {
432448
433449 while (SUCCEEDED(symbols->Next(1, &symbol, &count)) && count == 1) {
434450 if (SUCCEEDED(symbol->get_relativeVirtualAddress(&rva))) {
435- if (rvas.count(rva) == 0) {
436- rvas.insert(rva); // Keep symbols in rva order.
451+ if (rva_symbols.find(rva) == rva_symbols.end()) {
452+ // Keep symbols in rva order.
453+ rva_symbols.insert(std::make_pair(rva, symbol));
437454 public_only_rvas.insert(rva);
438455 }
439456 } else {
@@ -447,40 +464,29 @@ bool PDBSourceLineWriter::PrintFunctions() {
447464 symbols.Release();
448465 }
449466
450- std::set<DWORD>::iterator it;
451-
452- // For each rva, dump the first symbol DIA knows about at the address.
453- for (it = rvas.begin(); it != rvas.end(); ++it) {
454- CComPtr<IDiaSymbol> symbol = NULL;
455- // If the symbol is not in the public list, look for SymTagFunction. This is
456- // a workaround to a bug where DIA will hang if searching for a private
457- // symbol at an address where only a public symbol exists.
458- // See http://connect.microsoft.com/VisualStudio/feedback/details/722366
459- if (public_only_rvas.count(*it) == 0) {
460- if (SUCCEEDED(session_->findSymbolByRVA(*it, SymTagFunction, &symbol))) {
461- // Sometimes findSymbolByRVA returns S_OK, but NULL.
462- if (symbol) {
463- if (!PrintFunction(symbol, symbol))
464- return false;
465- symbol.Release();
466- }
467- } else {
468- fprintf(stderr, "findSymbolByRVA SymTagFunction failed\n");
467+ // For each rva, dump one symbol at the address.
468+ SymbolMultimap::iterator it = rva_symbols.begin();
469+ while (it != rva_symbols.end()) {
470+ std::pair<SymbolMultimap::iterator, SymbolMultimap::iterator> symbol_range =
471+ rva_symbols.equal_range(it->first);
472+ // Find the minimum symbol by name to make the output more consistent
473+ // between runs on different releases of the same module, in the case of
474+ // multiple symbols sharing an address.
475+ SymbolMultimap::iterator least_symbol_iter =
476+ std::min_element(symbol_range.first, symbol_range.second, CompareSymbols);
477+ CComPtr<IDiaSymbol> symbol = least_symbol_iter->second;
478+ // Only print public symbols if there is no function symbol for the address.
479+ if (public_only_rvas.count(it->first) == 0) {
480+ if (!PrintFunction(symbol, symbol))
469481 return false;
470- }
471- } else if (SUCCEEDED(session_->findSymbolByRVA(*it,
472- SymTagPublicSymbol,
473- &symbol))) {
474- // Sometimes findSymbolByRVA returns S_OK, but NULL.
475- if (symbol) {
476- if (!PrintCodePublicSymbol(symbol))
477- return false;
478- symbol.Release();
479- }
482+ symbol.Release();
480483 } else {
481- fprintf(stderr, "findSymbolByRVA SymTagPublicSymbol failed\n");
482- return false;
484+ if (!PrintCodePublicSymbol(symbol))
485+ return false;
486+ symbol.Release();
483487 }
488+
489+ it = symbol_range.second;
484490 }
485491
486492 // When building with PGO, the compiler can split functions into
Show on old repository browser