aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGabriele Svelto <gsvelto@mozilla.com>2019-01-30 09:26:07 +0100
committerGabriele Svelto <gsvelto@mozilla.com>2019-01-30 16:21:52 +0000
commit44384d80b32a5bb361c2ec3bee667f7ccee566d7 (patch)
tree643111d36c6065d8f8045da25a60939a21724ef5
parentMake the minidump processor correctly populate the CPU information string for... (diff)
downloadbreakpad-44384d80b32a5bb361c2ec3bee667f7ccee566d7.tar.xz
Always emit a 32-bit crash address for 32-bit architectures
Certain minidumps for 32-bit crashes have the upper 32-bit of the crash address (which is a 64-bit value) set to non-zero values. This caused a crash address with more than 32-bits to be printed out for minidumps of 32-bit architectures. This patch masks out those bits when reading the raw minidump data to ensure this doesn't happen anymore. Bug: google-breakpad:783 Change-Id: Ieef6dff759fd0ee2efc47c4c4a3cf863a48f0659 Reviewed-on: https://chromium-review.googlesource.com/c/1427819 Reviewed-by: Ted Mielczarek <ted.mielczarek@gmail.com>
-rw-r--r--src/processor/minidump_processor.cc26
-rw-r--r--src/processor/minidump_processor_unittest.cc38
-rw-r--r--src/processor/testdata/minidump_32bit_crash_addr.dmpbin0 -> 11317 bytes
3 files changed, 56 insertions, 8 deletions
diff --git a/src/processor/minidump_processor.cc b/src/processor/minidump_processor.cc
index 2b40cf96..e3c3562c 100644
--- a/src/processor/minidump_processor.cc
+++ b/src/processor/minidump_processor.cc
@@ -355,6 +355,26 @@ static const MDRawSystemInfo* GetSystemInfo(Minidump *dump,
return minidump_system_info->system_info();
}
+static uint64_t GetAddressForArchitecture(const MDCPUArchitecture architecture,
+ size_t raw_address)
+{
+ switch (architecture) {
+ case MD_CPU_ARCHITECTURE_X86:
+ case MD_CPU_ARCHITECTURE_MIPS:
+ case MD_CPU_ARCHITECTURE_PPC:
+ case MD_CPU_ARCHITECTURE_SHX:
+ case MD_CPU_ARCHITECTURE_ARM:
+ case MD_CPU_ARCHITECTURE_X86_WIN64:
+ // 32-bit architectures, mask the upper bits.
+ return raw_address & 0xffffffffULL;
+
+ default:
+ // All other architectures either have 64-bit pointers or it's impossible
+ // to tell from the minidump (e.g. MSIL or SPARC) so use 64-bits anyway.
+ return raw_address;
+ }
+}
+
// Extract CPU info string from ARM-specific MDRawSystemInfo structure.
// raw_info: pointer to source MDRawSystemInfo.
// cpu_info: address of target string, cpu info text will be appended to it.
@@ -1637,6 +1657,12 @@ string MinidumpProcessor::GetCrashReason(Minidump *dump, uint64_t *address) {
}
}
+ if (address) {
+ *address = GetAddressForArchitecture(
+ static_cast<MDCPUArchitecture>(raw_system_info->processor_architecture),
+ *address);
+ }
+
return reason;
}
diff --git a/src/processor/minidump_processor_unittest.cc b/src/processor/minidump_processor_unittest.cc
index 83682a51..a4ac3685 100644
--- a/src/processor/minidump_processor_unittest.cc
+++ b/src/processor/minidump_processor_unittest.cc
@@ -203,6 +203,12 @@ static const char *kSystemInfoCPUInfo =
#define ASSERT_EQ_ABORT(e1, e2) ASSERT_TRUE_ABORT((e1) == (e2))
+static string GetTestDataPath() {
+ char *srcdir = getenv("srcdir");
+
+ return string(srcdir ? srcdir : ".") + "/src/processor/testdata/";
+}
+
class TestSymbolSupplier : public SymbolSupplier {
public:
TestSymbolSupplier() : interrupt_(false) {}
@@ -249,10 +255,8 @@ SymbolSupplier::SymbolResult TestSymbolSupplier::GetSymbolFile(
}
if (module && module->code_file() == "c:\\test_app.exe") {
- *symbol_file = string(getenv("srcdir") ? getenv("srcdir") : ".") +
- "/src/processor/testdata/symbols/test_app.pdb/" +
- module->debug_identifier() +
- "/test_app.sym";
+ *symbol_file = GetTestDataPath() + "symbols/test_app.pdb/" +
+ module->debug_identifier() + "/test_app.sym";
return FOUND;
}
@@ -460,8 +464,7 @@ TEST_F(MinidumpProcessorTest, TestSymbolSupplierLookupCounts) {
BasicSourceLineResolver resolver;
MinidumpProcessor processor(&supplier, &resolver);
- string minidump_file = string(getenv("srcdir") ? getenv("srcdir") : ".") +
- "/src/processor/testdata/minidump2.dmp";
+ string minidump_file = GetTestDataPath() + "minidump2.dmp";
ProcessState state;
EXPECT_CALL(supplier, GetCStringSymbolData(
Property(&google_breakpad::CodeModule::code_file,
@@ -501,8 +504,7 @@ TEST_F(MinidumpProcessorTest, TestBasicProcessing) {
BasicSourceLineResolver resolver;
MinidumpProcessor processor(&supplier, &resolver);
- string minidump_file = string(getenv("srcdir") ? getenv("srcdir") : ".") +
- "/src/processor/testdata/minidump2.dmp";
+ string minidump_file = GetTestDataPath() + "minidump2.dmp";
ProcessState state;
ASSERT_EQ(processor.Process(minidump_file, &state),
@@ -739,6 +741,26 @@ TEST_F(MinidumpProcessorTest, TestThreadMissingContext) {
ASSERT_EQ(0U, state.threads()->at(0)->frames()->size());
}
+TEST_F(MinidumpProcessorTest, Test32BitCrashingAddress) {
+ TestSymbolSupplier supplier;
+ BasicSourceLineResolver resolver;
+ MinidumpProcessor processor(&supplier, &resolver);
+
+ string minidump_file = GetTestDataPath() + "minidump_32bit_crash_addr.dmp";
+
+ ProcessState state;
+ ASSERT_EQ(processor.Process(minidump_file, &state),
+ google_breakpad::PROCESS_OK);
+ ASSERT_EQ(state.system_info()->os, kSystemInfoOS);
+ ASSERT_EQ(state.system_info()->os_short, kSystemInfoOSShort);
+ ASSERT_EQ(state.system_info()->os_version, kSystemInfoOSVersion);
+ ASSERT_EQ(state.system_info()->cpu, kSystemInfoCPU);
+ ASSERT_EQ(state.system_info()->cpu_info, kSystemInfoCPUInfo);
+ ASSERT_TRUE(state.crashed());
+ ASSERT_EQ(state.crash_reason(), "EXCEPTION_ACCESS_VIOLATION_WRITE");
+ ASSERT_EQ(state.crash_address(), 0x45U);
+}
+
} // namespace
int main(int argc, char *argv[]) {
diff --git a/src/processor/testdata/minidump_32bit_crash_addr.dmp b/src/processor/testdata/minidump_32bit_crash_addr.dmp
new file mode 100644
index 00000000..78becc6f
--- /dev/null
+++ b/src/processor/testdata/minidump_32bit_crash_addr.dmp
Binary files differ