Bug 1683232 - Handle non-fixed size AMD64 and x86 contexts when processing minidumps r=KrisWright
authorGabriele Svelto <gsvelto@mozilla.com>
Wed, 06 Jan 2021 19:19:21 +0000
changeset 562194 9c7eb4cade8b0f0568786f4b163c50e73d1956fb
parent 562193 a428dc6c16ddde993452912253c9e3cc62176949
child 562195 c39478cd3068e1d9bf1b8c835d3f48db58cca067
push id133651
push usergsvelto@mozilla.com
push dateWed, 06 Jan 2021 19:21:53 +0000
treeherderautoland@9c7eb4cade8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersKrisWright
bugs1683232
milestone86.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1683232 - Handle non-fixed size AMD64 and x86 contexts when processing minidumps r=KrisWright Depends on D99882 Differential Revision: https://phabricator.services.mozilla.com/D100324
toolkit/crashreporter/breakpad-patches/14-handle-non-fixed-size-amd64-and-x86-contexts.patch
toolkit/crashreporter/google-breakpad/src/processor/minidump.cc
new file mode 100644
--- /dev/null
+++ b/toolkit/crashreporter/breakpad-patches/14-handle-non-fixed-size-amd64-and-x86-contexts.patch
@@ -0,0 +1,344 @@
+diff --git a/src/processor/minidump.cc b/src/processor/minidump.cc
+index 1f479558..aa63fe47 100644
+--- a/src/processor/minidump.cc
++++ b/src/processor/minidump.cc
+@@ -444,140 +444,30 @@ MinidumpContext::MinidumpContext(Minidump* minidump)
+ 
+ MinidumpContext::~MinidumpContext() {
+ }
+ 
+ bool MinidumpContext::Read(uint32_t expected_size) {
+   valid_ = false;
+ 
+   // Certain raw context types are currently assumed to have unique sizes.
+-  if (!IsContextSizeUnique(sizeof(MDRawContextAMD64))) {
+-    BPLOG(ERROR) << "sizeof(MDRawContextAMD64) cannot match the size of any "
+-                 << "other raw context";
+-    return false;
+-  }
+   if (!IsContextSizeUnique(sizeof(MDRawContextPPC64))) {
+     BPLOG(ERROR) << "sizeof(MDRawContextPPC64) cannot match the size of any "
+                  << "other raw context";
+     return false;
+   }
+   if (!IsContextSizeUnique(sizeof(MDRawContextARM64_Old))) {
+     BPLOG(ERROR) << "sizeof(MDRawContextARM64_Old) cannot match the size of any "
+                  << "other raw context";
+     return false;
+   }
+ 
+   FreeContext();
+ 
+-  // First, figure out what type of CPU this context structure is for.
+-  // For some reason, the AMD64 Context doesn't have context_flags
+-  // at the beginning of the structure, so special case it here.
+-  if (expected_size == sizeof(MDRawContextAMD64)) {
+-    BPLOG(INFO) << "MinidumpContext: looks like AMD64 context";
+-
+-    scoped_ptr<MDRawContextAMD64> context_amd64(new MDRawContextAMD64());
+-    if (!minidump_->ReadBytes(context_amd64.get(),
+-                              sizeof(MDRawContextAMD64))) {
+-      BPLOG(ERROR) << "MinidumpContext could not read amd64 context";
+-      return false;
+-    }
+-
+-    if (minidump_->swap())
+-      Swap(&context_amd64->context_flags);
+-
+-    uint32_t cpu_type = context_amd64->context_flags & MD_CONTEXT_CPU_MASK;
+-    if (cpu_type == 0) {
+-      if (minidump_->GetContextCPUFlagsFromSystemInfo(&cpu_type)) {
+-        context_amd64->context_flags |= cpu_type;
+-      } else {
+-        BPLOG(ERROR) << "Failed to preserve the current stream position";
+-        return false;
+-      }
+-    }
+-
+-    if (cpu_type != MD_CONTEXT_AMD64) {
+-      // TODO: Fall through to switch below.
+-      // https://bugs.chromium.org/p/google-breakpad/issues/detail?id=550
+-      BPLOG(ERROR) << "MinidumpContext not actually amd64 context";
+-      return false;
+-    }
+-
+-    // Do this after reading the entire MDRawContext structure because
+-    // GetSystemInfo may seek minidump to a new position.
+-    if (!CheckAgainstSystemInfo(cpu_type)) {
+-      BPLOG(ERROR) << "MinidumpContext amd64 does not match system info";
+-      return false;
+-    }
+-
+-    // Normalize the 128-bit types in the dump.
+-    // Since this is AMD64, by definition, the values are little-endian.
+-    for (unsigned int vr_index = 0;
+-         vr_index < MD_CONTEXT_AMD64_VR_COUNT;
+-         ++vr_index)
+-      Normalize128(&context_amd64->vector_register[vr_index], false);
+-
+-    if (minidump_->swap()) {
+-      Swap(&context_amd64->p1_home);
+-      Swap(&context_amd64->p2_home);
+-      Swap(&context_amd64->p3_home);
+-      Swap(&context_amd64->p4_home);
+-      Swap(&context_amd64->p5_home);
+-      Swap(&context_amd64->p6_home);
+-      // context_flags is already swapped
+-      Swap(&context_amd64->mx_csr);
+-      Swap(&context_amd64->cs);
+-      Swap(&context_amd64->ds);
+-      Swap(&context_amd64->es);
+-      Swap(&context_amd64->fs);
+-      Swap(&context_amd64->ss);
+-      Swap(&context_amd64->eflags);
+-      Swap(&context_amd64->dr0);
+-      Swap(&context_amd64->dr1);
+-      Swap(&context_amd64->dr2);
+-      Swap(&context_amd64->dr3);
+-      Swap(&context_amd64->dr6);
+-      Swap(&context_amd64->dr7);
+-      Swap(&context_amd64->rax);
+-      Swap(&context_amd64->rcx);
+-      Swap(&context_amd64->rdx);
+-      Swap(&context_amd64->rbx);
+-      Swap(&context_amd64->rsp);
+-      Swap(&context_amd64->rbp);
+-      Swap(&context_amd64->rsi);
+-      Swap(&context_amd64->rdi);
+-      Swap(&context_amd64->r8);
+-      Swap(&context_amd64->r9);
+-      Swap(&context_amd64->r10);
+-      Swap(&context_amd64->r11);
+-      Swap(&context_amd64->r12);
+-      Swap(&context_amd64->r13);
+-      Swap(&context_amd64->r14);
+-      Swap(&context_amd64->r15);
+-      Swap(&context_amd64->rip);
+-      // FIXME: I'm not sure what actually determines
+-      // which member of the union {flt_save, sse_registers}
+-      // is valid.  We're not currently using either,
+-      // but it would be good to have them swapped properly.
+-
+-      for (unsigned int vr_index = 0;
+-           vr_index < MD_CONTEXT_AMD64_VR_COUNT;
+-           ++vr_index)
+-        Swap(&context_amd64->vector_register[vr_index]);
+-      Swap(&context_amd64->vector_control);
+-      Swap(&context_amd64->debug_control);
+-      Swap(&context_amd64->last_branch_to_rip);
+-      Swap(&context_amd64->last_branch_from_rip);
+-      Swap(&context_amd64->last_exception_to_rip);
+-      Swap(&context_amd64->last_exception_from_rip);
+-    }
+-
+-    SetContextFlags(context_amd64->context_flags);
+-
+-    SetContextAMD64(context_amd64.release());
+-  } else if (expected_size == sizeof(MDRawContextPPC64)) {
++  if (expected_size == sizeof(MDRawContextPPC64)) {
+     // |context_flags| of MDRawContextPPC64 is 64 bits, but other MDRawContext
+     // in the else case have 32 bits |context_flags|, so special case it here.
+     uint64_t context_flags;
+     if (!minidump_->ReadBytes(&context_flags, sizeof(context_flags))) {
+       BPLOG(ERROR) << "MinidumpContext could not read context flags";
+       return false;
+     }
+     if (minidump_->swap())
+@@ -739,56 +629,152 @@ bool MinidumpContext::Read(uint32_t expected_size) {
+       }
+     }
+ 
+     scoped_ptr<MDRawContextARM64> new_context(new MDRawContextARM64());
+     ConvertOldARM64Context(*context_arm64.get(), new_context.get());
+     SetContextFlags(new_context->context_flags);
+     SetContextARM64(new_context.release());
+   } else {
+-    uint32_t context_flags;
+-    if (!minidump_->ReadBytes(&context_flags, sizeof(context_flags))) {
+-      BPLOG(ERROR) << "MinidumpContext could not read context flags";
++    uint32_t cpu_type = 0;
++    if (!minidump_->GetContextCPUFlagsFromSystemInfo(&cpu_type)) {
++      BPLOG(ERROR) << "Failed to preserve the current stream position";
+       return false;
+     }
+-    if (minidump_->swap())
+-      Swap(&context_flags);
+ 
+-    uint32_t cpu_type = context_flags & MD_CONTEXT_CPU_MASK;
+-    if (cpu_type == 0) {
+-      // Unfortunately the flag for MD_CONTEXT_ARM that was taken
+-      // from a Windows CE SDK header conflicts in practice with
+-      // the CONTEXT_XSTATE flag. MD_CONTEXT_ARM has been renumbered,
+-      // but handle dumps with the legacy value gracefully here.
+-      if (context_flags & MD_CONTEXT_ARM_OLD) {
+-        context_flags |= MD_CONTEXT_ARM;
+-        context_flags &= ~MD_CONTEXT_ARM_OLD;
+-        cpu_type = MD_CONTEXT_ARM;
++    uint32_t context_flags = 0;
++    if ((cpu_type == 0) || cpu_type != MD_CONTEXT_AMD64) {
++      if (!minidump_->ReadBytes(&context_flags, sizeof(context_flags))) {
++        BPLOG(ERROR) << "MinidumpContext could not read context flags";
++        return false;
+       }
+-    }
+ 
+-    if (cpu_type == 0) {
+-      if (minidump_->GetContextCPUFlagsFromSystemInfo(&cpu_type)) {
+-        context_flags |= cpu_type;
++      if (minidump_->swap())
++        Swap(&context_flags);
++
++      if ((context_flags & MD_CONTEXT_CPU_MASK) == 0) {
++        // Unfortunately the flag for MD_CONTEXT_ARM that was taken
++        // from a Windows CE SDK header conflicts in practice with
++        // the CONTEXT_XSTATE flag. MD_CONTEXT_ARM has been renumbered,
++        // but handle dumps with the legacy value gracefully here.
++        if (context_flags & MD_CONTEXT_ARM_OLD) {
++          context_flags |= MD_CONTEXT_ARM;
++          context_flags &= ~MD_CONTEXT_ARM_OLD;
++          cpu_type = MD_CONTEXT_ARM;
++        } else {
++          context_flags |= cpu_type;
++        }
+       } else {
+-        BPLOG(ERROR) << "Failed to preserve the current stream position";
+-        return false;
++        cpu_type = context_flags & MD_CONTEXT_CPU_MASK;
+       }
+     }
+ 
+     // Allocate the context structure for the correct CPU and fill it.  The
+     // casts are slightly unorthodox, but it seems better to do that than to
+     // maintain a separate pointer for each type of CPU context structure
+     // when only one of them will be used.
+     switch (cpu_type) {
++      case MD_CONTEXT_AMD64: {
++        if (expected_size != sizeof(MDRawContextAMD64)) {
++          BPLOG(INFO) << "MinidumpContext AMD64 size mismatch, " <<
++            expected_size << " != " << sizeof(MDRawContextAMD64);
++        }
++        BPLOG(INFO) << "MinidumpContext: looks like AMD64 context";
++
++        scoped_ptr<MDRawContextAMD64> context_amd64(new MDRawContextAMD64());
++        if (!minidump_->ReadBytes(context_amd64.get(),
++                                  sizeof(MDRawContextAMD64))) {
++          BPLOG(ERROR) << "MinidumpContext could not read amd64 context";
++          return false;
++        }
++
++        if (minidump_->swap())
++          Swap(&context_amd64->context_flags);
++
++        // Update context_flags since we haven't done it yet
++        context_flags = context_amd64->context_flags;
++
++        if (cpu_type != (context_flags & MD_CONTEXT_CPU_MASK)) {
++          BPLOG(ERROR) << "MinidumpContext amd64 does not match system info";
++          return false;
++        }
++
++        // Normalize the 128-bit types in the dump.
++        // Since this is AMD64, by definition, the values are little-endian.
++        for (unsigned int vr_index = 0;
++             vr_index < MD_CONTEXT_AMD64_VR_COUNT;
++             ++vr_index)
++          Normalize128(&context_amd64->vector_register[vr_index], false);
++
++        if (minidump_->swap()) {
++          Swap(&context_amd64->p1_home);
++          Swap(&context_amd64->p2_home);
++          Swap(&context_amd64->p3_home);
++          Swap(&context_amd64->p4_home);
++          Swap(&context_amd64->p5_home);
++          Swap(&context_amd64->p6_home);
++          // context_flags is already swapped
++          Swap(&context_amd64->mx_csr);
++          Swap(&context_amd64->cs);
++          Swap(&context_amd64->ds);
++          Swap(&context_amd64->es);
++          Swap(&context_amd64->fs);
++          Swap(&context_amd64->ss);
++          Swap(&context_amd64->eflags);
++          Swap(&context_amd64->dr0);
++          Swap(&context_amd64->dr1);
++          Swap(&context_amd64->dr2);
++          Swap(&context_amd64->dr3);
++          Swap(&context_amd64->dr6);
++          Swap(&context_amd64->dr7);
++          Swap(&context_amd64->rax);
++          Swap(&context_amd64->rcx);
++          Swap(&context_amd64->rdx);
++          Swap(&context_amd64->rbx);
++          Swap(&context_amd64->rsp);
++          Swap(&context_amd64->rbp);
++          Swap(&context_amd64->rsi);
++          Swap(&context_amd64->rdi);
++          Swap(&context_amd64->r8);
++          Swap(&context_amd64->r9);
++          Swap(&context_amd64->r10);
++          Swap(&context_amd64->r11);
++          Swap(&context_amd64->r12);
++          Swap(&context_amd64->r13);
++          Swap(&context_amd64->r14);
++          Swap(&context_amd64->r15);
++          Swap(&context_amd64->rip);
++          // FIXME: I'm not sure what actually determines
++          // which member of the union {flt_save, sse_registers}
++          // is valid.  We're not currently using either,
++          // but it would be good to have them swapped properly.
++
++          for (unsigned int vr_index = 0;
++               vr_index < MD_CONTEXT_AMD64_VR_COUNT;
++               ++vr_index)
++            Swap(&context_amd64->vector_register[vr_index]);
++          Swap(&context_amd64->vector_control);
++          Swap(&context_amd64->debug_control);
++          Swap(&context_amd64->last_branch_to_rip);
++          Swap(&context_amd64->last_branch_from_rip);
++          Swap(&context_amd64->last_exception_to_rip);
++          Swap(&context_amd64->last_exception_from_rip);
++        }
++
++        SetContextFlags(context_amd64->context_flags);
++
++        SetContextAMD64(context_amd64.release());
++        minidump_->SeekSet(
++          (minidump_->Tell() - sizeof(MDRawContextAMD64)) + expected_size);
++        break;
++      }
+       case MD_CONTEXT_X86: {
+         if (expected_size != sizeof(MDRawContextX86)) {
+-          BPLOG(ERROR) << "MinidumpContext x86 size mismatch, " <<
++          BPLOG(INFO) << "MinidumpContext x86 size mismatch, " <<
+             expected_size << " != " << sizeof(MDRawContextX86);
+-          return false;
+         }
+ 
+         scoped_ptr<MDRawContextX86> context_x86(new MDRawContextX86());
+ 
+         // Set the context_flags member, which has already been read, and
+         // read the rest of the structure beginning with the first member
+         // after context_flags.
+         context_x86->context_flags = context_flags;
+@@ -843,16 +829,18 @@ bool MinidumpContext::Read(uint32_t expected_size) {
+           Swap(&context_x86->eflags);
+           Swap(&context_x86->esp);
+           Swap(&context_x86->ss);
+           // context_x86->extended_registers[] contains 8-bit quantities and
+           // does not need to be swapped.
+         }
+ 
+         SetContextX86(context_x86.release());
++        minidump_->SeekSet(
++          (minidump_->Tell() - sizeof(MDRawContextX86)) + expected_size);
+ 
+         break;
+       }
+ 
+       case MD_CONTEXT_PPC: {
+         if (expected_size != sizeof(MDRawContextPPC)) {
+           BPLOG(ERROR) << "MinidumpContext ppc size mismatch, " <<
+             expected_size << " != " << sizeof(MDRawContextPPC);
+-- 
+2.26.2
+
--- a/toolkit/crashreporter/google-breakpad/src/processor/minidump.cc
+++ b/toolkit/crashreporter/google-breakpad/src/processor/minidump.cc
@@ -444,140 +444,30 @@ MinidumpContext::MinidumpContext(Minidum
 
 MinidumpContext::~MinidumpContext() {
 }
 
 bool MinidumpContext::Read(uint32_t expected_size) {
   valid_ = false;
 
   // Certain raw context types are currently assumed to have unique sizes.
-  if (!IsContextSizeUnique(sizeof(MDRawContextAMD64))) {
-    BPLOG(ERROR) << "sizeof(MDRawContextAMD64) cannot match the size of any "
-                 << "other raw context";
-    return false;
-  }
   if (!IsContextSizeUnique(sizeof(MDRawContextPPC64))) {
     BPLOG(ERROR) << "sizeof(MDRawContextPPC64) cannot match the size of any "
                  << "other raw context";
     return false;
   }
   if (!IsContextSizeUnique(sizeof(MDRawContextARM64_Old))) {
     BPLOG(ERROR) << "sizeof(MDRawContextARM64_Old) cannot match the size of any "
                  << "other raw context";
     return false;
   }
 
   FreeContext();
 
-  // First, figure out what type of CPU this context structure is for.
-  // For some reason, the AMD64 Context doesn't have context_flags
-  // at the beginning of the structure, so special case it here.
-  if (expected_size == sizeof(MDRawContextAMD64)) {
-    BPLOG(INFO) << "MinidumpContext: looks like AMD64 context";
-
-    scoped_ptr<MDRawContextAMD64> context_amd64(new MDRawContextAMD64());
-    if (!minidump_->ReadBytes(context_amd64.get(),
-                              sizeof(MDRawContextAMD64))) {
-      BPLOG(ERROR) << "MinidumpContext could not read amd64 context";
-      return false;
-    }
-
-    if (minidump_->swap())
-      Swap(&context_amd64->context_flags);
-
-    uint32_t cpu_type = context_amd64->context_flags & MD_CONTEXT_CPU_MASK;
-    if (cpu_type == 0) {
-      if (minidump_->GetContextCPUFlagsFromSystemInfo(&cpu_type)) {
-        context_amd64->context_flags |= cpu_type;
-      } else {
-        BPLOG(ERROR) << "Failed to preserve the current stream position";
-        return false;
-      }
-    }
-
-    if (cpu_type != MD_CONTEXT_AMD64) {
-      // TODO: Fall through to switch below.
-      // https://bugs.chromium.org/p/google-breakpad/issues/detail?id=550
-      BPLOG(ERROR) << "MinidumpContext not actually amd64 context";
-      return false;
-    }
-
-    // Do this after reading the entire MDRawContext structure because
-    // GetSystemInfo may seek minidump to a new position.
-    if (!CheckAgainstSystemInfo(cpu_type)) {
-      BPLOG(ERROR) << "MinidumpContext amd64 does not match system info";
-      return false;
-    }
-
-    // Normalize the 128-bit types in the dump.
-    // Since this is AMD64, by definition, the values are little-endian.
-    for (unsigned int vr_index = 0;
-         vr_index < MD_CONTEXT_AMD64_VR_COUNT;
-         ++vr_index)
-      Normalize128(&context_amd64->vector_register[vr_index], false);
-
-    if (minidump_->swap()) {
-      Swap(&context_amd64->p1_home);
-      Swap(&context_amd64->p2_home);
-      Swap(&context_amd64->p3_home);
-      Swap(&context_amd64->p4_home);
-      Swap(&context_amd64->p5_home);
-      Swap(&context_amd64->p6_home);
-      // context_flags is already swapped
-      Swap(&context_amd64->mx_csr);
-      Swap(&context_amd64->cs);
-      Swap(&context_amd64->ds);
-      Swap(&context_amd64->es);
-      Swap(&context_amd64->fs);
-      Swap(&context_amd64->ss);
-      Swap(&context_amd64->eflags);
-      Swap(&context_amd64->dr0);
-      Swap(&context_amd64->dr1);
-      Swap(&context_amd64->dr2);
-      Swap(&context_amd64->dr3);
-      Swap(&context_amd64->dr6);
-      Swap(&context_amd64->dr7);
-      Swap(&context_amd64->rax);
-      Swap(&context_amd64->rcx);
-      Swap(&context_amd64->rdx);
-      Swap(&context_amd64->rbx);
-      Swap(&context_amd64->rsp);
-      Swap(&context_amd64->rbp);
-      Swap(&context_amd64->rsi);
-      Swap(&context_amd64->rdi);
-      Swap(&context_amd64->r8);
-      Swap(&context_amd64->r9);
-      Swap(&context_amd64->r10);
-      Swap(&context_amd64->r11);
-      Swap(&context_amd64->r12);
-      Swap(&context_amd64->r13);
-      Swap(&context_amd64->r14);
-      Swap(&context_amd64->r15);
-      Swap(&context_amd64->rip);
-      // FIXME: I'm not sure what actually determines
-      // which member of the union {flt_save, sse_registers}
-      // is valid.  We're not currently using either,
-      // but it would be good to have them swapped properly.
-
-      for (unsigned int vr_index = 0;
-           vr_index < MD_CONTEXT_AMD64_VR_COUNT;
-           ++vr_index)
-        Swap(&context_amd64->vector_register[vr_index]);
-      Swap(&context_amd64->vector_control);
-      Swap(&context_amd64->debug_control);
-      Swap(&context_amd64->last_branch_to_rip);
-      Swap(&context_amd64->last_branch_from_rip);
-      Swap(&context_amd64->last_exception_to_rip);
-      Swap(&context_amd64->last_exception_from_rip);
-    }
-
-    SetContextFlags(context_amd64->context_flags);
-
-    SetContextAMD64(context_amd64.release());
-  } else if (expected_size == sizeof(MDRawContextPPC64)) {
+  if (expected_size == sizeof(MDRawContextPPC64)) {
     // |context_flags| of MDRawContextPPC64 is 64 bits, but other MDRawContext
     // in the else case have 32 bits |context_flags|, so special case it here.
     uint64_t context_flags;
     if (!minidump_->ReadBytes(&context_flags, sizeof(context_flags))) {
       BPLOG(ERROR) << "MinidumpContext could not read context flags";
       return false;
     }
     if (minidump_->swap())
@@ -739,56 +629,152 @@ bool MinidumpContext::Read(uint32_t expe
       }
     }
 
     scoped_ptr<MDRawContextARM64> new_context(new MDRawContextARM64());
     ConvertOldARM64Context(*context_arm64.get(), new_context.get());
     SetContextFlags(new_context->context_flags);
     SetContextARM64(new_context.release());
   } else {
-    uint32_t context_flags;
-    if (!minidump_->ReadBytes(&context_flags, sizeof(context_flags))) {
-      BPLOG(ERROR) << "MinidumpContext could not read context flags";
+    uint32_t cpu_type = 0;
+    if (!minidump_->GetContextCPUFlagsFromSystemInfo(&cpu_type)) {
+      BPLOG(ERROR) << "Failed to preserve the current stream position";
       return false;
     }
-    if (minidump_->swap())
-      Swap(&context_flags);
-
-    uint32_t cpu_type = context_flags & MD_CONTEXT_CPU_MASK;
-    if (cpu_type == 0) {
-      // Unfortunately the flag for MD_CONTEXT_ARM that was taken
-      // from a Windows CE SDK header conflicts in practice with
-      // the CONTEXT_XSTATE flag. MD_CONTEXT_ARM has been renumbered,
-      // but handle dumps with the legacy value gracefully here.
-      if (context_flags & MD_CONTEXT_ARM_OLD) {
-        context_flags |= MD_CONTEXT_ARM;
-        context_flags &= ~MD_CONTEXT_ARM_OLD;
-        cpu_type = MD_CONTEXT_ARM;
+
+    uint32_t context_flags = 0;
+    if ((cpu_type == 0) || cpu_type != MD_CONTEXT_AMD64) {
+      if (!minidump_->ReadBytes(&context_flags, sizeof(context_flags))) {
+        BPLOG(ERROR) << "MinidumpContext could not read context flags";
+        return false;
       }
-    }
-
-    if (cpu_type == 0) {
-      if (minidump_->GetContextCPUFlagsFromSystemInfo(&cpu_type)) {
-        context_flags |= cpu_type;
+
+      if (minidump_->swap())
+        Swap(&context_flags);
+
+      if ((context_flags & MD_CONTEXT_CPU_MASK) == 0) {
+        // Unfortunately the flag for MD_CONTEXT_ARM that was taken
+        // from a Windows CE SDK header conflicts in practice with
+        // the CONTEXT_XSTATE flag. MD_CONTEXT_ARM has been renumbered,
+        // but handle dumps with the legacy value gracefully here.
+        if (context_flags & MD_CONTEXT_ARM_OLD) {
+          context_flags |= MD_CONTEXT_ARM;
+          context_flags &= ~MD_CONTEXT_ARM_OLD;
+          cpu_type = MD_CONTEXT_ARM;
+        } else {
+          context_flags |= cpu_type;
+        }
       } else {
-        BPLOG(ERROR) << "Failed to preserve the current stream position";
-        return false;
+        cpu_type = context_flags & MD_CONTEXT_CPU_MASK;
       }
     }
 
     // Allocate the context structure for the correct CPU and fill it.  The
     // casts are slightly unorthodox, but it seems better to do that than to
     // maintain a separate pointer for each type of CPU context structure
     // when only one of them will be used.
     switch (cpu_type) {
+      case MD_CONTEXT_AMD64: {
+        if (expected_size != sizeof(MDRawContextAMD64)) {
+          BPLOG(INFO) << "MinidumpContext AMD64 size mismatch, " <<
+            expected_size << " != " << sizeof(MDRawContextAMD64);
+        }
+        BPLOG(INFO) << "MinidumpContext: looks like AMD64 context";
+
+        scoped_ptr<MDRawContextAMD64> context_amd64(new MDRawContextAMD64());
+        if (!minidump_->ReadBytes(context_amd64.get(),
+                                  sizeof(MDRawContextAMD64))) {
+          BPLOG(ERROR) << "MinidumpContext could not read amd64 context";
+          return false;
+        }
+
+        if (minidump_->swap())
+          Swap(&context_amd64->context_flags);
+
+        // Update context_flags since we haven't done it yet
+        context_flags = context_amd64->context_flags;
+
+        if (cpu_type != (context_flags & MD_CONTEXT_CPU_MASK)) {
+          BPLOG(ERROR) << "MinidumpContext amd64 does not match system info";
+          return false;
+        }
+
+        // Normalize the 128-bit types in the dump.
+        // Since this is AMD64, by definition, the values are little-endian.
+        for (unsigned int vr_index = 0;
+             vr_index < MD_CONTEXT_AMD64_VR_COUNT;
+             ++vr_index)
+          Normalize128(&context_amd64->vector_register[vr_index], false);
+
+        if (minidump_->swap()) {
+          Swap(&context_amd64->p1_home);
+          Swap(&context_amd64->p2_home);
+          Swap(&context_amd64->p3_home);
+          Swap(&context_amd64->p4_home);
+          Swap(&context_amd64->p5_home);
+          Swap(&context_amd64->p6_home);
+          // context_flags is already swapped
+          Swap(&context_amd64->mx_csr);
+          Swap(&context_amd64->cs);
+          Swap(&context_amd64->ds);
+          Swap(&context_amd64->es);
+          Swap(&context_amd64->fs);
+          Swap(&context_amd64->ss);
+          Swap(&context_amd64->eflags);
+          Swap(&context_amd64->dr0);
+          Swap(&context_amd64->dr1);
+          Swap(&context_amd64->dr2);
+          Swap(&context_amd64->dr3);
+          Swap(&context_amd64->dr6);
+          Swap(&context_amd64->dr7);
+          Swap(&context_amd64->rax);
+          Swap(&context_amd64->rcx);
+          Swap(&context_amd64->rdx);
+          Swap(&context_amd64->rbx);
+          Swap(&context_amd64->rsp);
+          Swap(&context_amd64->rbp);
+          Swap(&context_amd64->rsi);
+          Swap(&context_amd64->rdi);
+          Swap(&context_amd64->r8);
+          Swap(&context_amd64->r9);
+          Swap(&context_amd64->r10);
+          Swap(&context_amd64->r11);
+          Swap(&context_amd64->r12);
+          Swap(&context_amd64->r13);
+          Swap(&context_amd64->r14);
+          Swap(&context_amd64->r15);
+          Swap(&context_amd64->rip);
+          // FIXME: I'm not sure what actually determines
+          // which member of the union {flt_save, sse_registers}
+          // is valid.  We're not currently using either,
+          // but it would be good to have them swapped properly.
+
+          for (unsigned int vr_index = 0;
+               vr_index < MD_CONTEXT_AMD64_VR_COUNT;
+               ++vr_index)
+            Swap(&context_amd64->vector_register[vr_index]);
+          Swap(&context_amd64->vector_control);
+          Swap(&context_amd64->debug_control);
+          Swap(&context_amd64->last_branch_to_rip);
+          Swap(&context_amd64->last_branch_from_rip);
+          Swap(&context_amd64->last_exception_to_rip);
+          Swap(&context_amd64->last_exception_from_rip);
+        }
+
+        SetContextFlags(context_amd64->context_flags);
+
+        SetContextAMD64(context_amd64.release());
+        minidump_->SeekSet(
+          (minidump_->Tell() - sizeof(MDRawContextAMD64)) + expected_size);
+        break;
+      }
       case MD_CONTEXT_X86: {
         if (expected_size != sizeof(MDRawContextX86)) {
-          BPLOG(ERROR) << "MinidumpContext x86 size mismatch, " <<
+          BPLOG(INFO) << "MinidumpContext x86 size mismatch, " <<
             expected_size << " != " << sizeof(MDRawContextX86);
-          return false;
         }
 
         scoped_ptr<MDRawContextX86> context_x86(new MDRawContextX86());
 
         // Set the context_flags member, which has already been read, and
         // read the rest of the structure beginning with the first member
         // after context_flags.
         context_x86->context_flags = context_flags;
@@ -843,16 +829,18 @@ bool MinidumpContext::Read(uint32_t expe
           Swap(&context_x86->eflags);
           Swap(&context_x86->esp);
           Swap(&context_x86->ss);
           // context_x86->extended_registers[] contains 8-bit quantities and
           // does not need to be swapped.
         }
 
         SetContextX86(context_x86.release());
+        minidump_->SeekSet(
+          (minidump_->Tell() - sizeof(MDRawContextX86)) + expected_size);
 
         break;
       }
 
       case MD_CONTEXT_PPC: {
         if (expected_size != sizeof(MDRawContextPPC)) {
           BPLOG(ERROR) << "MinidumpContext ppc size mismatch, " <<
             expected_size << " != " << sizeof(MDRawContextPPC);