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>

1 files changed, 30 insertions, 8 deletions

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[]) {