SymbolizerResult kError will strictly mean "missing symbols" Adds a new symbolizer result, kNonRetriableError, to distinguish between missing symbols (kError) and other permanent failures like missing debug information, instruction outside module range, or internal resolver issues. Change-Id: If399cdba16d343e903c2f104c84ebce2978bd47b Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/7487708 Reviewed-by: Joshua Peraza <jperaza@chromium.org>
diff --git a/src/google_breakpad/processor/stack_frame_symbolizer.h b/src/google_breakpad/processor/stack_frame_symbolizer.h index f3173df..07ce6e8 100644 --- a/src/google_breakpad/processor/stack_frame_symbolizer.h +++ b/src/google_breakpad/processor/stack_frame_symbolizer.h
@@ -58,8 +58,8 @@ // Symbol data was found and successfully loaded in resolver. // This does NOT guarantee source line info is found within symbol file. kNoError, - // This indicates non-critical error, such as, no code module found for - // frame's instruction, no symbol file, or resolver failed to load symbol. + // This indicates a symbol file is missing. Retrying may help if the file + // becomes available later. kError, // This indicates error for which stack walk should be interrupted // and retried in future. @@ -67,6 +67,9 @@ // Symbol data was found and loaded in resolver however some corruptions // were detected. kWarningCorruptSymbols, + // Other non-retriable errors, like missing debug_file or debug_id, or + // instruction outside of module range. + kNonRetriableError, }; StackFrameSymbolizer(SymbolSupplier* supplier,
diff --git a/src/processor/stack_frame_symbolizer.cc b/src/processor/stack_frame_symbolizer.cc index 1b98a43..9f34d40 100644 --- a/src/processor/stack_frame_symbolizer.cc +++ b/src/processor/stack_frame_symbolizer.cc
@@ -71,13 +71,18 @@ module = unloaded_modules->GetModuleForAddress(frame->instruction); } - if (!module) return kError; + if (!module) { + BPLOG(INFO) << "Unable to find module from instruction"; + return kNonRetriableError; + } frame->module = module; - if (!resolver_) return kError; // no resolver. + if (!resolver_) return kNonRetriableError; // no resolver. // If module is known to have missing symbol file, return. if (no_symbol_modules_.find(module->code_file()) != no_symbol_modules_.end()) { + BPLOG(INFO) << "module is known to have missing symbol file: " + << module->code_file(); return kError; } @@ -90,7 +95,7 @@ // Module needs to fetch symbol file. First check to see if supplier exists. if (!supplier_) { - return kError; + return kNonRetriableError; } // Start fetching symbol from supplier. @@ -116,8 +121,7 @@ kWarningCorruptSymbols : kNoError; } else { BPLOG(ERROR) << "Failed to load symbol file in resolver."; - no_symbol_modules_.insert(module->code_file()); - return kError; + return kNonRetriableError; } } @@ -130,9 +134,8 @@ default: BPLOG(ERROR) << "Unknown SymbolResult enum: " << symbol_result; - return kError; } - return kError; + return kNonRetriableError; } WindowsFrameInfo* StackFrameSymbolizer::FindWindowsFrameInfo(
diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc index e84845e..0390f22 100644 --- a/src/processor/stackwalker.cc +++ b/src/processor/stackwalker.cc
@@ -165,6 +165,8 @@ break; case StackFrameSymbolizer::kNoError: break; + case StackFrameSymbolizer::kNonRetriableError: + break; default: assert(false); break;
diff --git a/src/processor/stackwalker_amd64_unittest.cc b/src/processor/stackwalker_amd64_unittest.cc index 76d4bd7..e82a5e6 100644 --- a/src/processor/stackwalker_amd64_unittest.cc +++ b/src/processor/stackwalker_amd64_unittest.cc
@@ -163,8 +163,7 @@ vector<const CodeModule*> modules_with_corrupt_symbols; ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, &modules_with_corrupt_symbols)); - ASSERT_EQ(1U, modules_without_symbols.size()); - ASSERT_EQ("module1", modules_without_symbols[0]->debug_file()); + ASSERT_EQ(0U, modules_without_symbols.size()); ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_GE(1U, frames->size());
diff --git a/src/processor/stackwalker_mips64_unittest.cc b/src/processor/stackwalker_mips64_unittest.cc index 4f702fb..d566425 100644 --- a/src/processor/stackwalker_mips64_unittest.cc +++ b/src/processor/stackwalker_mips64_unittest.cc
@@ -165,8 +165,7 @@ vector<const CodeModule*> modules_with_corrupt_symbols; ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, &modules_with_corrupt_symbols)); - ASSERT_EQ(1U, modules_without_symbols.size()); - ASSERT_EQ("module1", modules_without_symbols[0]->debug_file()); + ASSERT_EQ(0U, modules_without_symbols.size()); ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(1U, frames->size());
diff --git a/src/processor/stackwalker_mips_unittest.cc b/src/processor/stackwalker_mips_unittest.cc index 558ffe7..a8ecbd9 100644 --- a/src/processor/stackwalker_mips_unittest.cc +++ b/src/processor/stackwalker_mips_unittest.cc
@@ -163,8 +163,7 @@ vector<const CodeModule*> modules_with_corrupt_symbols; ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, &modules_with_corrupt_symbols)); - ASSERT_EQ(1U, modules_without_symbols.size()); - ASSERT_EQ("module1", modules_without_symbols[0]->debug_file()); + ASSERT_EQ(0U, modules_without_symbols.size()); ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(1U, frames->size());
diff --git a/src/processor/stackwalker_x86_unittest.cc b/src/processor/stackwalker_x86_unittest.cc index 2b7e87a..adce015 100644 --- a/src/processor/stackwalker_x86_unittest.cc +++ b/src/processor/stackwalker_x86_unittest.cc
@@ -173,8 +173,7 @@ vector<const CodeModule*> modules_with_corrupt_symbols; ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, &modules_with_corrupt_symbols)); - ASSERT_EQ(1U, modules_without_symbols.size()); - ASSERT_EQ("module1", modules_without_symbols[0]->debug_file()); + ASSERT_EQ(0U, modules_without_symbols.size()); ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); StackFrameX86 *frame = static_cast<StackFrameX86*>(frames->at(0));