diff options
Diffstat (limited to 'src')
34 files changed, 572 insertions, 213 deletions
diff --git a/src/google_breakpad/processor/basic_source_line_resolver.h b/src/google_breakpad/processor/basic_source_line_resolver.h index 8bda5784..c1946ca4 100644 --- a/src/google_breakpad/processor/basic_source_line_resolver.h +++ b/src/google_breakpad/processor/basic_source_line_resolver.h @@ -59,6 +59,7 @@ class BasicSourceLineResolver : public SourceLineResolverBase { using SourceLineResolverBase::ShouldDeleteMemoryBufferAfterLoadModule; using SourceLineResolverBase::UnloadModule; using SourceLineResolverBase::HasModule; + using SourceLineResolverBase::IsModuleCorrupt; using SourceLineResolverBase::FillSourceLineInfo; using SourceLineResolverBase::FindWindowsFrameInfo; using SourceLineResolverBase::FindCFIFrameInfo; diff --git a/src/google_breakpad/processor/fast_source_line_resolver.h b/src/google_breakpad/processor/fast_source_line_resolver.h index 60f6dfce..fdf91077 100644 --- a/src/google_breakpad/processor/fast_source_line_resolver.h +++ b/src/google_breakpad/processor/fast_source_line_resolver.h @@ -63,6 +63,7 @@ class FastSourceLineResolver : public SourceLineResolverBase { using SourceLineResolverBase::FindCFIFrameInfo; using SourceLineResolverBase::FindWindowsFrameInfo; using SourceLineResolverBase::HasModule; + using SourceLineResolverBase::IsModuleCorrupt; using SourceLineResolverBase::LoadModule; using SourceLineResolverBase::LoadModuleUsingMapBuffer; using SourceLineResolverBase::LoadModuleUsingMemoryBuffer; diff --git a/src/google_breakpad/processor/process_state.h b/src/google_breakpad/processor/process_state.h index d234ebbf..2a306f6f 100644 --- a/src/google_breakpad/processor/process_state.h +++ b/src/google_breakpad/processor/process_state.h @@ -109,6 +109,9 @@ class ProcessState { const vector<const CodeModule*>* modules_without_symbols() const { return &modules_without_symbols_; } + const vector<const CodeModule*>* modules_with_corrupt_symbols() const { + return &modules_with_corrupt_symbols_; + } ExploitabilityRating exploitability() const { return exploitability_; } private: @@ -164,6 +167,9 @@ class ProcessState { // The modules that didn't have symbols when the report was processed. vector<const CodeModule*> modules_without_symbols_; + // The modules that had corrupt symbols when the report was processed. + vector<const CodeModule*> modules_with_corrupt_symbols_; + // The exploitability rating as determined by the exploitability // engine. When the exploitability engine is not enabled this // defaults to EXPLOITABILITY_NONE. diff --git a/src/google_breakpad/processor/source_line_resolver_base.h b/src/google_breakpad/processor/source_line_resolver_base.h index efa76e7e..c720b0c3 100644 --- a/src/google_breakpad/processor/source_line_resolver_base.h +++ b/src/google_breakpad/processor/source_line_resolver_base.h @@ -42,6 +42,7 @@ #define GOOGLE_BREAKPAD_PROCESSOR_SOURCE_LINE_RESOLVER_BASE_H__ #include <map> +#include <set> #include <string> #include "google_breakpad/processor/source_line_resolver_interface.h" @@ -49,6 +50,7 @@ namespace google_breakpad { using std::map; +using std::set; // Forward declaration. // ModuleFactory is a simple factory interface for creating a Module instance @@ -62,7 +64,9 @@ class SourceLineResolverBase : public SourceLineResolverInterface { // LoadMap() method. // Place dynamically allocated heap buffer in symbol_data. Caller has the // ownership of the buffer, and should call delete [] to free the buffer. - static bool ReadSymbolFile(char **symbol_data, const string &file_name); + static bool ReadSymbolFile(const string &file_name, + char **symbol_data, + size_t *symbol_data_size); protected: // Users are not allowed create SourceLineResolverBase instance directly. @@ -74,10 +78,12 @@ class SourceLineResolverBase : public SourceLineResolverInterface { virtual bool LoadModuleUsingMapBuffer(const CodeModule *module, const string &map_buffer); virtual bool LoadModuleUsingMemoryBuffer(const CodeModule *module, - char *memory_buffer); + char *memory_buffer, + size_t memory_buffer_size); virtual bool ShouldDeleteMemoryBufferAfterLoadModule(); virtual void UnloadModule(const CodeModule *module); virtual bool HasModule(const CodeModule *module); + virtual bool IsModuleCorrupt(const CodeModule *module); virtual void FillSourceLineInfo(StackFrame *frame); virtual WindowsFrameInfo *FindWindowsFrameInfo(const StackFrame *frame); virtual CFIFrameInfo *FindCFIFrameInfo(const StackFrame *frame); @@ -97,6 +103,10 @@ class SourceLineResolverBase : public SourceLineResolverInterface { typedef map<string, Module*, CompareString> ModuleMap; ModuleMap *modules_; + // The loaded modules that were detecting to be corrupt during load. + typedef set<string, CompareString> ModuleSet; + ModuleSet *corrupt_modules_; + // All of heap-allocated buffers that are owned locally by resolver. typedef std::map<string, char*, CompareString> MemoryMap; MemoryMap *memory_buffers_; diff --git a/src/google_breakpad/processor/source_line_resolver_interface.h b/src/google_breakpad/processor/source_line_resolver_interface.h index 7b69b0d3..a694bf2e 100644 --- a/src/google_breakpad/processor/source_line_resolver_interface.h +++ b/src/google_breakpad/processor/source_line_resolver_interface.h @@ -64,12 +64,15 @@ class SourceLineResolverInterface { virtual bool LoadModuleUsingMapBuffer(const CodeModule *module, const string &map_buffer) = 0; - // Add an interface to load symbol using C-String data insteading string. + // Add an interface to load symbol using C-String data instead of string. // This is useful in the optimization design for avoiding unnecessary copying // of symbol data, in order to improve memory efficiency. // LoadModuleUsingMemoryBuffer() does NOT take ownership of memory_buffer. + // LoadModuleUsingMemoryBuffer() null terminates the passed in buffer, if + // the last character is not a null terminator. virtual bool LoadModuleUsingMemoryBuffer(const CodeModule *module, - char *memory_buffer) = 0; + char *memory_buffer, + size_t memory_buffer_size) = 0; // Return true if the memory buffer should be deleted immediately after // LoadModuleUsingMemoryBuffer(). Return false if the memory buffer has to be @@ -83,6 +86,9 @@ class SourceLineResolverInterface { // Returns true if the module has been loaded. virtual bool HasModule(const CodeModule *module) = 0; + // Returns true if the module has been loaded and it is corrupt. + virtual bool IsModuleCorrupt(const CodeModule *module) = 0; + // Fills in the function_base, function_name, source_file_name, // and source_line fields of the StackFrame. The instruction and // module_name fields must already be filled in. diff --git a/src/google_breakpad/processor/stack_frame_symbolizer.h b/src/google_breakpad/processor/stack_frame_symbolizer.h index 6f89167e..074907cb 100644 --- a/src/google_breakpad/processor/stack_frame_symbolizer.h +++ b/src/google_breakpad/processor/stack_frame_symbolizer.h @@ -62,7 +62,10 @@ class StackFrameSymbolizer { kError, // This indicates error for which stack walk should be interrupted // and retried in future. - kInterrupt + kInterrupt, + // Symbol data was found and loaded in resolver however some corruptions + // were detected. + kWarningCorruptSymbols, }; StackFrameSymbolizer(SymbolSupplier* supplier, diff --git a/src/google_breakpad/processor/stackwalker.h b/src/google_breakpad/processor/stackwalker.h index 11039d6e..9229237c 100644 --- a/src/google_breakpad/processor/stackwalker.h +++ b/src/google_breakpad/processor/stackwalker.h @@ -68,15 +68,18 @@ class Stackwalker { // GetCallerFrame. The frames are further processed to fill all available // data. Returns true if the stackwalk completed, or false if it was // interrupted by SymbolSupplier::GetSymbolFile(). - // Upon return, modules_without_symbols will be populated with pointers to + // Upon return, |modules_without_symbols| will be populated with pointers to // the code modules (CodeModule*) that DON'T have symbols. - // modules_without_symbols DOES NOT take ownership of the code modules. + // |modules_with_corrupt_symbols| will be populated with pointers to the + // modules which have corrupt symbols. |modules_without_symbols| and + // |modules_with_corrupt_symbols| DO NOT take ownership of the code modules. // The lifetime of these code modules is the same as the lifetime of the // CodeModules passed to the StackWalker constructor (which currently // happens to be the lifetime of the Breakpad's ProcessingState object). // There is a check for duplicate modules so no duplicates are expected. bool Walk(CallStack* stack, - vector<const CodeModule*>* modules_without_symbols); + vector<const CodeModule*>* modules_without_symbols, + vector<const CodeModule*>* modules_with_corrupt_symbols); // Returns a new concrete subclass suitable for the CPU that a stack was // generated on, according to the CPU type indicated by the context diff --git a/src/google_breakpad/processor/symbol_supplier.h b/src/google_breakpad/processor/symbol_supplier.h index dccab509..a042081f 100644 --- a/src/google_breakpad/processor/symbol_supplier.h +++ b/src/google_breakpad/processor/symbol_supplier.h @@ -87,7 +87,8 @@ class SymbolSupplier { virtual SymbolResult GetCStringSymbolData(const CodeModule *module, const SystemInfo *system_info, string *symbol_file, - char **symbol_data) = 0; + char **symbol_data, + size_t *symbol_data_size) = 0; // Frees the data buffer allocated for the module in GetCStringSymbolData. virtual void FreeSymbolData(const CodeModule *module) = 0; diff --git a/src/processor/basic_source_line_resolver.cc b/src/processor/basic_source_line_resolver.cc index 55f8f9a1..6fb47b3b 100644 --- a/src/processor/basic_source_line_resolver.cc +++ b/src/processor/basic_source_line_resolver.cc @@ -60,25 +60,65 @@ namespace google_breakpad { #endif static const char *kWhitespace = " \r\n"; +static const int kMaxErrorsPrinted = 5; +static const int kMaxErrorsBeforeBailing = 100; BasicSourceLineResolver::BasicSourceLineResolver() : SourceLineResolverBase(new BasicModuleFactory) { } -bool BasicSourceLineResolver::Module::LoadMapFromMemory(char *memory_buffer) { +// static +void BasicSourceLineResolver::Module::LogParseError( + const string &message, + int line_number, + int *num_errors) { + if (++(*num_errors) <= kMaxErrorsPrinted) { + if (line_number > 0) { + BPLOG(ERROR) << "Line " << line_number << ": " << message; + } else { + BPLOG(ERROR) << message; + } + } +} + +bool BasicSourceLineResolver::Module::LoadMapFromMemory( + char *memory_buffer, + size_t memory_buffer_size) { linked_ptr<Function> cur_func; int line_number = 0; + int num_errors = 0; char *save_ptr; - size_t map_buffer_length = strlen(memory_buffer); // If the length is 0, we can still pretend we have a symbol file. This is // for scenarios that want to test symbol lookup, but don't necessarily care // if certain modules do not have any information, like system libraries. - if (map_buffer_length == 0) { + if (memory_buffer_size == 0) { return true; } - if (memory_buffer[map_buffer_length - 1] == '\n') { - memory_buffer[map_buffer_length - 1] = '\0'; + // Make sure the last character is null terminator. + size_t last_null_terminator = memory_buffer_size - 1; + if (memory_buffer[last_null_terminator] != '\0') { + memory_buffer[last_null_terminator] = '\0'; + } + + // Skip any null terminators at the end of the memory buffer, and make sure + // there are no other null terminators in the middle of the memory buffer. + bool has_null_terminator_in_the_middle = false; + while (last_null_terminator > 0 && + memory_buffer[last_null_terminator - 1] == '\0') { + last_null_terminator--; + } + for (size_t i = 0; i < last_null_terminator; i++) { + if (memory_buffer[i] == '\0') { + memory_buffer[i] = '_'; + has_null_terminator_in_the_middle = true; + } + } + if (has_null_terminator_in_the_middle) { + LogParseError( + "Null terminator is not expected in the middle of the symbol data", + line_number, + &num_errors); } char *buffer; @@ -89,35 +129,28 @@ bool BasicSourceLineResolver::Module::LoadMapFromMemory(char *memory_buffer) { if (strncmp(buffer, "FILE ", 5) == 0) { if (!ParseFile(buffer)) { - BPLOG(ERROR) << "ParseFile on buffer failed at " << - ":" << line_number; - return false; + LogParseError("ParseFile on buffer failed", line_number, &num_errors); } } else if (strncmp(buffer, "STACK ", 6) == 0) { if (!ParseStackInfo(buffer)) { - BPLOG(ERROR) << "ParseStackInfo failed at " << - ":" << line_number; - return false; + LogParseError("ParseStackInfo failed", line_number, &num_errors); } } else if (strncmp(buffer, "FUNC ", 5) == 0) { cur_func.reset(ParseFunction(buffer)); if (!cur_func.get()) { - BPLOG(ERROR) << "ParseFunction failed at " << - ":" << line_number; - return false; + LogParseError("ParseFunction failed", line_number, &num_errors); + } else { + // StoreRange will fail if the function has an invalid address or size. + // We'll silently ignore this, the function and any corresponding lines + // will be destroyed when cur_func is released. + functions_.StoreRange(cur_func->address, cur_func->size, cur_func); } - // StoreRange will fail if the function has an invalid address or size. - // We'll silently ignore this, the function and any corresponding lines - // will be destroyed when cur_func is released. - functions_.StoreRange(cur_func->address, cur_func->size, cur_func); } else if (strncmp(buffer, "PUBLIC ", 7) == 0) { // Clear cur_func: public symbols don't contain line number information. cur_func.reset(); if (!ParsePublicSymbol(buffer)) { - BPLOG(ERROR) << "ParsePublicSymbol failed at " << - ":" << line_number; - return false; + LogParseError("ParsePublicSymbol failed", line_number, &num_errors); } } else if (strncmp(buffer, "MODULE ", 7) == 0) { // Ignore these. They're not of any use to BasicSourceLineResolver, @@ -132,21 +165,24 @@ bool BasicSourceLineResolver::Module::LoadMapFromMemory(char *memory_buffer) { // INFO CODE_ID <code id> <filename> } else { if (!cur_func.get()) { - BPLOG(ERROR) << "Found source line data without a function at " << - ":" << line_number; - return false; - } - Line *line = ParseLine(buffer); - if (!line) { - BPLOG(ERROR) << "ParseLine failed at " << line_number << " for " << - buffer; - return false; + LogParseError("Found source line data without a function", + line_number, &num_errors); + } else { + Line *line = ParseLine(buffer); + if (!line) { + LogParseError("ParseLine failed", line_number, &num_errors); + } else { + cur_func->lines.StoreRange(line->address, line->size, + linked_ptr<Line>(line)); + } } - cur_func->lines.StoreRange(line->address, line->size, - linked_ptr<Line>(line)); + } + if (num_errors > kMaxErrorsBeforeBailing) { + break; } buffer = strtok_r(NULL, "\r\n", &save_ptr); } + is_corrupt_ = num_errors > 0; return true; } diff --git a/src/processor/basic_source_line_resolver_types.h b/src/processor/basic_source_line_resolver_types.h index 94616dcb..a022bc0d 100644 --- a/src/processor/basic_source_line_resolver_types.h +++ b/src/processor/basic_source_line_resolver_types.h @@ -73,12 +73,20 @@ BasicSourceLineResolver::Function : public SourceLineResolverBase::Function { class BasicSourceLineResolver::Module : public SourceLineResolverBase::Module { public: - explicit Module(const string &name) : name_(name) { } + explicit Module(const string &name) : name_(name), is_corrupt_(false) { } virtual ~Module() { } // Loads a map from the given buffer in char* type. // Does NOT have ownership of memory_buffer. - virtual bool LoadMapFromMemory(char *memory_buffer); + // The passed in |memory buffer| is of size |memory_buffer_size|. If it is + // not null terminated, LoadMapFromMemory() will null terminate it by + // modifying the passed in buffer. + virtual bool LoadMapFromMemory(char *memory_buffer, + size_t memory_buffer_size); + + // Tells whether the loaded symbol data is corrupt. Return value is + // undefined, if the symbol data hasn't been loaded yet. + virtual bool IsCorrupt() const { return is_corrupt_; } // Looks up the given relative address, and fills the StackFrame struct // with the result. @@ -105,6 +113,13 @@ class BasicSourceLineResolver::Module : public SourceLineResolverBase::Module { typedef std::map<int, string> FileMap; + // Logs parse errors. |*num_errors| is increased every time LogParseError is + // called. + static void LogParseError( + const string &message, + int line_number, + int *num_errors); + // Parses a file declaration bool ParseFile(char *file_line); @@ -129,6 +144,7 @@ class BasicSourceLineResolver::Module : public SourceLineResolverBase::Module { FileMap files_; RangeMap< MemAddr, linked_ptr<Function> > functions_; AddressMap< MemAddr, linked_ptr<PublicSymbol> > public_symbols_; + bool is_corrupt_; // Each element in the array is a ContainedRangeMap for a type // listed in WindowsFrameInfoTypes. These are split by type because diff --git a/src/processor/basic_source_line_resolver_unittest.cc b/src/processor/basic_source_line_resolver_unittest.cc index 8525e9e4..1a760bbf 100644 --- a/src/processor/basic_source_line_resolver_unittest.cc +++ b/src/processor/basic_source_line_resolver_unittest.cc @@ -376,13 +376,15 @@ TEST_F(TestBasicSourceLineResolver, TestLoadAndResolve) TEST_F(TestBasicSourceLineResolver, TestInvalidLoads) { TestCodeModule module3("module3"); - ASSERT_FALSE(resolver.LoadModule(&module3, + ASSERT_TRUE(resolver.LoadModule(&module3, testdata_dir + "/module3_bad.out")); - ASSERT_FALSE(resolver.HasModule(&module3)); + ASSERT_TRUE(resolver.HasModule(&module3)); + ASSERT_TRUE(resolver.IsModuleCorrupt(&module3)); TestCodeModule module4("module4"); - ASSERT_FALSE(resolver.LoadModule(&module4, + ASSERT_TRUE(resolver.LoadModule(&module4, testdata_dir + "/module4_bad.out")); - ASSERT_FALSE(resolver.HasModule(&module4)); + ASSERT_TRUE(resolver.HasModule(&module4)); + ASSERT_TRUE(resolver.IsModuleCorrupt(&module4)); TestCodeModule module5("module5"); ASSERT_FALSE(resolver.LoadModule(&module5, testdata_dir + "/invalid-filename")); diff --git a/src/processor/exploitability_unittest.cc b/src/processor/exploitability_unittest.cc index 1aeaa54a..ceb4ee85 100644 --- a/src/processor/exploitability_unittest.cc +++ b/src/processor/exploitability_unittest.cc @@ -86,7 +86,8 @@ class TestSymbolSupplier : public SymbolSupplier { virtual SymbolResult GetCStringSymbolData(const CodeModule *module, const SystemInfo *system_info, string *symbol_file, - char **symbol_data); + char **symbol_data, + size_t *symbol_data_size); virtual void FreeSymbolData(const CodeModule *module) { } // When set to true, causes the SymbolSupplier to return INTERRUPT @@ -112,7 +113,8 @@ SymbolSupplier::SymbolResult TestSymbolSupplier::GetCStringSymbolData( const CodeModule *module, const SystemInfo *system_info, string *symbol_file, - char **symbol_data) { + char **symbol_data, + size_t *symbol_data_size) { return GetSymbolFile(module, system_info, symbol_file); } diff --git a/src/processor/fast_source_line_resolver.cc b/src/processor/fast_source_line_resolver.cc index 86073d2d..4a3d0007 100644 --- a/src/processor/fast_source_line_resolver.cc +++ b/src/processor/fast_source_line_resolver.cc @@ -47,6 +47,7 @@ #include "common/scoped_ptr.h" #include "common/using_std_string.h" #include "processor/module_factory.h" +#include "processor/simple_serializer-inl.h" using std::map; using std::make_pair; @@ -143,8 +144,14 @@ WindowsFrameInfo FastSourceLineResolver::CopyWFI(const char *raw) { // Loads a map from the given buffer in char* type. // Does NOT take ownership of mem_buffer. // In addition, treat mem_buffer as const char*. -bool FastSourceLineResolver::Module::LoadMapFromMemory(char *mem_buffer) { - if (!mem_buffer) return false; +bool FastSourceLineResolver::Module::LoadMapFromMemory( + char *memory_buffer, + size_t memory_buffer_size) { + if (!memory_buffer) return false; + + // Read the "is_corrupt" flag. + const char *mem_buffer = memory_buffer; + mem_buffer = SimpleSerializer<bool>::Read(mem_buffer, &is_corrupt_); const uint32_t *map_sizes = reinterpret_cast<const uint32_t*>(mem_buffer); diff --git a/src/processor/fast_source_line_resolver_types.h b/src/processor/fast_source_line_resolver_types.h index c4cec60f..2c010470 100644 --- a/src/processor/fast_source_line_resolver_types.h +++ b/src/processor/fast_source_line_resolver_types.h @@ -112,7 +112,7 @@ public SourceLineResolverBase::PublicSymbol { class FastSourceLineResolver::Module: public SourceLineResolverBase::Module { public: - explicit Module(const string &name) : name_(name) { } + explicit Module(const string &name) : name_(name), is_corrupt_(false) { } virtual ~Module() { } // Looks up the given relative address, and fills the StackFrame struct @@ -120,7 +120,12 @@ class FastSourceLineResolver::Module: public SourceLineResolverBase::Module { virtual void LookupAddress(StackFrame *frame) const; // Loads a map from the given buffer in char* type. - virtual bool LoadMapFromMemory(char *memory_buffer); + virtual bool LoadMapFromMemory(char *memory_buffer, + size_t memory_buffer_size); + + // Tells whether the loaded symbol data is corrupt. Return value is + // undefined, if the symbol data hasn't been loaded yet. + virtual bool IsCorrupt() const { return is_corrupt_; } // If Windows stack walking information is available covering ADDRESS, // return a WindowsFrameInfo structure describing it. If the information @@ -147,6 +152,7 @@ class FastSourceLineResolver::Module: public SourceLineResolverBase::Module { StaticMap<int, char> files_; StaticRangeMap<MemAddr, Function> functions_; StaticAddressMap<MemAddr, PublicSymbol> public_symbols_; + bool is_corrupt_; // Each element in the array is a ContainedRangeMap for a type // listed in WindowsFrameInfoTypes. These are split by type because diff --git a/src/processor/fast_source_line_resolver_unittest.cc b/src/processor/fast_source_line_resolver_unittest.cc index 26982bfc..7998fef1 100644 --- a/src/processor/fast_source_line_resolver_unittest.cc +++ b/src/processor/fast_source_line_resolver_unittest.cc @@ -403,28 +403,32 @@ TEST_F(TestFastSourceLineResolver, TestLoadAndResolve) { TEST_F(TestFastSourceLineResolver, TestInvalidLoads) { TestCodeModule module3("module3"); - ASSERT_FALSE(basic_resolver.LoadModule(&module3, - testdata_dir + "/module3_bad.out")); - ASSERT_FALSE(basic_resolver.HasModule(&module3)); + ASSERT_TRUE(basic_resolver.LoadModule(&module3, + testdata_dir + "/module3_bad.out")); + ASSERT_TRUE(basic_resolver.HasModule(&module3)); + ASSERT_TRUE(basic_resolver.IsModuleCorrupt(&module3)); // Convert module3 to fast_module: - ASSERT_FALSE(serializer.ConvertOneModule(module3.code_file(), - &basic_resolver, - &fast_resolver)); - ASSERT_FALSE(fast_resolver.HasModule(&module3)); + ASSERT_TRUE(serializer.ConvertOneModule(module3.code_file(), + &basic_resolver, + &fast_resolver)); + ASSERT_TRUE(fast_resolver.HasModule(&module3)); + ASSERT_TRUE(fast_resolver.IsModuleCorrupt(&module3)); TestCodeModule module4("module4"); - ASSERT_FALSE(basic_resolver.LoadModule(&module4, - testdata_dir + "/module4_bad.out")); - ASSERT_FALSE(basic_resolver.HasModule(&module4)); + ASSERT_TRUE(basic_resolver.LoadModule(&module4, + testdata_dir + "/module4_bad.out")); + ASSERT_TRUE(basic_resolver.HasModule(&module4)); + ASSERT_TRUE(basic_resolver.IsModuleCorrupt(&module4)); // Convert module4 to fast_module: - ASSERT_FALSE(serializer.ConvertOneModule(module4.code_file(), - &basic_resolver, - &fast_resolver)); - ASSERT_FALSE(fast_resolver.HasModule(&module4)); + ASSERT_TRUE(serializer.ConvertOneModule(module4.code_file(), + &basic_resolver, + &fast_resolver)); + ASSERT_TRUE(fast_resolver.HasModule(&module4)); + ASSERT_TRUE(fast_resolver.IsModuleCorrupt(&module4)); TestCodeModule module5("module5"); ASSERT_FALSE(fast_resolver.LoadModule(&module5, - testdata_dir + "/invalid-filename")); + testdata_dir + "/invalid-filename")); ASSERT_FALSE(fast_resolver.HasModule(&module5)); TestCodeModule invalidmodule("invalid-module"); @@ -457,6 +461,7 @@ TEST_F(TestFastSourceLineResolver, TestUnload) { TEST_F(TestFastSourceLineResolver, CompareModule) { char *symbol_data; + size_t symbol_data_size; string symbol_data_string; string filename; @@ -465,8 +470,8 @@ TEST_F(TestFastSourceLineResolver, CompareModule) { ss << testdata_dir << "/module" << module_index << ".out"; filename = ss.str(); ASSERT_TRUE(SourceLineResolverBase::ReadSymbolFile( - &symbol_data, symbol_file(module_index))); - symbol_data_string = symbol_data; + symbol_file(module_index), &symbol_data, &symbol_data_size)); + symbol_data_string.assign(symbol_data, symbol_data_size); delete [] symbol_data; ASSERT_TRUE(module_comparer.Compare(symbol_data_string)); } diff --git a/src/processor/minidump_processor.cc b/src/processor/minidump_processor.cc index a8742cd2..51ee1164 100644 --- a/src/processor/minidump_processor.cc +++ b/src/processor/minidump_processor.cc @@ -247,7 +247,8 @@ ProcessResult MinidumpProcessor::Process( scoped_ptr<CallStack> stack(new CallStack()); if (stackwalker.get()) { if (!stackwalker->Walk(stack.get(), - &process_state->modules_without_symbols_)) { + &process_state->modules_without_symbols_, + &process_state->modules_with_corrupt_symbols_)) { BPLOG(INFO) << "Stackwalker interrupt (missing symbols?) at " << thread_string; interrupted = true; diff --git a/src/processor/minidump_processor_unittest.cc b/src/processor/minidump_processor_unittest.cc index d0d8b780..9395cc2d 100644 --- a/src/processor/minidump_processor_unittest.cc +++ b/src/processor/minidump_processor_unittest.cc @@ -188,7 +188,8 @@ class TestSymbolSupplier : public SymbolSupplier { virtual SymbolResult GetCStringSymbolData(const CodeModule *module, const SystemInfo *system_info, string *symbol_file, - char **symbol_data); + char **symbol_data, + size_t *symbol_data_size); virtual void FreeSymbolData(const CodeModule *module); @@ -248,21 +249,23 @@ SymbolSupplier::SymbolResult TestSymbolSupplier::GetCStringSymbolData( const CodeModule *module, const SystemInfo *system_info, string *symbol_file, - char **symbol_data) { + char **symbol_data, + size_t *symbol_data_size) { string symbol_data_string; SymbolSupplier::SymbolResult s = GetSymbolFile(module, system_info, symbol_file, &symbol_data_string); if (s == FOUND) { - unsigned int size = symbol_data_string.size() + 1; - *symbol_data = new char[size]; + *symbol_data_size = symbol_data_string.size() + 1; + *symbol_data = new char[*symbol_data_size]; if (*symbol_data == NULL) { BPLOG(ERROR) << "Memory allocation failed for module: " - << module->code_file() << " size: " << size; + << module->code_file() << " size: " << *symbol_data_size; return INTERRUPT; } - strcpy(*symbol_data, symbol_data_string.c_str()); + memcpy(*symbol_data, symbol_data_string.c_str(), symbol_data_string.size()); + (*symbol_data)[symbol_data_string.size()] = '\0'; memory_buffers_.insert(make_pair(module->code_file(), *symbol_data)); } @@ -348,11 +351,11 @@ TEST_F(MinidumpProcessorTest, TestSymbolSupplierLookupCounts) { EXPECT_CALL(supplier, GetCStringSymbolData( Property(&google_breakpad::CodeModule::code_file, "c:\\test_app.exe"), - _, _, _)).WillOnce(Return(SymbolSupplier::NOT_FOUND)); + _, _, _, _)).WillOnce(Return(SymbolSupplier::NOT_FOUND)); EXPECT_CALL(supplier, GetCStringSymbolData( Property(&google_breakpad::CodeModule::code_file, Ne("c:\\test_app.exe")), - _, _, _)).WillRepeatedly(Return(SymbolSupplier::NOT_FOUND)); + _, _, _, _)).WillRepeatedly(Return(SymbolSupplier::NOT_FOUND)); // Avoid GMOCK WARNING "Uninteresting mock function call - returning // directly" for FreeSymbolData(). EXPECT_CALL(supplier, FreeSymbolData(_)).Times(AnyNumber()); @@ -366,11 +369,11 @@ TEST_F(MinidumpProcessorTest, TestSymbolSupplierLookupCounts) { EXPECT_CALL(supplier, GetCStringSymbolData( Property(&google_breakpad::CodeModule::code_file, "c:\\test_app.exe"), - _, _, _)).WillOnce(Return(SymbolSupplier::NOT_FOUND)); + _, _, _, _)).WillOnce(Return(SymbolSupplier::NOT_FOUND)); EXPECT_CALL(supplier, GetCStringSymbolData( Property(&google_breakpad::CodeModule::code_file, Ne("c:\\test_app.exe")), - _, _, _)).WillRepeatedly(Return(SymbolSupplier::NOT_FOUND)); + _, _, _, _)).WillRepeatedly(Return(SymbolSupplier::NOT_FOUND)); // Avoid GMOCK WARNING "Uninteresting mock function call - returning // directly" for FreeSymbolData(). EXPECT_CALL(supplier, FreeSymbolData(_)).Times(AnyNumber()); diff --git a/src/processor/minidump_stackwalk.cc b/src/processor/minidump_stackwalk.cc index 8bcf0dc1..756d8d0b 100644 --- a/src/processor/minidump_stackwalk.cc +++ b/src/processor/minidump_stackwalk.cc @@ -358,10 +358,15 @@ static bool ContainsModule( static void PrintModule( const CodeModule *module, const vector<const CodeModule*> *modules_without_symbols, + const vector<const CodeModule*> *modules_with_corrupt_symbols, uint64_t main_address) { - string missing_symbols; + string symbol_issues; if (ContainsModule(modules_without_symbols, module)) { - missing_symbols = " (WARNING: No symbols, " + + symbol_issues = " (WARNING: No symbols, " + + PathnameStripper::File(module->debug_file()) + ", " + + module->debug_identifier() + ")"; + } else if (ContainsModule(modules_with_corrupt_symbols, module)) { + symbol_issues = " (WARNING: Corrupt symbols, " + PathnameStripper::File(module->debug_file()) + ", " + module->debug_identifier() + ")"; } @@ -371,7 +376,7 @@ static void PrintModule( PathnameStripper::File(module->code_file()).c_str(), module->version().empty() ? "???" : module->version().c_str(), main_address != 0 && base_address == main_address ? " (main)" : "", - missing_symbols.c_str()); + symbol_issues.c_str()); } // PrintModules prints the list of all loaded |modules| to stdout. @@ -379,7 +384,8 @@ static void PrintModule( // confirmed to be missing their symbols during the stack walk. static void PrintModules( const CodeModules *modules, - const vector<const CodeModule*> *modules_without_symbols) { + const vector<const CodeModule*> *modules_without_symbols, + const vector<const CodeModule*> *modules_with_corrupt_symbols) { if (!modules) return; @@ -397,7 +403,8 @@ static void PrintModules( module_sequence < module_count; ++module_sequence) { const CodeModule *module = modules->GetModuleAtSequence(module_sequence); - PrintModule(module, modules_without_symbols, main_address); + PrintModule(module, modules_without_symbols, modules_with_corrupt_symbols, + main_address); } } @@ -490,7 +497,8 @@ static void PrintProcessState(const ProcessState& process_state) { } PrintModules(process_state.modules(), - process_state.modules_without_symbols()); + process_state.modules_without_symbols(), + process_state.modules_with_corrupt_symbols()); } static void PrintProcessStateMachineReadable(const ProcessState& process_state) diff --git a/src/processor/module_comparer.cc b/src/processor/module_comparer.cc index ba561eea..025ab883 100644 --- a/src/processor/module_comparer.cc +++ b/src/processor/module_comparer.cc @@ -58,8 +58,10 @@ bool ModuleComparer::Compare(const string &symbol_data) { // Load symbol data into basic_module scoped_array<char> buffer(new char[symbol_data.size() + 1]); - strcpy(buffer.get(), symbol_data.c_str()); - ASSERT_TRUE(basic_module->LoadMapFromMemory(buffer.get())); + memcpy(buffer.get(), symbol_data.c_str(), symbol_data.size()); + buffer.get()[symbol_data.size()] = '\0'; + ASSERT_TRUE(basic_module->LoadMapFromMemory(buffer.get(), + symbol_data.size() + 1)); buffer.reset(); // Serialize BasicSourceLineResolver::Module. @@ -70,7 +72,9 @@ bool ModuleComparer::Compare(const string &symbol_data) { BPLOG(INFO) << "Serialized size = " << serialized_size << " Bytes"; // Load FastSourceLineResolver::Module using serialized data. - ASSERT_TRUE(fast_module->LoadMapFromMemory(serialized_data.get())); + ASSERT_TRUE(fast_module->LoadMapFromMemory(serialized_data.get(), + serialized_size)); + ASSERT_TRUE(fast_module->IsCorrupt() == basic_module->IsCorrupt()); // Compare FastSourceLineResolver::Module with // BasicSourceLineResolver::Module. diff --git a/src/processor/module_serializer.cc b/src/processor/module_serializer.cc index cf64a7ef..6ac60c1f 100644 --- a/src/processor/module_serializer.cc +++ b/src/processor/module_serializer.cc @@ -51,6 +51,9 @@ SimpleSerializer<BasicSourceLineResolver::Function>::range_map_serializer_; size_t ModuleSerializer::SizeOf(const BasicSourceLineResolver::Module &module) { size_t total_size_alloc_ = 0; + // Size of the "is_corrupt" flag. + total_size_alloc_ += SimpleSerializer<bool>::SizeOf(module.is_corrupt_); + // Compute memory size for each map component in Module class. int map_index = 0; map_sizes_[map_index++] = files_serializer_.SizeOf(module.files_); @@ -65,19 +68,22 @@ size_t ModuleSerializer::SizeOf(const BasicSourceLineResolver::Module &module) { module.cfi_delta_rules_); // Header size. - total_size_alloc_ = kNumberMaps_ * sizeof(uint32_t); + total_size_alloc_ += kNumberMaps_ * sizeof(uint32_t); - for (int i = 0; i < kNumberMaps_; ++i) - total_size_alloc_ += map_sizes_[i]; + for (int i = 0; i < kNumberMaps_; ++i) { + total_size_alloc_ += map_sizes_[i]; + } // Extra one byte for null terminator for C-string copy safety. - ++total_size_alloc_; + total_size_alloc_ += SimpleSerializer<char>::SizeOf(0); return total_size_alloc_; } char *ModuleSerializer::Write(const BasicSourceLineResolver::Module &module, char *dest) { + // Write the is_corrupt flag. + dest = SimpleSerializer<bool>::Write(module.is_corrupt_, dest); // Write header. memcpy(dest, map_sizes_, kNumberMaps_ * sizeof(uint32_t)); dest += kNumberMaps_ * sizeof(uint32_t); @@ -189,8 +195,9 @@ char* ModuleSerializer::SerializeSymbolFileData( scoped_ptr<BasicSourceLineResolver::Module> module( new BasicSourceLineResolver::Module("no name")); scoped_array<char> buffer(new char[symbol_data.size() + 1]); - strcpy(buffer.get(), symbol_data.c_str()); - if (!module->LoadMapFromMemory(buffer.get())) { + memcpy(buffer.get(), symbol_data.c_str(), symbol_data.size()); + buffer.get()[symbol_data.size()] = '\0'; + if (!module->LoadMapFromMemory(buffer.get(), symbol_data.size() + 1)) { return NULL; } buffer.reset(NULL); diff --git a/src/processor/process_state.cc b/src/processor/process_state.cc index 6c3a6567..fae66776 100644 --- a/src/processor/process_state.cc +++ b/src/processor/process_state.cc @@ -57,9 +57,10 @@ void ProcessState::Clear() { } threads_.clear(); system_info_.Clear(); - // modules_without_symbols_ DOES NOT owns the underlying CodeModule pointers. - // Just clear the vector. + // modules_without_symbols_ and modules_with_corrupt_symbols_ DO NOT own + // the underlying CodeModule pointers. Just clear the vectors. modules_without_symbols_.clear(); + modules_with_corrupt_symbols_.clear(); delete modules_; modules_ = NULL; } diff --git a/src/processor/simple_serializer-inl.h b/src/processor/simple_serializer-inl.h index 6e5fe5d1..606bb3ce 100644 --- a/src/processor/simple_serializer-inl.h +++ b/src/processor/simple_serializer-inl.h @@ -60,6 +60,11 @@ class SimpleSerializer<bool> { *dest = static_cast<char>(boolean? 255 : 0); return ++dest; } + + static const char *Read(const char *source, bool *value) { + *value = ((*source) == 0 ? false : true); + return ++source; + } }; // Specializations of SimpleSerializer: string diff --git a/src/processor/simple_symbol_supplier.cc b/src/processor/simple_symbol_supplier.cc index 4a3a1059..bc5ebb68 100644 --- a/src/processor/simple_symbol_supplier.cc +++ b/src/processor/simple_symbol_supplier.cc @@ -84,8 +84,8 @@ SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFile( assert(symbol_data); symbol_data->clear(); - SymbolSupplier::SymbolResult s = GetSymbolFile(module, system_info, symbol_file); - + SymbolSupplier::SymbolResult s = GetSymbolFile(module, system_info, + symbol_file); if (s == FOUND) { std::ifstream in(symbol_file->c_str()); std::getline(in, *symbol_data, string::traits_type::to_char_type( @@ -99,22 +99,25 @@ SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetCStringSymbolData( const CodeModule *module, const SystemInfo *system_info, string *symbol_file, - char **symbol_data) { + char **symbol_data, + size_t *symbol_data_size) { assert(symbol_data); + assert(symbol_data_size); string symbol_data_string; SymbolSupplier::SymbolResult s = GetSymbolFile(module, system_info, symbol_file, &symbol_data_string); if (s == FOUND) { - unsigned int size = symbol_data_string.size() + 1; - *symbol_data = new char[size]; + *symbol_data_size = symbol_data_string.size() + 1; + *symbol_data = new char[*symbol_data_size]; if (*symbol_data == NULL) { - BPLOG(ERROR) << "Memory allocation for size " << size << " failed"; + BPLOG(ERROR) << "Memory allocation for size " << *symbol_data_size + << " failed"; return INTERRUPT; } - memcpy(*symbol_data, symbol_data_string.c_str(), size - 1); - (*symbol_data)[size - 1] = '\0'; + memcpy(*symbol_data, symbol_data_string.c_str(), symbol_data_string.size()); + (*symbol_data)[symbol_data_string.size()] = '\0'; memory_buffers_.insert(make_pair(module->code_file(), *symbol_data)); } return s; diff --git a/src/processor/simple_symbol_supplier.h b/src/processor/simple_symbol_supplier.h index e19b194c..0cde85cd 100644 --- a/src/processor/simple_symbol_supplier.h +++ b/src/processor/simple_symbol_supplier.h @@ -118,7 +118,8 @@ class SimpleSymbolSupplier : public SymbolSupplier { virtual SymbolResult GetCStringSymbolData(const CodeModule *module, const SystemInfo *system_info, string *symbol_file, - char **symbol_data); + char **symbol_data, + size_t *symbol_data_size); // Free the data buffer allocated in the above GetCStringSymbolData(); virtual void FreeSymbolData(const CodeModule *module); diff --git a/src/processor/source_line_resolver_base.cc b/src/processor/source_line_resolver_base.cc index 49088c89..6eff1f99 100644 --- a/src/processor/source_line_resolver_base.cc +++ b/src/processor/source_line_resolver_base.cc @@ -53,6 +53,7 @@ namespace google_breakpad { SourceLineResolverBase::SourceLineResolverBase( ModuleFactory *module_factory) : modules_(new ModuleMap), + corrupt_modules_(new ModuleSet), memory_buffers_(new MemoryMap), module_factory_(module_factory) { } @@ -66,6 +67,11 @@ SourceLineResolverBase::~SourceLineResolverBase() { } // Delete the map of modules. delete modules_; + modules_ = NULL; + + // Delete the set of corrupt modules. + delete corrupt_modules_; + corrupt_modules_ = NULL; MemoryMap::iterator iter = memory_buffers_->begin(); for (; iter != memory_buffers_->end(); ++iter) { @@ -73,13 +79,16 @@ SourceLineResolverBase::~SourceLineResolverBase() { } // Delete the map of memory buffers. delete memory_buffers_; + memory_buffers_ = NULL; delete module_factory_; + module_factory_ = NULL; } -bool SourceLineResolverBase::ReadSymbolFile(char **symbol_data, - const string &map_file) { - if (symbol_data == NULL) { +bool SourceLineResolverBase::ReadSymbolFile(const string &map_file, + char **symbol_data, + size_t *symbol_data_size) { + if (symbol_data == NULL || symbol_data_size == NULL) { BPLOG(ERROR) << "Could not Read file into Null memory pointer"; return false; } @@ -98,6 +107,7 @@ bool SourceLineResolverBase::ReadSymbolFile(char **symbol_data, // Allocate memory for file contents, plus a null terminator // since we may use strtok() on the contents. + *symbol_data_size = file_size + 1; *symbol_data = new char[file_size + 1]; if (*symbol_data == NULL) { @@ -154,12 +164,14 @@ bool SourceLineResolverBase::LoadModule(const CodeModule *module, << " from " << map_file; char *memory_buffer; - if (!ReadSymbolFile(&memory_buffer, map_file)) + size_t memory_buffer_size; + if (!ReadSymbolFile(map_file, &memory_buffer, &memory_buffer_size)) return false; BPLOG(INFO) << "Read symbol file " << map_file << " succeeded"; - bool load_result = LoadModuleUsingMemoryBuffer(module, memory_buffer); + bool load_result = LoadModuleUsingMemoryBuffer(module, memory_buffer, + memory_buffer_size); if (load_result && !ShouldDeleteMemoryBufferAfterLoadModule()) { // memory_buffer has to stay alive as long as the module. @@ -183,7 +195,8 @@ bool SourceLineResolverBase::LoadModuleUsingMapBuffer( return false; } - char *memory_buffer = new char[map_buffer.size() + 1]; + size_t memory_buffer_size = map_buffer.size() + 1; + char *memory_buffer = new char[memory_buffer_size]; if (memory_buffer == NULL) { BPLOG(ERROR) << "Could not allocate memory for " << module->code_file(); return false; @@ -193,7 +206,8 @@ bool SourceLineResolverBase::LoadModuleUsingMapBuffer( memcpy(memory_buffer, map_buffer.c_str(), map_buffer.size()); memory_buffer[map_buffer.size()] = '\0'; - bool load_result = LoadModuleUsingMemoryBuffer(module, memory_buffer); + bool load_result = LoadModuleUsingMemoryBuffer(module, memory_buffer, + memory_buffer_size); if (load_result && !ShouldDeleteMemoryBufferAfterLoadModule()) { // memory_buffer has to stay alive as long as the module. @@ -206,7 +220,9 @@ bool SourceLineResolverBase::LoadModuleUsingMapBuffer( } bool SourceLineResolverBase::LoadModuleUsingMemoryBuffer( - const CodeModule *module, char *memory_buffer) { + const CodeModule *module, + char *memory_buffer, + size_t memory_buffer_size) { if (!module) return false; @@ -223,12 +239,19 @@ bool SourceLineResolverBase::LoadModuleUsingMemoryBuffer( Module *basic_module = module_factory_->CreateModule(module->code_file()); // Ownership of memory is NOT transfered to Module::LoadMapFromMemory(). - if (!basic_module->LoadMapFromMemory(memory_buffer)) { - delete basic_module; - return false; + if (!basic_module->LoadMapFromMemory(memory_buffer, memory_buffer_size)) { + BPLOG(ERROR) << "Too many error while parsing symbol data for module " + << module->code_file(); + // Returning false from here would be an indication that the symbols for + // this module are missing which would be wrong. Intentionally fall through + // and add the module to both the modules_ and the corrupt_modules_ lists. + assert(basic_module->IsCorrupt()); } modules_->insert(make_pair(module->code_file(), basic_module)); + if (basic_module->IsCorrupt()) { + corrupt_modules_->insert(module->code_file()); + } return true; } @@ -244,6 +267,7 @@ void SourceLineResolverBase::UnloadModule(const CodeModule *code_module) { if (mod_iter != modules_->end()) { Module *symbol_module = mod_iter->second; delete symbol_module; + corrupt_modules_->erase(mod_iter->first); modules_->erase(mod_iter); } @@ -265,6 +289,12 @@ bool SourceLineResolverBase::HasModule(const CodeModule *module) { return modules_->find(module->code_file()) != modules_->end(); } +bool SourceLineResolverBase::IsModuleCorrupt(const CodeModule *module) { + if (!module) + return false; + return corrupt_modules_->find(module->code_file()) != corrupt_modules_->end(); +} + void SourceLineResolverBase::FillSourceLineInfo(StackFrame *frame) { if (frame->module) { ModuleMap::const_iterator it = modules_->find(frame->module->code_file()); diff --git a/src/processor/source_line_resolver_base_types.h b/src/processor/source_line_resolver_base_types.h index 1dc3d62a..4a9dfb3c 100644 --- a/src/processor/source_line_resolver_base_types.h +++ b/src/processor/source_line_resolver_base_types.h @@ -121,7 +121,15 @@ class SourceLineResolverBase::Module { // Loads a map from the given buffer in char* type. // Does NOT take ownership of memory_buffer (the caller, source line resolver, // is the owner of memory_buffer). - virtual bool LoadMapFromMemory(char *memory_buffer) = 0; + // The passed in |memory buffer| is of size |memory_buffer_size|. If it is + // not null terminated, LoadMapFromMemory will null terminate it by modifying + // the passed in buffer. + virtual bool LoadMapFromMemory(char *memory_buffer, + size_t memory_buffer_size) = 0; + + // Tells whether the loaded symbol data is corrupt. Return value is + // undefined, if the symbol data hasn't been loaded yet. + virtual bool IsCorrupt() const = 0; // Looks up the given relative address, and fills the StackFrame struct // with the result. diff --git a/src/processor/stack_frame_symbolizer.cc b/src/processor/stack_frame_symbolizer.cc index 8e020c98..5c8dbe5e 100644 --- a/src/processor/stack_frame_symbolizer.cc +++ b/src/processor/stack_frame_symbolizer.cc @@ -74,7 +74,8 @@ StackFrameSymbolizer::SymbolizerResult StackFrameSymbolizer::FillSourceLineInfo( // If module is already loaded, go ahead to fill source line info and return. if (resolver_->HasModule(frame->module)) { resolver_->FillSourceLineInfo(frame); - return kNoError; + return resolver_->IsModuleCorrupt(frame->module) ? + kWarningCorruptSymbols : kNoError; } // Module needs to fetch symbol file. First check to see if supplier exists. @@ -85,20 +86,24 @@ StackFrameSymbolizer::SymbolizerResult StackFrameSymbolizer::FillSourceLineInfo( // Start fetching symbol from supplier. string symbol_file; char* symbol_data = NULL; + size_t symbol_data_size; SymbolSupplier::SymbolResult symbol_result = supplier_->GetCStringSymbolData( - module, system_info, &symbol_file, &symbol_data); + module, system_info, &symbol_file, &symbol_data, &symbol_data_size); switch (symbol_result) { case SymbolSupplier::FOUND: { - bool load_success = resolver_->LoadModuleUsingMemoryBuffer(frame->module, - symbol_data); + bool load_success = resolver_->LoadModuleUsingMemoryBuffer( + frame->module, + symbol_data, + symbol_data_size); if (resolver_->ShouldDeleteMemoryBufferAfterLoadModule()) { supplier_->FreeSymbolData(module); } if (load_success) { resolver_->FillSourceLineInfo(frame); - return kNoError; + return resolver_->IsModuleCorrupt(frame->module) ? + kWarningCorruptSymbols : kNoError; } else { BPLOG(ERROR) << "Failed to load symbol file in resolver."; no_symbol_modules_.insert(module->code_file()); diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc index fff3392d..00358b26 100644 --- a/src/processor/stackwalker.cc +++ b/src/processor/stackwalker.cc @@ -71,16 +71,46 @@ Stackwalker::Stackwalker(const SystemInfo* system_info, assert(frame_symbolizer_); } +void InsertSpecialAttentionModule( + StackFrameSymbolizer::SymbolizerResult symbolizer_result, + const CodeModule* module, + vector<const CodeModule*>* modules) { + if (!module) { + return; + } + assert(symbolizer_result == StackFrameSymbolizer::kError || + symbolizer_result == StackFrameSymbolizer::kWarningCorruptSymbols); + bool found = false; + vector<const CodeModule*>::iterator iter; + for (iter = modules->begin(); iter != modules->end(); ++iter) { + if (*iter == module) { + found = true; + break; + } + } + if (!found) { + BPLOG(INFO) << ((symbolizer_result == StackFrameSymbolizer::kError) ? + "Couldn't load symbols for: " : + "Detected corrupt symbols for: ") + << module->debug_file() << "|" << module->debug_identifier(); + modules->push_back(module); + } +} -bool Stackwalker::Walk(CallStack* stack, - vector<const CodeModule*>* modules_without_symbols) { +bool Stackwalker::Walk( + CallStack* stack, + vector<const CodeModule*>* modules_without_symbols, + vector<const CodeModule*>* modules_with_corrupt_symbols) { BPLOG_IF(ERROR, !stack) << "Stackwalker::Walk requires |stack|"; assert(stack); stack->Clear(); BPLOG_IF(ERROR, !modules_without_symbols) << "Stackwalker::Walk requires " << "|modules_without_symbols|"; + BPLOG_IF(ERROR, !modules_without_symbols) << "Stackwalker::Walk requires " + << "|modules_with_corrupt_symbols|"; assert(modules_without_symbols); + assert(modules_with_corrupt_symbols); // Begin with the context frame, and keep getting callers until there are // no more. @@ -97,30 +127,24 @@ bool Stackwalker::Walk(CallStack* stack, StackFrameSymbolizer::SymbolizerResult symbolizer_result = frame_symbolizer_->FillSourceLineInfo(modules_, system_info_, frame.get()); - if (symbolizer_result == StackFrameSymbolizer::kInterrupt) { - BPLOG(INFO) << "Stack walk is interrupted."; - return false; - } - - // Keep track of modules that have no symbols. - if (symbolizer_result == StackFrameSymbolizer::kError && - frame->module != NULL) { - bool found = false; - vector<const CodeModule*>::iterator iter; - for (iter = modules_without_symbols->begin(); - iter != modules_without_symbols->end(); - ++iter) { - if (*iter == frame->module) { - found = true; - break; - } - } - if (!found) { - BPLOG(INFO) << "Couldn't load symbols for: " - << frame->module->debug_file() << "|" - << frame->module->debug_identifier(); - modules_without_symbols->push_back(frame->module); - } + switch (symbolizer_result) { + case StackFrameSymbolizer::kInterrupt: + BPLOG(INFO) << "Stack walk is interrupted."; + return false; + break; + case StackFrameSymbolizer::kError: + InsertSpecialAttentionModule(symbolizer_result, frame->module, + modules_without_symbols); + break; + case StackFrameSymbolizer::kWarningCorruptSymbols: + InsertSpecialAttentionModule(symbolizer_result, frame->module, + modules_with_corrupt_symbols); + break; + case StackFrameSymbolizer::kNoError: + break; + default: + assert(false); + break; } // Add the frame to the call stack. Relinquish the ownership claim @@ -222,7 +246,8 @@ bool Stackwalker::InstructionAddressSeemsValid(uint64_t address) { return true; } - if (symbolizer_result != StackFrameSymbolizer::kNoError) { + if (symbolizer_result != StackFrameSymbolizer::kNoError && + symbolizer_result != StackFrameSymbolizer::kWarningCorruptSymbols) { // Some error occurred during symbolization, but the address is within a // known module return true; diff --git a/src/processor/stackwalker_amd64_unittest.cc b/src/processor/stackwalker_amd64_unittest.cc index a0175a9b..169316c6 100644 --- a/src/processor/stackwalker_amd64_unittest.cc +++ b/src/processor/stackwalker_amd64_unittest.cc @@ -89,7 +89,7 @@ class StackwalkerAMD64Fixture { // By default, none of the modules have symbol info; call // SetModuleSymbols to override this. - EXPECT_CALL(supplier, GetCStringSymbolData(_, _, _, _)) + EXPECT_CALL(supplier, GetCStringSymbolData(_, _, _, _, _)) .WillRepeatedly(Return(MockSymbolSupplier::NOT_FOUND)); // Avoid GMOCK WARNING "Uninteresting mock function call - returning @@ -100,9 +100,11 @@ class StackwalkerAMD64Fixture { // Set the Breakpad symbol information that supplier should return for // MODULE to INFO. void SetModuleSymbols(MockCodeModule *module, const string &info) { - char *buffer = supplier.CopySymbolDataAndOwnTheCopy(info); - EXPECT_CALL(supplier, GetCStringSymbolData(module, &system_info, _, _)) + size_t buffer_size; + char *buffer = supplier.CopySymbolDataAndOwnTheCopy(info, &buffer_size); + EXPECT_CALL(supplier, GetCStringSymbolData(module, &system_info, _, _, _)) .WillRepeatedly(DoAll(SetArgumentPointee<3>(buffer), + SetArgumentPointee<4>(buffer_size), Return(MockSymbolSupplier::FOUND))); } @@ -151,9 +153,12 @@ TEST_F(SanityCheck, NoResolver) { &frame_symbolizer); // This should succeed even without a resolver or supplier. vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_GE(1U, frames->size()); StackFrameAMD64 *frame = static_cast<StackFrameAMD64 *>(frames->at(0)); @@ -174,9 +179,12 @@ TEST_F(GetContextFrame, Simple) { StackwalkerAMD64 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_GE(1U, frames->size()); StackFrameAMD64 *frame = static_cast<StackFrameAMD64 *>(frames->at(0)); @@ -195,9 +203,12 @@ TEST_F(GetContextFrame, NoStackMemory) { StackwalkerAMD64 walker(&system_info, &raw_context, NULL, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_GE(1U, frames->size()); StackFrameAMD64 *frame = static_cast<StackFrameAMD64 *>(frames->at(0)); @@ -253,10 +264,13 @@ TEST_F(GetCallerFrame, ScanWithoutSymbols) { StackwalkerAMD64 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(2U, modules_without_symbols.size()); ASSERT_EQ("module1", modules_without_symbols[0]->debug_file()); ASSERT_EQ("module2", modules_without_symbols[1]->debug_file()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(3U, frames->size()); @@ -325,8 +339,11 @@ TEST_F(GetCallerFrame, ScanWithFunctionSymbols) { StackwalkerAMD64 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -392,8 +409,11 @@ TEST_F(GetCallerFrame, CallerPushedRBP) { StackwalkerAMD64 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -470,8 +490,11 @@ struct CFIFixture: public StackwalkerAMD64Fixture { StackwalkerAMD64 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); diff --git a/src/processor/stackwalker_arm_unittest.cc b/src/processor/stackwalker_arm_unittest.cc index 29deac87..b50f4e99 100644 --- a/src/processor/stackwalker_arm_unittest.cc +++ b/src/processor/stackwalker_arm_unittest.cc @@ -91,7 +91,7 @@ class StackwalkerARMFixture { // By default, none of the modules have symbol info; call // SetModuleSymbols to override this. - EXPECT_CALL(supplier, GetCStringSymbolData(_, _, _, _)) + EXPECT_CALL(supplier, GetCStringSymbolData(_, _, _, _, _)) .WillRepeatedly(Return(MockSymbolSupplier::NOT_FOUND)); // Avoid GMOCK WARNING "Uninteresting mock function call - returning @@ -102,9 +102,11 @@ class StackwalkerARMFixture { // Set the Breakpad symbol information that supplier should return for // MODULE to INFO. void SetModuleSymbols(MockCodeModule *module, const string &info) { - char *buffer = supplier.CopySymbolDataAndOwnTheCopy(info); - EXPECT_CALL(supplier, GetCStringSymbolData(module, &system_info, _, _)) + size_t buffer_size; + char *buffer = supplier.CopySymbolDataAndOwnTheCopy(info, &buffer_size); + EXPECT_CALL(supplier, GetCStringSymbolData(module, &system_info, _, _, _)) .WillRepeatedly(DoAll(SetArgumentPointee<3>(buffer), + SetArgumentPointee<4>(buffer_size), Return(MockSymbolSupplier::FOUND))); } @@ -147,8 +149,11 @@ TEST_F(SanityCheck, NoResolver) { &frame_symbolizer); // This should succeed even without a resolver or supplier. vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(1U, frames->size()); StackFrameARM *frame = static_cast<StackFrameARM *>(frames->at(0)); @@ -167,8 +172,11 @@ TEST_F(GetContextFrame, Simple) { StackwalkerARM walker(&system_info, &raw_context, -1, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(1U, frames->size()); StackFrameARM *frame = static_cast<StackFrameARM *>(frames->at(0)); @@ -184,8 +192,11 @@ TEST_F(GetContextFrame, NoStackMemory) { StackwalkerARM walker(&system_info, &raw_context, -1, NULL, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(1U, frames->size()); StackFrameARM *frame = static_cast<StackFrameARM *>(frames->at(0)); @@ -234,10 +245,13 @@ TEST_F(GetCallerFrame, ScanWithoutSymbols) { StackwalkerARM walker(&system_info, &raw_context, -1, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(2U, modules_without_symbols.size()); ASSERT_EQ("module1", modules_without_symbols[0]->debug_file()); ASSERT_EQ("module2", modules_without_symbols[1]->debug_file()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(3U, frames->size()); @@ -302,8 +316,11 @@ TEST_F(GetCallerFrame, ScanWithFunctionSymbols) { StackwalkerARM walker(&system_info, &raw_context, -1, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -365,10 +382,13 @@ TEST_F(GetCallerFrame, ScanFirstFrame) { StackwalkerARM walker(&system_info, &raw_context, -1, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(2U, modules_without_symbols.size()); ASSERT_EQ("module1", modules_without_symbols[0]->debug_file()); ASSERT_EQ("module2", modules_without_symbols[1]->debug_file()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -477,8 +497,11 @@ struct CFIFixture: public StackwalkerARMFixture { &modules, &frame_symbolizer); walker.SetContextFrameValidity(context_frame_validity); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -671,8 +694,11 @@ TEST_F(CFI, RejectBackwards) { StackwalkerARM walker(&system_info, &raw_context, -1, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(1U, frames->size()); } @@ -685,8 +711,11 @@ TEST_F(CFI, RejectBadExpressions) { StackwalkerARM walker(&system_info, &raw_context, -1, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(1U, frames->size()); } @@ -745,10 +774,13 @@ TEST_F(GetFramesByFramePointer, OnlyFramePointer) { &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(2U, modules_without_symbols.size()); ASSERT_EQ("module1", modules_without_symbols[0]->debug_file()); ASSERT_EQ("module2", modules_without_symbols[1]->debug_file()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(3U, frames->size()); @@ -839,9 +871,12 @@ TEST_F(GetFramesByFramePointer, FramePointerAndCFI) { &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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("module2", modules_without_symbols[0]->debug_file()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(3U, frames->size()); diff --git a/src/processor/stackwalker_unittest_utils.h b/src/processor/stackwalker_unittest_utils.h index 07638661..a72b8e0d 100644 --- a/src/processor/stackwalker_unittest_utils.h +++ b/src/processor/stackwalker_unittest_utils.h @@ -171,18 +171,21 @@ class MockSymbolSupplier: public google_breakpad::SymbolSupplier { const SystemInfo *system_info, string *symbol_file, string *symbol_data)); - MOCK_METHOD4(GetCStringSymbolData, SymbolResult(const CodeModule *module, + MOCK_METHOD5(GetCStringSymbolData, SymbolResult(const CodeModule *module, const SystemInfo *system_info, string *symbol_file, - char **symbol_data)); + char **symbol_data, + size_t *symbol_data_size)); MOCK_METHOD1(FreeSymbolData, void(const CodeModule *module)); // Copies the passed string contents into a newly allocated buffer. // The newly allocated buffer will be freed during destruction. - char* CopySymbolDataAndOwnTheCopy(const std::string &info) { - unsigned int buffer_size = info.size() + 1; - char *symbol_data = new char [buffer_size]; - strcpy(symbol_data, info.c_str()); + char* CopySymbolDataAndOwnTheCopy(const std::string &info, + size_t *symbol_data_size) { + *symbol_data_size = info.size() + 1; + char *symbol_data = new char[*symbol_data_size]; + memcpy(symbol_data, info.c_str(), info.size()); + symbol_data[info.size()] = '\0'; symbol_data_to_free_.push_back(symbol_data); return symbol_data; } diff --git a/src/processor/stackwalker_x86_unittest.cc b/src/processor/stackwalker_x86_unittest.cc index d25a570a..03df3210 100644 --- a/src/processor/stackwalker_x86_unittest.cc +++ b/src/processor/stackwalker_x86_unittest.cc @@ -98,7 +98,7 @@ class StackwalkerX86Fixture { // By default, none of the modules have symbol info; call // SetModuleSymbols to override this. - EXPECT_CALL(supplier, GetCStringSymbolData(_, _, _, _)) + EXPECT_CALL(supplier, GetCStringSymbolData(_, _, _, _, _)) .WillRepeatedly(Return(MockSymbolSupplier::NOT_FOUND)); // Avoid GMOCK WARNING "Uninteresting mock function call - returning @@ -109,9 +109,11 @@ class StackwalkerX86Fixture { // Set the Breakpad symbol information that supplier should return for // MODULE to INFO. void SetModuleSymbols(MockCodeModule *module, const string &info) { - char *buffer = supplier.CopySymbolDataAndOwnTheCopy(info); - EXPECT_CALL(supplier, GetCStringSymbolData(module, &system_info, _, _)) + size_t buffer_size; + char *buffer = supplier.CopySymbolDataAndOwnTheCopy(info, &buffer_size); + EXPECT_CALL(supplier, GetCStringSymbolData(module, &system_info, _, _, _)) .WillRepeatedly(DoAll(SetArgumentPointee<3>(buffer), + SetArgumentPointee<4>(buffer_size), Return(MockSymbolSupplier::FOUND))); } @@ -161,9 +163,12 @@ TEST_F(SanityCheck, NoResolver) { &frame_symbolizer); // This should succeed, even without a resolver or supplier. vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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_with_corrupt_symbols.size()); frames = call_stack.frames(); StackFrameX86 *frame = static_cast<StackFrameX86 *>(frames->at(0)); // Check that the values from the original raw context made it @@ -184,9 +189,12 @@ TEST_F(GetContextFrame, Simple) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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_with_corrupt_symbols.size()); frames = call_stack.frames(); StackFrameX86 *frame = static_cast<StackFrameX86 *>(frames->at(0)); // Check that the values from the original raw context made it @@ -204,9 +212,12 @@ TEST_F(GetContextFrame, NoStackMemory) { StackwalkerX86 walker(&system_info, &raw_context, NULL, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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_with_corrupt_symbols.size()); frames = call_stack.frames(); StackFrameX86 *frame = static_cast<StackFrameX86 *>(frames->at(0)); // Check that the values from the original raw context made it @@ -214,7 +225,10 @@ TEST_F(GetContextFrame, NoStackMemory) { EXPECT_EQ(0, memcmp(&raw_context, &frame->context, sizeof(raw_context))); } -class GetCallerFrame: public StackwalkerX86Fixture, public Test { }; +class GetCallerFrame: public StackwalkerX86Fixture, public Test { + protected: + void IPAddressIsNotInKnownModuleTestImpl(bool has_corrupt_symbols); +}; // Walk a traditional frame. A traditional frame saves the caller's // %ebp just below the return address, and has its own %ebp pointing @@ -240,9 +254,12 @@ TEST_F(GetCallerFrame, Traditional) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -301,9 +318,12 @@ TEST_F(GetCallerFrame, TraditionalScan) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -364,9 +384,12 @@ TEST_F(GetCallerFrame, TraditionalScanLongWay) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -435,8 +458,11 @@ TEST_F(GetCallerFrame, WindowsFrameData) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -509,9 +535,12 @@ TEST_F(GetCallerFrame, WindowsFrameDataAligned) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + 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("module2", modules_without_symbols[0]->debug_file()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -595,8 +624,11 @@ TEST_F(GetCallerFrame, WindowsFrameDataParameterSize) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(3U, frames->size()); @@ -692,8 +724,11 @@ TEST_F(GetCallerFrame, WindowsFrameDataScan) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -778,8 +813,11 @@ TEST_F(GetCallerFrame, WindowsFrameDataBadEIPScan) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -848,8 +886,11 @@ TEST_F(GetCallerFrame, WindowsFPOUnchangedEBP) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -928,8 +969,11 @@ TEST_F(GetCallerFrame, WindowsFPOUsedEBP) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); @@ -1068,8 +1112,11 @@ TEST_F(GetCallerFrame, WindowsFPOSystemCall) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(4U, frames->size()); @@ -1278,8 +1325,11 @@ TEST_F(GetCallerFrame, ReturnAddressIsNotInKnownModule) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &local_modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(3U, frames->size()); @@ -1361,14 +1411,16 @@ TEST_F(GetCallerFrame, ReturnAddressIsNotInKnownModule) { // this scan is limited to 120 search words for the context frame and 30 // search words (pointers) for the other frames: // const int kRASearchWords = 30; -TEST_F(GetCallerFrame, IPAddressIsNotInKnownModule) { +void GetCallerFrame::IPAddressIsNotInKnownModuleTestImpl( + bool has_corrupt_symbols) { MockCodeModule remoting_core_dll(0x54080000, 0x501000, "remoting_core.dll", "version1"); - SetModuleSymbols(&remoting_core_dll, // remoting_core.dll - "FUNC 137214 17d 10 PK11_Verify\n" - "FUNC 15c834 37 14 nsc_ECDSAVerifyStub\n" - "FUNC 1611d3 91 14 NSC_Verify\n" - "FUNC 162ff7 60 4 sftk_SessionFromHandle\n" + string symbols_func_section = + "FUNC 137214 17d 10 PK11_Verify\n" + "FUNC 15c834 37 14 nsc_ECDSAVerifyStub\n" + "FUNC 1611d3 91 14 NSC_Verify\n" + "FUNC 162ff7 60 4 sftk_SessionFromHandle\n"; + string symbols_stack_section = "STACK WIN 4 137214 17d 9 0 10 0 10 0 1 $T0 $ebp = " "$eip $T0 4 + ^ = $ebp $T0 ^ = $esp $T0 8 + =\n" "STACK WIN 4 15c834 37 6 0 14 0 18 0 1 $T0 $ebp = " @@ -1376,7 +1428,21 @@ TEST_F(GetCallerFrame, IPAddressIsNotInKnownModule) { "STACK WIN 4 1611d3 91 7 0 14 0 8 0 1 $T0 $ebp = " "$eip $T0 4 + ^ = $ebp $T0 ^ = $esp $T0 8 + =\n" "STACK WIN 4 162ff7 60 5 0 4 0 0 0 1 $T0 $ebp = " - "$eip $T0 4 + ^ = $ebp $T0 ^ = $esp $T0 8 + =\n"); + "$eip $T0 4 + ^ = $ebp $T0 ^ = $esp $T0 8 + =\n"; + + string symbols = symbols_func_section; + if (has_corrupt_symbols) { + symbols.append(string(1, '\0')); // null terminator in the middle + symbols.append("\n"); + symbols.append("FUNC 1234\n" // invalid FUNC records + "FUNNC 1234\n" + "STACK WIN 4 1234 234 23 " // invalid STACK record + "23423423 234 23 234 234 " + "234 23 234 23 234 234 " + "234 234 234\n"); + } + symbols.append(symbols_stack_section); + SetModuleSymbols(&remoting_core_dll, symbols); // Create some modules with some stock debugging information. MockCodeModules local_modules; @@ -1505,8 +1571,17 @@ TEST_F(GetCallerFrame, IPAddressIsNotInKnownModule) { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &local_modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + if (has_corrupt_symbols) { + ASSERT_EQ(1U, modules_with_corrupt_symbols.size()); + ASSERT_EQ("remoting_core.dll", + modules_with_corrupt_symbols[0]->debug_file()); + } else { + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); + } frames = call_stack.frames(); ASSERT_EQ(4U, frames->size()); @@ -1584,6 +1659,16 @@ TEST_F(GetCallerFrame, IPAddressIsNotInKnownModule) { } } +// Runs IPAddressIsNotInKnownModule test with good symbols +TEST_F(GetCallerFrame, IPAddressIsNotInKnownModule) { + IPAddressIsNotInKnownModuleTestImpl(false /* has_corrupt_modules */); +} + +// Runs IPAddressIsNotInKnownModule test with corrupt symbols +TEST_F(GetCallerFrame, IPAddressIsNotInKnownModule_CorruptSymbols) { + IPAddressIsNotInKnownModuleTestImpl(true /* has_corrupt_modules */); +} + struct CFIFixture: public StackwalkerX86Fixture { CFIFixture() { // Provide a bunch of STACK CFI records; individual tests walk to the @@ -1635,8 +1720,11 @@ struct CFIFixture: public StackwalkerX86Fixture { StackwalkerX86 walker(&system_info, &raw_context, &stack_region, &modules, &frame_symbolizer); vector<const CodeModule*> modules_without_symbols; - ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols)); + vector<const CodeModule*> modules_with_corrupt_symbols; + ASSERT_TRUE(walker.Walk(&call_stack, &modules_without_symbols, + &modules_with_corrupt_symbols)); ASSERT_EQ(0U, modules_without_symbols.size()); + ASSERT_EQ(0U, modules_with_corrupt_symbols.size()); frames = call_stack.frames(); ASSERT_EQ(2U, frames->size()); diff --git a/src/tools/mac/crash_report/on_demand_symbol_supplier.h b/src/tools/mac/crash_report/on_demand_symbol_supplier.h index 28002c6b..3fbe108e 100644 --- a/src/tools/mac/crash_report/on_demand_symbol_supplier.h +++ b/src/tools/mac/crash_report/on_demand_symbol_supplier.h @@ -66,7 +66,8 @@ class OnDemandSymbolSupplier : public SymbolSupplier { virtual SymbolResult GetCStringSymbolData(const CodeModule *module, const SystemInfo *system_info, string *symbol_file, - char **symbol_data); + char **symbol_data, + size_t *symbol_data_size); // Delete the data buffer allocated for module in GetCStringSymbolData(). virtual void FreeSymbolData(const CodeModule *module); diff --git a/src/tools/mac/crash_report/on_demand_symbol_supplier.mm b/src/tools/mac/crash_report/on_demand_symbol_supplier.mm index 1bd1f693..93b4c362 100644 --- a/src/tools/mac/crash_report/on_demand_symbol_supplier.mm +++ b/src/tools/mac/crash_report/on_demand_symbol_supplier.mm @@ -164,19 +164,22 @@ SymbolSupplier::SymbolResult OnDemandSymbolSupplier::GetCStringSymbolData(const CodeModule *module, const SystemInfo *system_info, string *symbol_file, - char **symbol_data) { + char **symbol_data, + uint64_t *symbol_data_size) { std::string symbol_data_string; SymbolSupplier::SymbolResult result = GetSymbolFile(module, system_info, symbol_file, &symbol_data_string); if (result == FOUND) { - *symbol_data = new char[symbol_data_string.size() + 1]; + *symbol_data_size = symbol_data_string.size() + 1; + *symbol_data = new char[*symbol_data_size]; if (*symbol_data == NULL) { // Should return INTERRUPT on memory allocation failure. return INTERRUPT; } - strcpy(*symbol_data, symbol_data_string.c_str()); + memcpy(*symbol_data, symbol_data_string.c_str(), symbol_data_string.size()); + (*symbol_data)[symbol_data_string.size()] = '\0'; memory_buffers_.insert(make_pair(module->code_file(), *symbol_data)); } return result; |