aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornealsid <nealsid@4c0a9323-5329-0410-9bdc-e9ce6186880e>2010-03-03 01:29:04 +0000
committernealsid <nealsid@4c0a9323-5329-0410-9bdc-e9ce6186880e>2010-03-03 01:29:04 +0000
commit19374d263649a51c8bb56f2f01d3466905847670 (patch)
tree1c0c3d639bb651ce56fa57dcb6093910e2301cf3
parentBreakpad Linux dumper: Tolerate STABS data from code linked with --gc-sections. (diff)
downloadbreakpad-19374d263649a51c8bb56f2f01d3466905847670.tar.xz
Fix to cache NOT_FOUND results from symbol supplier on a per-minidump basis
http://breakpad.appspot.com/64001 R=ted.mielczarek, brdevmn A=nealsid git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@543 4c0a9323-5329-0410-9bdc-e9ce6186880e
-rw-r--r--src/google_breakpad/processor/stackwalker.h10
-rw-r--r--src/processor/minidump_processor_unittest.cc57
-rw-r--r--src/processor/stackwalker.cc3
3 files changed, 68 insertions, 2 deletions
diff --git a/src/google_breakpad/processor/stackwalker.h b/src/google_breakpad/processor/stackwalker.h
index a475b230..72da76b0 100644
--- a/src/google_breakpad/processor/stackwalker.h
+++ b/src/google_breakpad/processor/stackwalker.h
@@ -41,12 +41,13 @@
#ifndef GOOGLE_BREAKPAD_PROCESSOR_STACKWALKER_H__
#define GOOGLE_BREAKPAD_PROCESSOR_STACKWALKER_H__
-#include <vector>
+#include <set>
#include "google_breakpad/common/breakpad_types.h"
namespace google_breakpad {
class CallStack;
+class CodeModule;
class CodeModules;
class MemoryRegion;
class MinidumpContext;
@@ -55,7 +56,7 @@ struct StackFrame;
class SymbolSupplier;
class SystemInfo;
-using std::vector;
+using std::set;
class Stackwalker {
@@ -139,6 +140,11 @@ class Stackwalker {
// The optional SymbolSupplier for resolving source line info.
SymbolSupplier *supplier_;
+
+ // A list of modules that we haven't found symbols for. We track
+ // this in order to avoid repeatedly looking them up again within
+ // one minidump.
+ set<std::string> no_symbol_modules_;
};
diff --git a/src/processor/minidump_processor_unittest.cc b/src/processor/minidump_processor_unittest.cc
index 7df12172..dd4f633e 100644
--- a/src/processor/minidump_processor_unittest.cc
+++ b/src/processor/minidump_processor_unittest.cc
@@ -34,6 +34,7 @@
#include <string>
#include <iostream>
#include <fstream>
+#include <map>
#include "breakpad_googletest_includes.h"
#include "google_breakpad/processor/basic_source_line_resolver.h"
#include "google_breakpad/processor/call_stack.h"
@@ -47,6 +48,8 @@
#include "processor/logging.h"
#include "processor/scoped_ptr.h"
+using std::map;
+
namespace google_breakpad {
class MockMinidump : public Minidump {
public:
@@ -74,6 +77,10 @@ using google_breakpad::scoped_ptr;
using google_breakpad::SymbolSupplier;
using google_breakpad::SystemInfo;
using std::string;
+using ::testing::_;
+using ::testing::Mock;
+using ::testing::Ne;
+using ::testing::Property;
using ::testing::Return;
static const char *kSystemInfoOS = "Windows NT";
@@ -155,6 +162,19 @@ SymbolSupplier::SymbolResult TestSymbolSupplier::GetSymbolFile(
return s;
}
+// A mock symbol supplier that always returns NOT_FOUND; one current
+// use for testing the processor's caching of symbol lookups.
+class MockSymbolSupplier : public SymbolSupplier {
+ public:
+ MockSymbolSupplier() { }
+ MOCK_METHOD3(GetSymbolFile, SymbolResult(const CodeModule*,
+ const SystemInfo*,
+ string*));
+ MOCK_METHOD4(GetSymbolFile, SymbolResult(const CodeModule*,
+ const SystemInfo*,
+ string*,
+ string*));
+};
class MinidumpProcessorTest : public ::testing::Test {
@@ -186,6 +206,43 @@ TEST_F(MinidumpProcessorTest, TestCorruptMinidumps) {
google_breakpad::PROCESS_ERROR_NO_THREAD_LIST);
}
+// This test case verifies that the symbol supplier is only consulted
+// once per minidump per module.
+TEST_F(MinidumpProcessorTest, TestSymbolSupplierLookupCounts) {
+ MockSymbolSupplier supplier;
+ BasicSourceLineResolver resolver;
+ MinidumpProcessor processor(&supplier, &resolver);
+
+ string minidump_file = string(getenv("srcdir") ? getenv("srcdir") : ".") +
+ "/src/processor/testdata/minidump2.dmp";
+ ProcessState state;
+ EXPECT_CALL(supplier, GetSymbolFile(
+ Property(&google_breakpad::CodeModule::code_file,
+ "c:\\test_app.exe"),
+ _, _, _)).WillOnce(Return(SymbolSupplier::NOT_FOUND));
+ EXPECT_CALL(supplier, GetSymbolFile(
+ Property(&google_breakpad::CodeModule::code_file,
+ Ne("c:\\test_app.exe")),
+ _, _, _)).WillRepeatedly(Return(SymbolSupplier::NOT_FOUND));
+ ASSERT_EQ(processor.Process(minidump_file, &state),
+ google_breakpad::PROCESS_OK);
+
+ ASSERT_TRUE(Mock::VerifyAndClearExpectations(&supplier));
+
+ // We need to verify that across minidumps, the processor will refetch
+ // symbol files, even with the same symbol supplier.
+ EXPECT_CALL(supplier, GetSymbolFile(
+ Property(&google_breakpad::CodeModule::code_file,
+ "c:\\test_app.exe"),
+ _, _, _)).WillOnce(Return(SymbolSupplier::NOT_FOUND));
+ EXPECT_CALL(supplier, GetSymbolFile(
+ Property(&google_breakpad::CodeModule::code_file,
+ Ne("c:\\test_app.exe")),
+ _, _, _)).WillRepeatedly(Return(SymbolSupplier::NOT_FOUND));
+ ASSERT_EQ(processor.Process(minidump_file, &state),
+ google_breakpad::PROCESS_OK);
+}
+
TEST_F(MinidumpProcessorTest, TestBasicProcessing) {
TestSymbolSupplier supplier;
BasicSourceLineResolver resolver;
diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc
index 3a3725fa..3b9a313a 100644
--- a/src/processor/stackwalker.cc
+++ b/src/processor/stackwalker.cc
@@ -93,6 +93,8 @@ bool Stackwalker::Walk(CallStack *stack) {
frame->module = module;
if (resolver_ &&
!resolver_->HasModule(frame->module->code_file()) &&
+ no_symbol_modules_.find(
+ module->code_file()) == no_symbol_modules_.end() &&
supplier_) {
string symbol_data, symbol_file;
SymbolSupplier::SymbolResult symbol_result =
@@ -105,6 +107,7 @@ bool Stackwalker::Walk(CallStack *stack) {
symbol_data);
break;
case SymbolSupplier::NOT_FOUND:
+ no_symbol_modules_.insert(module->code_file());
break; // nothing to do
case SymbolSupplier::INTERRUPT:
return false;