aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--Makefile.am16
-rw-r--r--Makefile.in35
-rw-r--r--src/processor/address_map-inl.h12
-rw-r--r--src/processor/address_map.h4
-rw-r--r--src/processor/address_map_unittest.cc1
-rw-r--r--src/processor/basic_code_modules.cc18
-rw-r--r--src/processor/basic_source_line_resolver.cc56
-rw-r--r--src/processor/contained_range_map-inl.h26
-rw-r--r--src/processor/contained_range_map.h3
-rw-r--r--src/processor/minidump.cc4
-rw-r--r--src/processor/postfix_evaluator-inl.h40
-rw-r--r--src/processor/range_map-inl.h40
-rw-r--r--src/processor/range_map.h11
-rw-r--r--src/processor/simple_symbol_supplier.cc26
-rw-r--r--src/processor/stackwalker.cc9
-rw-r--r--src/processor/stackwalker_ppc.cc12
-rw-r--r--src/processor/stackwalker_x86.cc12
17 files changed, 265 insertions, 60 deletions
diff --git a/Makefile.am b/Makefile.am
index e6fd0a4c..c78c1293 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -128,14 +128,22 @@ TESTS_ENVIRONMENT =
src_processor_address_map_unittest_SOURCES = \
src/processor/address_map_unittest.cc
+src_processor_address_map_unittest_LDADD = \
+ src/processor/logging.lo \
+ src/processor/pathname_stripper.lo
src_processor_basic_source_line_resolver_unittest_SOURCES = \
src/processor/basic_source_line_resolver_unittest.cc
src_processor_basic_source_line_resolver_unittest_LDADD = \
- src/processor/basic_source_line_resolver.lo
+ src/processor/basic_source_line_resolver.lo \
+ src/processor/pathname_stripper.lo \
+ src/processor/logging.lo
src_processor_contained_range_map_unittest_SOURCES = \
src/processor/contained_range_map_unittest.cc
+src_processor_contained_range_map_unittest_LDADD = \
+ src/processor/logging.lo \
+ src/processor/pathname_stripper.lo
src_processor_minidump_processor_unittest_SOURCES = \
src/processor/minidump_processor_unittest.cc
@@ -159,9 +167,15 @@ src_processor_pathname_stripper_unittest_LDADD = \
src_processor_postfix_evaluator_unittest_SOURCES = \
src/processor/postfix_evaluator_unittest.cc
+src_processor_postfix_evaluator_unittest_LDADD = \
+ src/processor/logging.lo \
+ src/processor/pathname_stripper.lo
src_processor_range_map_unittest_SOURCES = \
src/processor/range_map_unittest.cc
+src_processor_range_map_unittest_LDADD = \
+ src/processor/logging.lo \
+ src/processor/pathname_stripper.lo
src_processor_stackwalker_selftest_SOURCES = \
src/processor/stackwalker_selftest.cc
diff --git a/Makefile.in b/Makefile.in
index d2bc94a7..c6283f55 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -123,17 +123,20 @@ am_src_processor_address_map_unittest_OBJECTS = \
src/processor/address_map_unittest.$(OBJEXT)
src_processor_address_map_unittest_OBJECTS = \
$(am_src_processor_address_map_unittest_OBJECTS)
-src_processor_address_map_unittest_LDADD = $(LDADD)
+src_processor_address_map_unittest_DEPENDENCIES = \
+ src/processor/logging.lo src/processor/pathname_stripper.lo
am_src_processor_basic_source_line_resolver_unittest_OBJECTS = \
src/processor/basic_source_line_resolver_unittest.$(OBJEXT)
src_processor_basic_source_line_resolver_unittest_OBJECTS = $(am_src_processor_basic_source_line_resolver_unittest_OBJECTS)
src_processor_basic_source_line_resolver_unittest_DEPENDENCIES = \
- src/processor/basic_source_line_resolver.lo
+ src/processor/basic_source_line_resolver.lo \
+ src/processor/pathname_stripper.lo src/processor/logging.lo
am_src_processor_contained_range_map_unittest_OBJECTS = \
src/processor/contained_range_map_unittest.$(OBJEXT)
src_processor_contained_range_map_unittest_OBJECTS = \
$(am_src_processor_contained_range_map_unittest_OBJECTS)
-src_processor_contained_range_map_unittest_LDADD = $(LDADD)
+src_processor_contained_range_map_unittest_DEPENDENCIES = \
+ src/processor/logging.lo src/processor/pathname_stripper.lo
am_src_processor_minidump_dump_OBJECTS = \
src/processor/minidump_dump.$(OBJEXT)
src_processor_minidump_dump_OBJECTS = \
@@ -178,12 +181,14 @@ am_src_processor_postfix_evaluator_unittest_OBJECTS = \
src/processor/postfix_evaluator_unittest.$(OBJEXT)
src_processor_postfix_evaluator_unittest_OBJECTS = \
$(am_src_processor_postfix_evaluator_unittest_OBJECTS)
-src_processor_postfix_evaluator_unittest_LDADD = $(LDADD)
+src_processor_postfix_evaluator_unittest_DEPENDENCIES = \
+ src/processor/logging.lo src/processor/pathname_stripper.lo
am_src_processor_range_map_unittest_OBJECTS = \
src/processor/range_map_unittest.$(OBJEXT)
src_processor_range_map_unittest_OBJECTS = \
$(am_src_processor_range_map_unittest_OBJECTS)
-src_processor_range_map_unittest_LDADD = $(LDADD)
+src_processor_range_map_unittest_DEPENDENCIES = \
+ src/processor/logging.lo src/processor/pathname_stripper.lo
am_src_processor_stackwalker_selftest_OBJECTS = \
src/processor/stackwalker_selftest.$(OBJEXT)
src_processor_stackwalker_selftest_OBJECTS = \
@@ -429,15 +434,25 @@ TESTS_ENVIRONMENT =
src_processor_address_map_unittest_SOURCES = \
src/processor/address_map_unittest.cc
+src_processor_address_map_unittest_LDADD = \
+ src/processor/logging.lo \
+ src/processor/pathname_stripper.lo
+
src_processor_basic_source_line_resolver_unittest_SOURCES = \
src/processor/basic_source_line_resolver_unittest.cc
src_processor_basic_source_line_resolver_unittest_LDADD = \
- src/processor/basic_source_line_resolver.lo
+ src/processor/basic_source_line_resolver.lo \
+ src/processor/pathname_stripper.lo \
+ src/processor/logging.lo
src_processor_contained_range_map_unittest_SOURCES = \
src/processor/contained_range_map_unittest.cc
+src_processor_contained_range_map_unittest_LDADD = \
+ src/processor/logging.lo \
+ src/processor/pathname_stripper.lo
+
src_processor_minidump_processor_unittest_SOURCES = \
src/processor/minidump_processor_unittest.cc
@@ -463,9 +478,17 @@ src_processor_pathname_stripper_unittest_LDADD = \
src_processor_postfix_evaluator_unittest_SOURCES = \
src/processor/postfix_evaluator_unittest.cc
+src_processor_postfix_evaluator_unittest_LDADD = \
+ src/processor/logging.lo \
+ src/processor/pathname_stripper.lo
+
src_processor_range_map_unittest_SOURCES = \
src/processor/range_map_unittest.cc
+src_processor_range_map_unittest_LDADD = \
+ src/processor/logging.lo \
+ src/processor/pathname_stripper.lo
+
src_processor_stackwalker_selftest_SOURCES = \
src/processor/stackwalker_selftest.cc
diff --git a/src/processor/address_map-inl.h b/src/processor/address_map-inl.h
index e09dcae5..d88b4fcc 100644
--- a/src/processor/address_map-inl.h
+++ b/src/processor/address_map-inl.h
@@ -36,7 +36,10 @@
#ifndef PROCESSOR_ADDRESS_MAP_INL_H__
#define PROCESSOR_ADDRESS_MAP_INL_H__
+#include <cassert>
+
#include "processor/address_map.h"
+#include "processor/logging.h"
namespace google_breakpad {
@@ -45,8 +48,11 @@ bool AddressMap<AddressType, EntryType>::Store(const AddressType &address,
const EntryType &entry) {
// Ensure that the specified address doesn't conflict with something already
// in the map.
- if (map_.find(address) != map_.end())
+ if (map_.find(address) != map_.end()) {
+ BPLOG(INFO) << "Store failed, address " << HexString(address) <<
+ " is already present";
return false;
+ }
map_.insert(MapValue(address, entry));
return true;
@@ -56,8 +62,8 @@ template<typename AddressType, typename EntryType>
bool AddressMap<AddressType, EntryType>::Retrieve(
const AddressType &address,
EntryType *entry, AddressType *entry_address) const {
- if (!entry)
- return false;
+ BPLOG_IF(ERROR, !entry) << "AddressMap::Retrieve requires |entry|";
+ assert(entry);
// upper_bound gives the first element whose key is greater than address,
// but we want the first element whose key is less than or equal to address.
diff --git a/src/processor/address_map.h b/src/processor/address_map.h
index 44632323..14139e7a 100644
--- a/src/processor/address_map.h
+++ b/src/processor/address_map.h
@@ -53,8 +53,8 @@ class AddressMap {
bool Store(const AddressType &address, const EntryType &entry);
// Locates the entry stored at the highest address less than or equal to
- // the address argument. If there is no such range, or if there is a
- // parameter error, returns false. The entry is returned in entry. If
+ // the address argument. If there is no such range, returns false. The
+ // entry is returned in entry, which is a required argument. If
// entry_address is not NULL, it will be set to the address that the entry
// was stored at.
bool Retrieve(const AddressType &address,
diff --git a/src/processor/address_map_unittest.cc b/src/processor/address_map_unittest.cc
index cd6b5a79..3c865895 100644
--- a/src/processor/address_map_unittest.cc
+++ b/src/processor/address_map_unittest.cc
@@ -107,7 +107,6 @@ static bool DoAddressMapTest() {
ASSERT_EQ(entry->id(), 1);
ASSERT_EQ(address, 10);
ASSERT_TRUE(test_map.Retrieve(11, &entry, &address));
- ASSERT_FALSE(test_map.Retrieve(11, NULL, &address)); // parameter error
ASSERT_TRUE(test_map.Retrieve(11, &entry, NULL)); // NULL ok here
// Add some more elements.
diff --git a/src/processor/basic_code_modules.cc b/src/processor/basic_code_modules.cc
index 1e3ba99a..a21491c4 100644
--- a/src/processor/basic_code_modules.cc
+++ b/src/processor/basic_code_modules.cc
@@ -39,6 +39,7 @@
#include "processor/basic_code_modules.h"
#include "google_breakpad/processor/code_module.h"
#include "processor/linked_ptr.h"
+#include "processor/logging.h"
#include "processor/range_map-inl.h"
namespace google_breakpad {
@@ -46,6 +47,8 @@ namespace google_breakpad {
BasicCodeModules::BasicCodeModules(const CodeModules *that)
: main_address_(0),
map_(new RangeMap<u_int64_t, linked_ptr<const CodeModule> >()) {
+ BPLOG_IF(ERROR, !that) << "BasicCodeModules::BasicCodeModules requires "
+ "|that|";
assert(that);
const CodeModule *main_module = that->GetMainModule();
@@ -61,8 +64,11 @@ BasicCodeModules::BasicCodeModules(const CodeModules *that)
// entire list, and GetModuleAtIndex may be faster than
// GetModuleAtSequence.
const CodeModule *module = that->GetModuleAtIndex(module_sequence)->Copy();
- map_->StoreRange(module->base_address(), module->size(),
- linked_ptr<const CodeModule>(module));
+ if (!map_->StoreRange(module->base_address(), module->size(),
+ linked_ptr<const CodeModule>(module))) {
+ BPLOG(ERROR) << "Module " << module->code_file() <<
+ " could not be stored";
+ }
}
}
@@ -77,8 +83,10 @@ unsigned int BasicCodeModules::module_count() const {
const CodeModule* BasicCodeModules::GetModuleForAddress(
u_int64_t address) const {
linked_ptr<const CodeModule> module;
- if (!map_->RetrieveRange(address, &module, NULL, NULL))
+ if (!map_->RetrieveRange(address, &module, NULL, NULL)) {
+ BPLOG(INFO) << "No module at " << HexString(address);
return NULL;
+ }
return module.get();
}
@@ -90,8 +98,10 @@ const CodeModule* BasicCodeModules::GetMainModule() const {
const CodeModule* BasicCodeModules::GetModuleAtSequence(
unsigned int sequence) const {
linked_ptr<const CodeModule> module;
- if (!map_->RetrieveRangeAtIndex(sequence, &module, NULL, NULL))
+ if (!map_->RetrieveRangeAtIndex(sequence, &module, NULL, NULL)) {
+ BPLOG(ERROR) << "RetrieveRangeAtIndex failed for sequence " << sequence;
return NULL;
+ }
return module.get();
}
diff --git a/src/processor/basic_source_line_resolver.cc b/src/processor/basic_source_line_resolver.cc
index a66a4700..e5d1bd7f 100644
--- a/src/processor/basic_source_line_resolver.cc
+++ b/src/processor/basic_source_line_resolver.cc
@@ -145,7 +145,7 @@ class BasicSourceLineResolver::Module {
static bool Tokenize(char *line, int max_tokens, vector<char*> *tokens);
// Parses a file declaration
- void ParseFile(char *file_line);
+ bool ParseFile(char *file_line);
// Parses a function declaration, returning a new Function object.
Function* ParseFunction(char *function_line);
@@ -188,9 +188,13 @@ bool BasicSourceLineResolver::LoadModule(const string &module_name,
const string &map_file) {
// 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 " <<
+ map_file;
+
Module *module = new Module(module_name);
if (!module->LoadMap(map_file)) {
delete module;
@@ -216,28 +220,56 @@ StackFrameInfo* BasicSourceLineResolver::FillSourceLineInfo(
return NULL;
}
+class AutoFileCloser {
+ public:
+ AutoFileCloser(FILE *file) : file_(file) {}
+ ~AutoFileCloser() {
+ if (file_)
+ fclose(file_);
+ }
+
+ private:
+ 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;
return false;
}
+ AutoFileCloser closer(f);
+
// 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<Function> cur_func;
+ int line_number = 0;
while (fgets(buffer, sizeof(buffer), f)) {
+ ++line_number;
if (strncmp(buffer, "FILE ", 5) == 0) {
- ParseFile(buffer);
+ if (!ParseFile(buffer)) {
+ BPLOG(ERROR) << "ParseFile failed at " <<
+ map_file << ":" << line_number;
+ return false;
+ }
} else if (strncmp(buffer, "STACK ", 6) == 0) {
if (!ParseStackInfo(buffer)) {
+ BPLOG(ERROR) << "ParseStackInfo failed at " <<
+ map_file << ":" << line_number;
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;
return false;
}
// StoreRange will fail if the function has an invalid address or size.
@@ -249,6 +281,8 @@ bool BasicSourceLineResolver::Module::LoadMap(const string &map_file) {
cur_func.reset();
if (!ParsePublicSymbol(buffer)) {
+ BPLOG(ERROR) << "ParsePublicSymbol failed at " <<
+ map_file << ":" << line_number;
return false;
}
} else if (strncmp(buffer, "MODULE ", 7) == 0) {
@@ -260,10 +294,14 @@ bool BasicSourceLineResolver::Module::LoadMap(const string &map_file) {
// MODULE <guid> <age> <filename>
} else {
if (!cur_func.get()) {
+ BPLOG(ERROR) << "Found source line data without a function at " <<
+ map_file << ":" << line_number;
return false;
}
Line *line = ParseLine(buffer);
if (!line) {
+ BPLOG(ERROR) << "ParseLine failed at " <<
+ map_file << ":" << line_number;
return false;
}
cur_func->lines.StoreRange(line->address, line->size,
@@ -271,7 +309,6 @@ bool BasicSourceLineResolver::Module::LoadMap(const string &map_file) {
}
}
- fclose(f);
return true;
}
@@ -387,24 +424,27 @@ bool BasicSourceLineResolver::Module::Tokenize(char *line, int max_tokens,
return tokens->size() == static_cast<unsigned int>(max_tokens);
}
-void BasicSourceLineResolver::Module::ParseFile(char *file_line) {
+bool BasicSourceLineResolver::Module::ParseFile(char *file_line) {
// FILE <id> <filename>
file_line += 5; // skip prefix
vector<char*> tokens;
if (!Tokenize(file_line, 2, &tokens)) {
- return;
+ return false;
}
int index = atoi(tokens[0]);
if (index < 0) {
- return;
+ return false;
}
char *filename = tokens[1];
- if (filename) {
- files_.insert(make_pair(index, string(filename)));
+ if (!filename) {
+ return false;
}
+
+ files_.insert(make_pair(index, string(filename)));
+ return true;
}
BasicSourceLineResolver::Function*
diff --git a/src/processor/contained_range_map-inl.h b/src/processor/contained_range_map-inl.h
index 5f029712..97f9fdbe 100644
--- a/src/processor/contained_range_map-inl.h
+++ b/src/processor/contained_range_map-inl.h
@@ -37,7 +37,10 @@
#define PROCESSOR_CONTAINED_RANGE_MAP_INL_H__
+#include <cassert>
+
#include "processor/contained_range_map.h"
+#include "processor/logging.h"
namespace google_breakpad {
@@ -56,8 +59,11 @@ bool ContainedRangeMap<AddressType, EntryType>::StoreRange(
AddressType high = base + size - 1;
// Check for undersize or overflow.
- if (size <= 0 || high < base)
+ if (size <= 0 || high < base) {
+ BPLOG(INFO) << "StoreRange failed, " << HexString(base) << "+" <<
+ HexString(size) << ", " << HexString(high);
return false;
+ }
if (!map_)
map_ = new AddressToRangeMap();
@@ -74,8 +80,11 @@ bool ContainedRangeMap<AddressType, EntryType>::StoreRange(
// range's, it violates the containment rules, and an attempt to store
// it must fail. iterator_base->first contains the key, which was the
// containing child's high address.
- if (iterator_base->second->base_ == base && iterator_base->first == high)
+ if (iterator_base->second->base_ == base && iterator_base->first == high) {
+ BPLOG(INFO) << "StoreRange failed, identical range is already "
+ "present: " << HexString(base) << "+" << HexString(size);
return false;
+ }
// Pass the new range on to the child to attempt to store.
return iterator_base->second->StoreRange(base, size, entry);
@@ -92,6 +101,12 @@ bool ContainedRangeMap<AddressType, EntryType>::StoreRange(
// fully. Partial containment isn't allowed.
if ((iterator_base != iterator_end && base > iterator_base->second->base_) ||
(contains_high && high < iterator_high->first)) {
+ // TODO(mmentovai): Some symbol files will trip this check frequently
+ // on STACK lines. Too many messages will be produced. These are more
+ // suitable for a DEBUG channel than an INFO channel.
+ // BPLOG(INFO) << "StoreRange failed, new range partially contains "
+ // "existing range: " << HexString(base) << "+" <<
+ // HexString(size);
return false;
}
@@ -130,7 +145,12 @@ bool ContainedRangeMap<AddressType, EntryType>::StoreRange(
template<typename AddressType, typename EntryType>
bool ContainedRangeMap<AddressType, EntryType>::RetrieveRange(
const AddressType &address, EntryType *entry) const {
- if (!entry || !map_)
+ BPLOG_IF(ERROR, !entry) << "ContainedRangeMap::RetrieveRange requires "
+ "|entry|";
+ assert(entry);
+
+ // If nothing was ever stored, then there's nothing to retrieve.
+ if (!map_)
return false;
// Get an iterator to the child range whose high address is equal to or
diff --git a/src/processor/contained_range_map.h b/src/processor/contained_range_map.h
index deb1e061..f30016f3 100644
--- a/src/processor/contained_range_map.h
+++ b/src/processor/contained_range_map.h
@@ -92,8 +92,7 @@ class ContainedRangeMap {
// the specified address. This method will only return entries held by
// child ranges, and not the entry contained by |this|. This is necessary
// to support a sparsely-populated root range. If no descendant range
- // encompasses the address, or if there is a parameter error, returns
- // false.
+ // encompasses the address, returns false.
bool RetrieveRange(const AddressType &address, EntryType *entry) const;
// Removes all children. Note that Clear only removes descendants,
diff --git a/src/processor/minidump.cc b/src/processor/minidump.cc
index 63a36037..28cc22ed 100644
--- a/src/processor/minidump.cc
+++ b/src/processor/minidump.cc
@@ -2650,9 +2650,9 @@ bool MinidumpMiscInfo::Read(u_int32_t expected_size) {
}
}
- if (misc_info_.size_of_info != expected_size) {
+ if (expected_size != misc_info_.size_of_info) {
BPLOG(ERROR) << "MinidumpMiscInfo size mismatch, " <<
- misc_info_.size_of_info << " != " << expected_size;
+ expected_size << " != " << misc_info_.size_of_info;
return false;
}
diff --git a/src/processor/postfix_evaluator-inl.h b/src/processor/postfix_evaluator-inl.h
index 54796df1..0b5f9c9b 100644
--- a/src/processor/postfix_evaluator-inl.h
+++ b/src/processor/postfix_evaluator-inl.h
@@ -27,6 +27,7 @@
#include "processor/postfix_evaluator.h"
#include "google_breakpad/processor/memory_region.h"
+#include "processor/logging.h"
namespace google_breakpad {
@@ -83,8 +84,11 @@ bool PostfixEvaluator<ValueType>::Evaluate(const string &expression,
if (operation != BINARY_OP_NONE) {
// Get the operands.
ValueType operand1, operand2;
- if (!PopValues(&operand1, &operand2))
+ if (!PopValues(&operand1, &operand2)) {
+ BPLOG(ERROR) << "Could not PopValues to get two values for binary "
+ "operation " << token << ": " << expression;
return false;
+ }
// Perform the operation.
ValueType result;
@@ -107,6 +111,7 @@ bool PostfixEvaluator<ValueType>::Evaluate(const string &expression,
case BINARY_OP_NONE:
// This will not happen, but compilers will want a default or
// BINARY_OP_NONE case.
+ BPLOG(ERROR) << "Not reached!";
return false;
break;
}
@@ -115,32 +120,51 @@ bool PostfixEvaluator<ValueType>::Evaluate(const string &expression,
PushValue(result);
} else if (token == "^") {
// ^ for unary dereference. Can't dereference without memory.
- if (!memory_)
+ if (!memory_) {
+ BPLOG(ERROR) << "Attempt to dereference without memory: " <<
+ expression;
return false;
+ }
ValueType address;
- if (!PopValue(&address))
+ if (!PopValue(&address)) {
+ BPLOG(ERROR) << "Could not PopValue to get value to derefence: " <<
+ expression;
return false;
+ }
ValueType value;
- if (!memory_->GetMemoryAtAddress(address, &value))
+ if (!memory_->GetMemoryAtAddress(address, &value)) {
+ BPLOG(ERROR) << "Could not dereference memory at address " <<
+ HexString(address) << ": " << expression;
return false;
+ }
PushValue(value);
} else if (token == "=") {
// = for assignment.
ValueType value;
- if (!PopValue(&value))
+ if (!PopValue(&value)) {
+ BPLOG(ERROR) << "Could not PopValue to get value to assign: " <<
+ expression;
return false;
+ }
// Assignment is only meaningful when assigning into an identifier.
// The identifier must name a variable, not a constant. Variables
// begin with '$'.
string identifier;
- if (PopValueOrIdentifier(NULL, &identifier) != POP_RESULT_IDENTIFIER)
+ if (PopValueOrIdentifier(NULL, &identifier) != POP_RESULT_IDENTIFIER) {
+ BPLOG(ERROR) << "PopValueOrIdentifier returned a value, but an "
+ "identifier is needed to assign " <<
+ HexString(value) << ": " << expression;
return false;
- if (identifier.empty() || identifier[0] != '$')
+ }
+ if (identifier.empty() || identifier[0] != '$') {
+ BPLOG(ERROR) << "Can't assign " << HexString(value) << " to " <<
+ identifier << ": " << expression;
return false;
+ }
(*dictionary_)[identifier] = value;
if (assigned)
@@ -157,6 +181,7 @@ bool PostfixEvaluator<ValueType>::Evaluate(const string &expression,
// If there's anything left on the stack, it indicates incomplete execution.
// This is a failure case. If the stack is empty, evalution was complete
// and successful.
+ BPLOG_IF(ERROR, !stack_.empty()) << "Incomplete execution: " << expression;
return stack_.empty();
}
@@ -210,6 +235,7 @@ bool PostfixEvaluator<ValueType>::PopValue(ValueType *value) {
if (iterator == dictionary_->end()) {
// The identifier wasn't found in the dictionary. Don't imply any
// default value, just fail.
+ BPLOG(ERROR) << "Identifier " << token << " not in dictionary";
return false;
}
diff --git a/src/processor/range_map-inl.h b/src/processor/range_map-inl.h
index 3b01f9e3..17169ea4 100644
--- a/src/processor/range_map-inl.h
+++ b/src/processor/range_map-inl.h
@@ -38,6 +38,7 @@
#include "processor/range_map.h"
+#include "processor/logging.h"
namespace google_breakpad {
@@ -50,8 +51,15 @@ bool RangeMap<AddressType, EntryType>::StoreRange(const AddressType &base,
AddressType high = base + size - 1;
// Check for undersize or overflow.
- if (size <= 0 || high < base)
+ if (size <= 0 || high < base) {
+ // The processor will hit this case too frequently with common symbol
+ // files in the size == 0 case, which is more suited to a DEBUG channel.
+ // Filter those out since there's no DEBUG channel at the moment.
+ BPLOG_IF(INFO, size != 0) << "StoreRange failed, " << HexString(base) <<
+ "+" << HexString(size) << ", " <<
+ HexString(high);
return false;
+ }
// Ensure that this range does not overlap with another one already in the
// map.
@@ -62,14 +70,27 @@ bool RangeMap<AddressType, EntryType>::StoreRange(const AddressType &base,
// Some other range begins in the space used by this range. It may be
// contained within the space used by this range, or it may extend lower.
// Regardless, it is an error.
+ AddressType other_base = iterator_base->second.base();
+ AddressType other_size = iterator_base->first - other_base + 1;
+ BPLOG(INFO) << "StoreRange failed, an existing range is contained by or "
+ "extends lower than the new range: new " <<
+ HexString(base) << "+" << HexString(size) << ", existing " <<
+ HexString(other_base) << "+" << HexString(other_size);
return false;
}
if (iterator_high != map_.end()) {
if (iterator_high->second.base() <= high) {
// The range above this one overlaps with this one. It may fully
- // contain this range, or it may begin within this range and extend
+ // contain this range, or it may begin within this range and extend
// higher. Regardless, it's an error.
+ AddressType other_base = iterator_high->second.base();
+ AddressType other_size = iterator_high->first - other_base + 1;
+ BPLOG(INFO) << "StoreRange failed, an existing range contains or "
+ "extends higher than the new range: new " <<
+ HexString(base) << "+" << HexString(size) <<
+ ", existing " <<
+ HexString(other_base) << "+" << HexString(other_size);
return false;
}
}
@@ -85,8 +106,8 @@ template<typename AddressType, typename EntryType>
bool RangeMap<AddressType, EntryType>::RetrieveRange(
const AddressType &address, EntryType *entry,
AddressType *entry_base, AddressType *entry_size) const {
- if (!entry)
- return false;
+ BPLOG_IF(ERROR, !entry) << "RangeMap::RetrieveRange requires |entry|";
+ assert(entry);
MapConstIterator iterator = map_.lower_bound(address);
if (iterator == map_.end())
@@ -114,8 +135,8 @@ template<typename AddressType, typename EntryType>
bool RangeMap<AddressType, EntryType>::RetrieveNearestRange(
const AddressType &address, EntryType *entry,
AddressType *entry_base, AddressType *entry_size) const {
- if (!entry)
- return false;
+ BPLOG_IF(ERROR, !entry) << "RangeMap::RetrieveNearestRange requires |entry|";
+ assert(entry);
// If address is within a range, RetrieveRange can handle it.
if (RetrieveRange(address, entry, entry_base, entry_size))
@@ -145,8 +166,13 @@ template<typename AddressType, typename EntryType>
bool RangeMap<AddressType, EntryType>::RetrieveRangeAtIndex(
int index, EntryType *entry,
AddressType *entry_base, AddressType *entry_size) const {
- if (!entry || index >= GetCount())
+ BPLOG_IF(ERROR, !entry) << "RangeMap::RetrieveRangeAtIndex requires |entry|";
+ assert(entry);
+
+ if (index >= GetCount()) {
+ BPLOG(ERROR) << "Index out of range: " << index << "/" << GetCount();
return false;
+ }
// Walk through the map. Although it's ordered, it's not a vector, so it
// can't be addressed directly by index.
diff --git a/src/processor/range_map.h b/src/processor/range_map.h
index 9366563d..a7b67412 100644
--- a/src/processor/range_map.h
+++ b/src/processor/range_map.h
@@ -60,9 +60,8 @@ class RangeMap {
const EntryType &entry);
// Locates the range encompassing the supplied address. If there is
- // no such range, or if there is a parameter error, returns false.
- // entry_base and entry_size, if non-NULL, are set to the base and size
- // of the entry's range.
+ // no such range, returns false. entry_base and entry_size, if non-NULL,
+ // are set to the base and size of the entry's range.
bool RetrieveRange(const AddressType &address, EntryType *entry,
AddressType *entry_base, AddressType *entry_size) const;
@@ -77,9 +76,9 @@ class RangeMap {
// Treating all ranges as a list ordered by the address spaces that they
// occupy, locates the range at the index specified by index. Returns
- // false if index is larger than the number of ranges stored, or if another
- // parameter error occurs. entry_base and entry_size, if non-NULL, are set
- // to the base and size of the entry's range.
+ // false if index is larger than the number of ranges stored. entry_base
+ // and entry_size, if non-NULL, are set to the base and size of the entry's
+ // range.
//
// RetrieveRangeAtIndex is not optimized for speedy operation.
bool RetrieveRangeAtIndex(int index, EntryType *entry,
diff --git a/src/processor/simple_symbol_supplier.cc b/src/processor/simple_symbol_supplier.cc
index 734a7f77..62800013 100644
--- a/src/processor/simple_symbol_supplier.cc
+++ b/src/processor/simple_symbol_supplier.cc
@@ -41,6 +41,7 @@
#include "processor/simple_symbol_supplier.h"
#include "google_breakpad/processor/code_module.h"
#include "google_breakpad/processor/system_info.h"
+#include "processor/logging.h"
#include "processor/pathname_stripper.h"
namespace google_breakpad {
@@ -53,7 +54,11 @@ static bool file_exists(const string &file_name) {
SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFile(
const CodeModule *module, const SystemInfo *system_info,
string *symbol_file) {
+ BPLOG_IF(ERROR, !symbol_file) << "SimpleSymbolSupplier::GetSymbolFile "
+ "requires |symbol_file|";
assert(symbol_file);
+ symbol_file->clear();
+
for (unsigned int path_index = 0; path_index < paths_.size(); ++path_index) {
SymbolResult result;
if ((result = GetSymbolFileAtPath(module, system_info, paths_[path_index],
@@ -67,7 +72,11 @@ SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFile(
SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFileAtPath(
const CodeModule *module, const SystemInfo *system_info,
const string &root_path, string *symbol_file) {
+ BPLOG_IF(ERROR, !symbol_file) << "SimpleSymbolSupplier::GetSymbolFileAtPath "
+ "requires |symbol_file|";
assert(symbol_file);
+ symbol_file->clear();
+
if (!module)
return NOT_FOUND;
@@ -77,15 +86,24 @@ SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFileAtPath(
// Append the debug (pdb) file name as a directory name.
path.append("/");
string debug_file_name = PathnameStripper::File(module->debug_file());
- if (debug_file_name.empty())
+ if (debug_file_name.empty()) {
+ BPLOG(ERROR) << "Can't construct symbol file path without debug_file "
+ "(code_file = " <<
+ PathnameStripper::File(module->code_file()) << ")";
return NOT_FOUND;
+ }
path.append(debug_file_name);
// Append the identifier as a directory name.
path.append("/");
string identifier = module->debug_identifier();
- if (identifier.empty())
+ if (identifier.empty()) {
+ BPLOG(ERROR) << "Can't construct symbol file path without debug_identifier "
+ "(code_file = " <<
+ PathnameStripper::File(module->code_file()) <<
+ ", debug_file = " << debug_file_name << ")";
return NOT_FOUND;
+ }
path.append(identifier);
// Transform the debug file name into one ending in .sym. If the existing
@@ -103,8 +121,10 @@ SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFileAtPath(
}
path.append(".sym");
- if (!file_exists(path))
+ if (!file_exists(path)) {
+ BPLOG(INFO) << "No symbol file at " << path;
return NOT_FOUND;
+ }
*symbol_file = path;
return FOUND;
diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc
index 00eaa6af..46d8384b 100644
--- a/src/processor/stackwalker.cc
+++ b/src/processor/stackwalker.cc
@@ -45,6 +45,7 @@
#include "google_breakpad/processor/stack_frame.h"
#include "google_breakpad/processor/symbol_supplier.h"
#include "processor/linked_ptr.h"
+#include "processor/logging.h"
#include "processor/scoped_ptr.h"
#include "processor/stack_frame_info.h"
#include "processor/stackwalker_ppc.h"
@@ -67,6 +68,7 @@ Stackwalker::Stackwalker(const SystemInfo *system_info,
bool Stackwalker::Walk(CallStack *stack) {
+ BPLOG_IF(ERROR, !stack) << "Stackwalker::Walk requires |stack|";
assert(stack);
stack->Clear();
@@ -139,8 +141,10 @@ Stackwalker* Stackwalker::StackwalkerForCPU(
const CodeModules *modules,
SymbolSupplier *supplier,
SourceLineResolverInterface *resolver) {
- if (!context)
+ if (!context) {
+ BPLOG(ERROR) << "Can't choose a stackwalker implementation without context";
return NULL;
+ }
Stackwalker *cpu_stackwalker = NULL;
@@ -161,6 +165,9 @@ Stackwalker* Stackwalker::StackwalkerForCPU(
break;
}
+ BPLOG_IF(ERROR, !cpu_stackwalker) << "Unknown CPU type " << HexString(cpu) <<
+ ", can't choose a stackwalker "
+ "implementation";
return cpu_stackwalker;
}
diff --git a/src/processor/stackwalker_ppc.cc b/src/processor/stackwalker_ppc.cc
index 044ae99b..02a49b21 100644
--- a/src/processor/stackwalker_ppc.cc
+++ b/src/processor/stackwalker_ppc.cc
@@ -38,6 +38,7 @@
#include "google_breakpad/processor/call_stack.h"
#include "google_breakpad/processor/memory_region.h"
#include "google_breakpad/processor/stack_frame_cpu.h"
+#include "processor/logging.h"
namespace google_breakpad {
@@ -54,14 +55,19 @@ StackwalkerPPC::StackwalkerPPC(const SystemInfo *system_info,
// This implementation only covers 32-bit ppc CPUs. The limits of the
// supplied stack are invalid. Mark memory_ = NULL, which will cause
// stackwalking to fail.
+ BPLOG(ERROR) << "Memory out of range for stackwalking: " <<
+ HexString(memory_->GetBase()) << "+" <<
+ HexString(memory_->GetSize());
memory_ = NULL;
}
}
StackFrame* StackwalkerPPC::GetContextFrame() {
- if (!context_ || !memory_)
+ if (!context_ || !memory_) {
+ BPLOG(ERROR) << "Can't get context frame without context or memory";
return NULL;
+ }
StackFramePPC *frame = new StackFramePPC();
@@ -78,8 +84,10 @@ StackFrame* StackwalkerPPC::GetContextFrame() {
StackFrame* StackwalkerPPC::GetCallerFrame(
const CallStack *stack,
const vector< linked_ptr<StackFrameInfo> > &stack_frame_info) {
- if (!memory_ || !stack)
+ if (!memory_ || !stack) {
+ BPLOG(ERROR) << "Can't get caller frame without memory or stack";
return NULL;
+ }
// The instruction pointers for previous frames are saved on the stack.
// The typical ppc calling convention is for the called procedure to store
diff --git a/src/processor/stackwalker_x86.cc b/src/processor/stackwalker_x86.cc
index 59662cdf..e39df17f 100644
--- a/src/processor/stackwalker_x86.cc
+++ b/src/processor/stackwalker_x86.cc
@@ -42,6 +42,7 @@
#include "google_breakpad/processor/memory_region.h"
#include "google_breakpad/processor/stack_frame_cpu.h"
#include "processor/linked_ptr.h"
+#include "processor/logging.h"
#include "processor/stack_frame_info.h"
namespace google_breakpad {
@@ -58,14 +59,19 @@ StackwalkerX86::StackwalkerX86(const SystemInfo *system_info,
if (memory_->GetBase() + memory_->GetSize() - 1 > 0xffffffff) {
// The x86 is a 32-bit CPU, the limits of the supplied stack are invalid.
// Mark memory_ = NULL, which will cause stackwalking to fail.
+ BPLOG(ERROR) << "Memory out of range for stackwalking: " <<
+ HexString(memory_->GetBase()) << "+" <<
+ HexString(memory_->GetSize());
memory_ = NULL;
}
}
StackFrame* StackwalkerX86::GetContextFrame() {
- if (!context_ || !memory_)
+ if (!context_ || !memory_) {
+ BPLOG(ERROR) << "Can't get context frame without context or memory";
return NULL;
+ }
StackFrameX86 *frame = new StackFrameX86();
@@ -82,8 +88,10 @@ StackFrame* StackwalkerX86::GetContextFrame() {
StackFrame* StackwalkerX86::GetCallerFrame(
const CallStack *stack,
const vector< linked_ptr<StackFrameInfo> > &stack_frame_info) {
- if (!memory_ || !stack)
+ if (!memory_ || !stack) {
+ BPLOG(ERROR) << "Can't get caller frame without memory or stack";
return NULL;
+ }
StackFrameX86 *last_frame = static_cast<StackFrameX86*>(
stack->frames()->back());
generated by cgit v1.2.1 (git 2.18.0) at 2024-11-05 00:46:45 +0000