From 0fd2f1ae2152782f2127c56fb5302002c95502d3 Mon Sep 17 00:00:00 2001 From: nealsid Date: Thu, 19 Feb 2009 21:26:20 +0000 Subject: Modify symbol supplier interface to support an overload that takes a symbol data buffer, to get around an extraneous read/write of symbol data R=doshimun git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@311 4c0a9323-5329-0410-9bdc-e9ce6186880e --- .../processor/basic_source_line_resolver.h | 6 + .../processor/source_line_resolver_interface.h | 3 + src/google_breakpad/processor/symbol_supplier.h | 16 ++- src/processor/basic_source_line_resolver.cc | 148 +++++++++++++++++---- src/processor/simple_symbol_supplier.cc | 7 +- src/processor/simple_symbol_supplier.h | 18 ++- src/processor/stackwalker.cc | 8 +- 7 files changed, 166 insertions(+), 40 deletions(-) (limited to 'src') diff --git a/src/google_breakpad/processor/basic_source_line_resolver.h b/src/google_breakpad/processor/basic_source_line_resolver.h index 814b01ae..38759579 100644 --- a/src/google_breakpad/processor/basic_source_line_resolver.h +++ b/src/google_breakpad/processor/basic_source_line_resolver.h @@ -69,6 +69,12 @@ class BasicSourceLineResolver : public SourceLineResolverInterface { // retained until the BasicSourceLineResolver is destroyed. virtual bool LoadModule(const string &module_name, const string &map_file); + // Exactly the same as above, except the given map_buffer is used + // for symbols. + virtual bool LoadModuleUsingMapBuffer(const string &module_name, + const string &map_buffer); + + virtual bool HasModule(const string &module_name) const; virtual StackFrameInfo* FillSourceLineInfo(StackFrame *frame) const; diff --git a/src/google_breakpad/processor/source_line_resolver_interface.h b/src/google_breakpad/processor/source_line_resolver_interface.h index 192177b2..93ef85d2 100644 --- a/src/google_breakpad/processor/source_line_resolver_interface.h +++ b/src/google_breakpad/processor/source_line_resolver_interface.h @@ -56,6 +56,9 @@ class SourceLineResolverInterface { // map_file should contain line/address mappings for this module. virtual bool LoadModule(const string &module_name, const string &map_file) = 0; + // Same as above, but takes the contents of a pre-read map buffer + virtual bool LoadModuleUsingMapBuffer(const string &module_name, + const string &map_buffer) = 0; // Returns true if a module with the given name has been loaded. virtual bool HasModule(const string &module_name) const = 0; diff --git a/src/google_breakpad/processor/symbol_supplier.h b/src/google_breakpad/processor/symbol_supplier.h index 0459c8aa..752bbbc3 100644 --- a/src/google_breakpad/processor/symbol_supplier.h +++ b/src/google_breakpad/processor/symbol_supplier.h @@ -59,12 +59,22 @@ class SymbolSupplier { // Retrieves the symbol file for the given CodeModule, placing the // path in symbol_file if successful. system_info contains strings - // identifying the operating system and CPU; SymbolSupplier may use to help - // locate the symbol file. system_info may be NULL or its fields may be - // empty if these values are unknown. + // identifying the operating system and CPU; SymbolSupplier may use + // to help locate the symbol file. system_info may be NULL or its + // fields may be empty if these values are unknown. symbol_file + // must be a pointer to a valid string virtual SymbolResult GetSymbolFile(const CodeModule *module, const SystemInfo *system_info, string *symbol_file) = 0; + // Same as above, except also places symbol data into symbol_data. + // If symbol_data is NULL, the data is not returned. + // TODO(nealsid) Once we have symbol data caching behavior implemented + // investigate making all symbol suppliers implement all methods, + // and make this pure virtual + virtual SymbolResult GetSymbolFile(const CodeModule *module, + const SystemInfo *system_info, + string *symbol_file, + string *symbol_data) { assert(0); } }; } // namespace google_breakpad diff --git a/src/processor/basic_source_line_resolver.cc b/src/processor/basic_source_line_resolver.cc index 6baa1a8b..bba86599 100644 --- a/src/processor/basic_source_line_resolver.cc +++ b/src/processor/basic_source_line_resolver.cc @@ -29,6 +29,9 @@ #include #include +#include +#include +#include #include #include @@ -106,9 +109,13 @@ class BasicSourceLineResolver::Module { public: Module(const string &name) : name_(name) { } - // Loads the given map file, returning true on success. + // Loads the given map file, returning true on success. Reads the + // map file into memory and calls LoadMapFromBuffer bool LoadMap(const string &map_file); + // Loads a map from the given buffer, returning true on success + bool LoadMapFromBuffer(const string &map_buffer); + // Looks up the given relative address, and fills the StackFrame struct // with the result. Additional debugging information, if available, is // returned. If no additional information is available, returns NULL. @@ -211,6 +218,27 @@ bool BasicSourceLineResolver::LoadModule(const string &module_name, return true; } +bool BasicSourceLineResolver::LoadModuleUsingMapBuffer( + const string &module_name, + const string &map_buffer) { + // Make sure we don't already have a module with the given name. + if (modules_->find(module_name) != modules_->end()) { + BPLOG(INFO) << "Symbols for module " << module_name << " already loaded"; + return false; + } + + BPLOG(INFO) << "Loading symbols for module " << module_name << " from buffer"; + + Module *module = new Module(module_name); + if (!module->LoadMapFromBuffer(map_buffer)) { + delete module; + return false; + } + + modules_->insert(make_pair(module_name, module)); + return true; +} + bool BasicSourceLineResolver::HasModule(const string &module_name) const { return modules_->find(module_name) != modules_->end(); } @@ -238,44 +266,56 @@ class AutoFileCloser { FILE *file_; }; -bool BasicSourceLineResolver::Module::LoadMap(const string &map_file) { - FILE *f = fopen(map_file.c_str(), "r"); - if (!f) { - string error_string; - int error_code = ErrnoString(&error_string); - BPLOG(ERROR) << "Could not open " << map_file << - ", error " << error_code << ": " << error_string; +bool BasicSourceLineResolver::Module::LoadMapFromBuffer( + const string &map_buffer) { + linked_ptr cur_func; + int line_number = 0; + const char *map_buffer_c_str = map_buffer.c_str(); + char *save_ptr; + + // set up our input buffer as a c-style string so we + // can we use strtok() + // have to copy because modifying the result of string::c_str is not + // permitted + size_t map_buffer_length = strlen(map_buffer_c_str); + char *map_buffer_chars = new char[map_buffer_length]; + if (map_buffer_chars == NULL) { + BPLOG(ERROR) << "Memory allocation of " << map_buffer_length << + " bytes failed"; return false; } - AutoFileCloser closer(f); + strncpy(map_buffer_chars, map_buffer_c_str, map_buffer_length); - // TODO(mmentovai): this might not be large enough to handle really long - // lines, which might be present for FUNC lines of highly-templatized - // code. - char buffer[8192]; - linked_ptr cur_func; + if (map_buffer_chars[map_buffer_length - 1] == '\n') { + map_buffer_chars[map_buffer_length - 1] = '\0'; + } + char *buffer; + buffer = strtok_r(map_buffer_chars, "\r\n", &save_ptr); - int line_number = 0; - while (fgets(buffer, sizeof(buffer), f)) { + while (buffer != NULL) { ++line_number; + if (strncmp(buffer, "FILE ", 5) == 0) { if (!ParseFile(buffer)) { - BPLOG(ERROR) << "ParseFile failed at " << - map_file << ":" << line_number; + BPLOG(ERROR) << "ParseFile on buffer failed at " << + ":" << line_number; + delete [] map_buffer_chars; return false; } } else if (strncmp(buffer, "STACK ", 6) == 0) { if (!ParseStackInfo(buffer)) { BPLOG(ERROR) << "ParseStackInfo failed at " << - map_file << ":" << line_number; + ":" << line_number; + delete [] map_buffer_chars; return false; } } else if (strncmp(buffer, "FUNC ", 5) == 0) { cur_func.reset(ParseFunction(buffer)); if (!cur_func.get()) { BPLOG(ERROR) << "ParseFunction failed at " << - map_file << ":" << line_number; + ":" << line_number; + delete [] map_buffer_chars; return false; } // StoreRange will fail if the function has an invalid address or size. @@ -288,7 +328,8 @@ bool BasicSourceLineResolver::Module::LoadMap(const string &map_file) { if (!ParsePublicSymbol(buffer)) { BPLOG(ERROR) << "ParsePublicSymbol failed at " << - map_file << ":" << line_number; + ":" << line_number; + delete [] map_buffer_chars; return false; } } else if (strncmp(buffer, "MODULE ", 7) == 0) { @@ -301,23 +342,82 @@ bool BasicSourceLineResolver::Module::LoadMap(const string &map_file) { } else { if (!cur_func.get()) { BPLOG(ERROR) << "Found source line data without a function at " << - map_file << ":" << line_number; + ":" << line_number; + delete [] map_buffer_chars; return false; } Line *line = ParseLine(buffer); if (!line) { - BPLOG(ERROR) << "ParseLine failed at " << - map_file << ":" << line_number; + BPLOG(ERROR) << "ParseLine failed at " << line_number << " for " << + buffer; + delete [] map_buffer_chars; return false; } cur_func->lines.StoreRange(line->address, line->size, linked_ptr(line)); } + + buffer = strtok_r(NULL, "\r\n", &save_ptr); } return true; } +bool BasicSourceLineResolver::Module::LoadMap(const string &map_file) { + struct stat buf; + int error_code = stat(map_file.c_str(), &buf); + if (error_code == -1) { + string error_string; + int error_code = ErrnoString(&error_string); + BPLOG(ERROR) << "Could not open " << map_file << + ", error " << error_code << ": " << error_string; + return false; + } + + off_t file_size = buf.st_size; + + // Allocate memory for file contents, plus a null terminator + // since we'll use strtok() on the contents. + char *file_buffer = new char[sizeof(char)*file_size + 1]; + + if (file_buffer == NULL) { + BPLOG(ERROR) << "Could not allocate memory for " << map_file; + return false; + } + + BPLOG(ERROR) << "Opening " << map_file; + + FILE *f = fopen(map_file.c_str(), "rt"); + if (!f) { + string error_string; + int error_code = ErrnoString(&error_string); + BPLOG(ERROR) << "Could not open " << map_file << + ", error " << error_code << ": " << error_string; + delete [] file_buffer; + return false; + } + + AutoFileCloser closer(f); + + int items_read = 0; + + items_read = fread(file_buffer, 1, file_size, f); + + if (items_read != file_size) { + string error_string; + int error_code = ErrnoString(&error_string); + BPLOG(ERROR) << "Could not slurp " << map_file << + ", error " << error_code << ": " << error_string; + delete [] file_buffer; + return false; + } + file_buffer[file_size] = '\0'; + string map_buffer(file_buffer); + delete [] file_buffer; + + return LoadMapFromBuffer(map_buffer); +} + StackFrameInfo* BasicSourceLineResolver::Module::LookupAddress( StackFrame *frame) const { MemAddr address = frame->instruction - frame->module->base_address(); diff --git a/src/processor/simple_symbol_supplier.cc b/src/processor/simple_symbol_supplier.cc index 291361cb..9c1aac4e 100644 --- a/src/processor/simple_symbol_supplier.cc +++ b/src/processor/simple_symbol_supplier.cc @@ -62,15 +62,16 @@ SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFile( for (unsigned int path_index = 0; path_index < paths_.size(); ++path_index) { SymbolResult result; - if ((result = GetSymbolFileAtPath(module, system_info, paths_[path_index], - symbol_file)) != NOT_FOUND) { + if ((result = GetSymbolFileAtPathFromRoot(module, system_info, + paths_[path_index], + symbol_file)) != NOT_FOUND) { return result; } } return NOT_FOUND; } -SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFileAtPath( +SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFileAtPathFromRoot( const CodeModule *module, const SystemInfo *system_info, const string &root_path, string *symbol_file) { BPLOG_IF(ERROR, !symbol_file) << "SimpleSymbolSupplier::GetSymbolFileAtPath " diff --git a/src/processor/simple_symbol_supplier.h b/src/processor/simple_symbol_supplier.h index 44e0d617..9343e448 100644 --- a/src/processor/simple_symbol_supplier.h +++ b/src/processor/simple_symbol_supplier.h @@ -102,15 +102,19 @@ class SimpleSymbolSupplier : public SymbolSupplier { // Returns the path to the symbol file for the given module. See the // description above. - SymbolResult GetSymbolFile(const CodeModule *module, - const SystemInfo *system_info, - string *symbol_file); + virtual SymbolResult GetSymbolFile(const CodeModule *module, + const SystemInfo *system_info, + string *symbol_file); + virtual SymbolResult GetSymbolFile(const CodeModule *module, + const SystemInfo *system_info, + string *symbol_file, + string *symbol_data) { assert(0); } protected: - SymbolResult GetSymbolFileAtPath(const CodeModule *module, - const SystemInfo *system_info, - const string &root_path, - string *symbol_file); + SymbolResult GetSymbolFileAtPathFromRoot(const CodeModule *module, + const SystemInfo *system_info, + const string &root_path, + string *symbol_file); private: vector paths_; diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc index cdb0ce52..de67bdaa 100644 --- a/src/processor/stackwalker.cc +++ b/src/processor/stackwalker.cc @@ -101,13 +101,15 @@ bool Stackwalker::Walk(CallStack *stack) { if (resolver_ && !resolver_->HasModule(frame->module->code_file()) && supplier_) { - string symbol_file; + string symbol_data, symbol_file; SymbolSupplier::SymbolResult symbol_result = - supplier_->GetSymbolFile(module, system_info_, &symbol_file); + supplier_->GetSymbolFile(module, system_info_, + &symbol_file, &symbol_data); switch (symbol_result) { case SymbolSupplier::FOUND: - resolver_->LoadModule(frame->module->code_file(), symbol_file); + resolver_->LoadModuleUsingMapBuffer(frame->module->code_file(), + symbol_data); break; case SymbolSupplier::NOT_FOUND: break; // nothing to do -- cgit v1.2.1