ppc stackwalker (#30). r=bryner

- Implementation of PowerPC stackwalker.  Tested using stackwalker_selftest
   (#18).
 - Hook up processor-side multi-CPU support in MinidumpProcessor and
   minidump_stackwalk using the new Stackwalker::StackwalkerForCPU method.

http://groups.google.com/group/airbag-dev/browse_thread/thread/1c2fa7c5182a77a9


git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@34 4c0a9323-5329-0410-9bdc-e9ce6186880e

Makefile.am

@@ -70,6 +70,8 @@ src_libairbag_la_SOURCES = \
 	src/processor/source_line_resolver.h \
 	src/processor/stackwalker.cc \
 	src/processor/stackwalker.h \
+	src/processor/stackwalker_ppc.cc \
+	src/processor/stackwalker_ppc.h \
 	src/processor/stackwalker_x86.cc \
 	src/processor/stackwalker_x86.h
 
@@ -102,6 +104,7 @@ src_processor_minidump_processor_unittest_LDADD = \
 	src/processor/minidump_processor.lo \
 	src/processor/minidump.lo \
 	src/processor/stackwalker.lo \
+	src/processor/stackwalker_ppc.lo \
 	src/processor/stackwalker_x86.lo \
 	src/processor/source_line_resolver.lo
 
@@ -130,6 +133,7 @@ src_processor_minidump_stackwalk_SOURCES = \
 src_processor_minidump_stackwalk_LDADD = \
 	src/processor/minidump.lo \
 	src/processor/stackwalker.lo \
+	src/processor/stackwalker_ppc.lo \
 	src/processor/stackwalker_x86.lo \
 	src/processor/source_line_resolver.lo
 

Makefile.in

@@ -107,7 +107,8 @@ am__dirstamp = $(am__leading_dot)dirstamp
 am_src_libairbag_la_OBJECTS = src/processor/minidump.lo \
 	src/processor/minidump_processor.lo \
 	src/processor/source_line_resolver.lo \
-	src/processor/stackwalker.lo src/processor/stackwalker_x86.lo
+	src/processor/stackwalker.lo src/processor/stackwalker_ppc.lo \
+	src/processor/stackwalker_x86.lo
 src_libairbag_la_OBJECTS = $(am_src_libairbag_la_OBJECTS)
 binPROGRAMS_INSTALL = $(INSTALL_PROGRAM)
 PROGRAMS = $(bin_PROGRAMS) $(noinst_PROGRAMS)
@@ -127,7 +128,8 @@ src_processor_minidump_processor_unittest_OBJECTS =  \
 	$(am_src_processor_minidump_processor_unittest_OBJECTS)
 src_processor_minidump_processor_unittest_DEPENDENCIES =  \
 	src/processor/minidump_processor.lo src/processor/minidump.lo \
-	src/processor/stackwalker.lo src/processor/stackwalker_x86.lo \
+	src/processor/stackwalker.lo src/processor/stackwalker_ppc.lo \
+	src/processor/stackwalker_x86.lo \
 	src/processor/source_line_resolver.lo
 am_src_processor_minidump_stackwalk_OBJECTS =  \
 	src/processor/minidump_stackwalk.$(OBJEXT)
@@ -135,6 +137,7 @@ src_processor_minidump_stackwalk_OBJECTS =  \
 	$(am_src_processor_minidump_stackwalk_OBJECTS)
 src_processor_minidump_stackwalk_DEPENDENCIES =  \
 	src/processor/minidump.lo src/processor/stackwalker.lo \
+	src/processor/stackwalker_ppc.lo \
 	src/processor/stackwalker_x86.lo \
 	src/processor/source_line_resolver.lo
 am_src_processor_postfix_evaluator_unittest_OBJECTS =  \
@@ -339,6 +342,8 @@ src_libairbag_la_SOURCES = \
 	src/processor/source_line_resolver.h \
 	src/processor/stackwalker.cc \
 	src/processor/stackwalker.h \
+	src/processor/stackwalker_ppc.cc \
+	src/processor/stackwalker_ppc.h \
 	src/processor/stackwalker_x86.cc \
 	src/processor/stackwalker_x86.h
 
@@ -358,6 +363,7 @@ src_processor_minidump_processor_unittest_LDADD = \
 	src/processor/minidump_processor.lo \
 	src/processor/minidump.lo \
 	src/processor/stackwalker.lo \
+	src/processor/stackwalker_ppc.lo \
 	src/processor/stackwalker_x86.lo \
 	src/processor/source_line_resolver.lo
 
@@ -386,6 +392,7 @@ src_processor_minidump_stackwalk_SOURCES = \
 src_processor_minidump_stackwalk_LDADD = \
 	src/processor/minidump.lo \
 	src/processor/stackwalker.lo \
+	src/processor/stackwalker_ppc.lo \
 	src/processor/stackwalker_x86.lo \
 	src/processor/source_line_resolver.lo
 
@@ -495,6 +502,8 @@ src/processor/source_line_resolver.lo: src/processor/$(am__dirstamp) \
 	src/processor/$(DEPDIR)/$(am__dirstamp)
 src/processor/stackwalker.lo: src/processor/$(am__dirstamp) \
 	src/processor/$(DEPDIR)/$(am__dirstamp)
+src/processor/stackwalker_ppc.lo: src/processor/$(am__dirstamp) \
+	src/processor/$(DEPDIR)/$(am__dirstamp)
 src/processor/stackwalker_x86.lo: src/processor/$(am__dirstamp) \
 	src/processor/$(DEPDIR)/$(am__dirstamp)
 src/$(am__dirstamp):
@@ -603,6 +612,8 @@ mostlyclean-compile:
 	-rm -f src/processor/source_line_resolver_unittest.$(OBJEXT)
 	-rm -f src/processor/stackwalker.$(OBJEXT)
 	-rm -f src/processor/stackwalker.lo
+	-rm -f src/processor/stackwalker_ppc.$(OBJEXT)
+	-rm -f src/processor/stackwalker_ppc.lo
 	-rm -f src/processor/stackwalker_x86.$(OBJEXT)
 	-rm -f src/processor/stackwalker_x86.lo
 
@@ -620,6 +631,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@src/processor/$(DEPDIR)/source_line_resolver.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@src/processor/$(DEPDIR)/source_line_resolver_unittest.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@src/processor/$(DEPDIR)/stackwalker.Plo@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@src/processor/$(DEPDIR)/stackwalker_ppc.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@src/processor/$(DEPDIR)/stackwalker_x86.Plo@am__quote@
 
 .cc.o:

src/processor/minidump.cc

@@ -588,7 +588,7 @@ void MinidumpContext::Print() {
       for (unsigned int fpr_index = 0;
            fpr_index < MD_FLOATINGSAVEAREA_PPC_FPR_COUNT;
            ++fpr_index) {
-        printf("  float_save.fpregs[%2d]    = 0x%x\n",
+        printf("  float_save.fpregs[%2d]    = 0x%llx\n",
                fpr_index, context_ppc->float_save.fpregs[fpr_index]);
       }
       printf("  float_save.fpscr         = 0x%x\n",

src/processor/minidump_processor.cc

@@ -27,10 +27,14 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+#include <memory>
+
 #include "google/minidump_processor.h"
 #include "processor/minidump.h"
 #include "processor/stackwalker_x86.h"
 
+using std::auto_ptr;
+
 namespace google_airbag {
 
 MinidumpProcessor::MinidumpProcessor(SymbolSupplier *supplier)
@@ -68,10 +72,14 @@ bool MinidumpProcessor::Process(const string &minidump_file,
     return false;
   }
 
-  // TODO(bryner): figure out which StackWalker we want
+  auto_ptr<Stackwalker> walker(
-  StackwalkerX86 walker(exception->GetContext(), thread_memory,
+    Stackwalker::StackwalkerForCPU(exception->GetContext(), thread_memory,
-                        dump.GetModuleList(), supplier_);
+                                   dump.GetModuleList(), supplier_));
-  walker.Walk(stack_frames);
+  if (!walker.get()) {
+    return false;
+  }
+
+  walker->Walk(stack_frames);
   return true;
 }
 

src/processor/minidump_stackwalk.cc

@@ -35,14 +35,25 @@
 #include <stdlib.h>
 #include <stdio.h>
 
+#include <memory>
 #include <string>
 
 #include "processor/minidump.h"
 #include "processor/stackwalker_x86.h"
 
 
+using std::auto_ptr;
 using std::string;
-using namespace google_airbag;
+using google_airbag::MemoryRegion;
+using google_airbag::Minidump;
+using google_airbag::MinidumpContext;
+using google_airbag::MinidumpException;
+using google_airbag::MinidumpModuleList;
+using google_airbag::MinidumpThread;
+using google_airbag::MinidumpThreadList;
+using google_airbag::StackFrame;
+using google_airbag::StackFrames;
+using google_airbag::Stackwalker;
 
 
 int main(int argc, char **argv) {
@@ -94,11 +105,15 @@ int main(int argc, char** argv) {
     exit(1);
   }
 
-  StackwalkerX86 stackwalker = StackwalkerX86(context, stack_memory,
+  auto_ptr<Stackwalker> stackwalker(
-                                              modules, NULL);
+      Stackwalker::StackwalkerForCPU(context, stack_memory, modules, NULL));
+  if (!stackwalker.get()) {
+    fprintf(stderr, "Stackwalker::StackwalkerForCPU failed\n");
+    exit(1);
+  }
 
   StackFrames stack;
-  stackwalker.Walk(&stack);
+  stackwalker->Walk(&stack);
 
   unsigned int index;
   for (index = 0 ; index < stack.size() ; index++) {

src/processor/stackwalker.cc

@@ -37,22 +37,20 @@
 #include <memory>
 
 #include "processor/stackwalker.h"
+#include "google/symbol_supplier.h"
 #include "processor/minidump.h"
 #include "processor/source_line_resolver.h"
-#include "google/symbol_supplier.h"
+#include "processor/stackwalker_ppc.h"
-
+#include "processor/stackwalker_x86.h"
 
 namespace google_airbag {
 
-
 using std::auto_ptr;
 
 
 Stackwalker::Stackwalker(MemoryRegion *memory, MinidumpModuleList *modules,
                          SymbolSupplier *supplier)
-    : memory_(memory),
+    : memory_(memory), modules_(modules), supplier_(supplier) {
-      modules_(modules),
-      supplier_(supplier) {
 }
 
 
@@ -97,9 +95,33 @@ void Stackwalker::Walk(StackFrames *frames) {
     frame.reset(new StackFrame());
 
     // Get the next frame.
-    valid = GetCallerFrame(frame.get());
+    valid = GetCallerFrame(frame.get(), frames);
   }
 }
 
 
+// static
+Stackwalker* Stackwalker::StackwalkerForCPU(MinidumpContext *context,
+                                            MemoryRegion *memory,
+                                            MinidumpModuleList *modules,
+                                            SymbolSupplier *supplier) {
+  Stackwalker *cpu_stackwalker = NULL;
+
+  u_int32_t cpu = context->GetContextCPU();
+  switch (cpu) {
+    case MD_CONTEXT_X86:
+      cpu_stackwalker = new StackwalkerX86(context->GetContextX86(),
+                                           memory, modules, supplier);
+      break;
+
+    case MD_CONTEXT_PPC:
+      cpu_stackwalker = new StackwalkerPPC(context->GetContextPPC(),
+                                           memory, modules, supplier);
+      break;
+  }
+
+  return cpu_stackwalker;
+}
+
+
 } // namespace google_airbag

src/processor/stackwalker.h

@@ -46,10 +46,9 @@
 #include "google/stack_frame.h"
 #include "processor/memory_region.h"
 
-
 namespace google_airbag {
 
-
+class MinidumpContext;
 class MinidumpModuleList;
 class SymbolSupplier;
 
@@ -63,6 +62,14 @@ class Stackwalker {
   // data.
   void Walk(StackFrames *frames);
 
+  // Returns a new concrete subclass suitable for the CPU that a stack was
+  // generated on, according to the CPU type indicated by the context
+  // argument.  If no suitable concrete subclass exists, returns NULL.
+  static Stackwalker* StackwalkerForCPU(MinidumpContext *context,
+                                        MemoryRegion *memory,
+                                        MinidumpModuleList *modules,
+                                        SymbolSupplier *supplier);
+
  protected:
   // memory identifies a MemoryRegion that provides the stack memory
   // for the stack to walk.  modules, if non-NULL, is a MinidumpModuleList
@@ -85,10 +92,12 @@ class Stackwalker {
 
   // Obtains a caller frame.  Each call to GetCallerFrame should return the
   // frame that called the last frame returned by GetContextFrame or
-  // GetCallerFrame.  GetCallerFrame should return false on failure or
+  // GetCallerFrame.  To aid this purpose, walked_frames contains the
-  // when there are no more caller frames (when the end of the stack has
+  // StackFrames vector of frames that have already been walked.
-  // been reached).
+  // GetCallerFrame should return false on failure or when there are no more
-  virtual bool GetCallerFrame(StackFrame* frame) = 0;
+  // caller frames (when the end of the stack has been reached).
+  virtual bool GetCallerFrame(StackFrame *frame,
+                              const StackFrames *walked_frames) = 0;
 
   // A list of modules, for populating each StackFrame's module information.
   // This field is optional and may be NULL.

src/processor/stackwalker_ppc.cc

@@ -0,0 +1,120 @@
+// Copyright (c) 2006, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// stackwalker_ppc.cc: ppc-specific stackwalker.
+//
+// See stackwalker_ppc.h for documentation.
+//
+// Author: Mark Mentovai
+
+
+#include "processor/stackwalker_ppc.h"
+#include "processor/minidump.h"
+
+namespace google_airbag {
+
+
+StackwalkerPPC::StackwalkerPPC(const MDRawContextPPC *context,
+                               MemoryRegion *memory,
+                               MinidumpModuleList *modules,
+                               SymbolSupplier *supplier)
+    : Stackwalker(memory, modules, supplier),
+      context_(context) {
+  if (memory_->GetBase() + memory_->GetSize() - 1 > 0xffffffff) {
+    // 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.
+    memory_ = NULL;
+  }
+}
+
+
+bool StackwalkerPPC::GetContextFrame(StackFrame *frame) {
+  if (!context_ || !memory_ || !frame)
+    return false;
+
+  // The stack frame and instruction pointers are stored directly in
+  // registers, so pull them straight out of the CPU context structure.
+  frame->frame_pointer = context_->gpr[1];
+  frame->instruction = context_->srr0;
+
+  return true;
+}
+
+
+bool StackwalkerPPC::GetCallerFrame(StackFrame *frame,
+                                    const StackFrames *walked_frames) {
+  if (!memory_ || !frame || !walked_frames)
+    return false;
+
+  // The stack frame and instruction pointers for previous frames are saved
+  // on the stack.  The typical ppc calling convention is for the called
+  // procedure to store its return address in the calling procedure's stack
+  // frame at 8(%r1), and to allocate its own stack frame by decrementing %r1
+  // (the stack pointer) and saving the old value of %r1 at 0(%r1).  Because
+  // the ppc has no hardware stack, there is no distinction between the
+  // stack pointer and frame pointer, and what is typically thought of as
+  // the frame pointer on an x86 is usually referred to as the stack pointer
+  // on a ppc.
+
+  u_int32_t last_stack_pointer = walked_frames->back().frame_pointer;
+
+  // Don't pass frame.frame_pointer or frame.instruction directly
+  // ReadMemory, because their types are too wide (64-bit), and we
+  // specifically want to read 32-bit quantities for both.
+  u_int32_t stack_pointer;
+  if (!memory_->GetMemoryAtAddress(last_stack_pointer, &stack_pointer))
+    return false;
+
+  // A caller frame must reside higher in memory than its callee frames.
+  // Anything else is an error, or an indication that we've reached the
+  // end of the stack.
+  if (stack_pointer <= last_stack_pointer)
+    return false;
+
+  u_int32_t instruction;
+  if (!memory_->GetMemoryAtAddress(stack_pointer + 8, &instruction))
+    return false;
+
+  // Mac OS X/Darwin gives 1 as the return address from the bottom-most
+  // frame in a stack (a thread's entry point).  I haven't found any
+  // documentation on this, but 0 or 1 would be bogus return addresses,
+  // so check for them here and return false (end of stack) when they're
+  // hit to avoid having a phantom frame.
+  if (instruction <= 1)
+    return false;
+
+  frame->frame_pointer = stack_pointer;
+  frame->instruction = instruction;
+
+  return true;
+}
+
+
+} // namespace google_airbag

src/processor/stackwalker_ppc.h

@@ -0,0 +1,80 @@
+// Copyright (c) 2006, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// stackwalker_ppc.h: ppc-specific stackwalker.
+//
+// Provides stack frames given ppc register context and a memory region
+// corresponding to a ppc stack.
+//
+// Author: Mark Mentovai
+
+
+#ifndef PROCESSOR_STACKWALKER_PPC_H__
+#define PROCESSOR_STACKWALKER_PPC_H__
+
+
+#include "google/airbag_types.h"
+#include "processor/stackwalker.h"
+#include "processor/minidump_format.h"
+
+namespace google_airbag {
+
+class MinidumpContext;
+class MinidumpModuleList;
+
+
+class StackwalkerPPC : public Stackwalker {
+ public:
+  // context is a MinidumpContext object that gives access to ppc-specific
+  // register state corresponding to the innermost called frame to be
+  // included in the stack.  The other arguments are passed directly through
+  // to the base Stackwalker constructor.
+  StackwalkerPPC(const MDRawContextPPC *context,
+                 MemoryRegion *memory,
+                 MinidumpModuleList *modules,
+                 SymbolSupplier *supplier);
+
+ private:
+  // Implementation of Stackwalker, using ppc context (stack pointer in %r1,
+  // saved program counter in %srr0) and stack conventions (saved stack
+  // pointer at 0(%r1), return address at 8(0(%r1)).
+  virtual bool GetContextFrame(StackFrame *frame);
+  virtual bool GetCallerFrame(StackFrame *frame,
+                              const StackFrames *walked_frames);
+
+  // Stores the CPU context corresponding to the innermost stack frame to
+  // be returned by GetContextFrame.
+  const MDRawContextPPC *context_;
+};
+
+
+} // namespace google_airbag
+
+
+#endif // PROCESSOR_STACKWALKER_PPC_H__

src/processor/stackwalker_x86.cc

@@ -37,26 +37,20 @@
 #include "processor/stackwalker_x86.h"
 #include "processor/minidump.h"
 
-
 namespace google_airbag {
 
 
-StackwalkerX86::StackwalkerX86(MinidumpContext*    context,
+StackwalkerX86::StackwalkerX86(const MDRawContextX86 *context,
                                MemoryRegion *memory,
                                MinidumpModuleList *modules,
                                SymbolSupplier *supplier)
     : Stackwalker(memory, modules, supplier),
-      last_frame_pointer_(0) {
+      context_(context) {
   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.
     memory_ = NULL;
   }
-
-  // If |context| is not an x86 context, context_ will be set to NULL,
-  // which will cause GetContextFrame to fail when called by Walk.
-  // For StackwalkerX86, |context| should only ever be an x86 context.
-  context_ = context->GetContextX86();
 }
 
 
@@ -66,15 +60,16 @@ bool StackwalkerX86::GetContextFrame(StackFrame* frame) {
 
   // The frame and instruction pointers are stored directly in registers,
   // so pull them straight out of the CPU context structure.
-  frame->frame_pointer = last_frame_pointer_ = context_->ebp;
+  frame->frame_pointer = context_->ebp;
   frame->instruction = context_->eip;
 
   return true;
 }
 
 
-bool StackwalkerX86::GetCallerFrame(StackFrame* frame) {
+bool StackwalkerX86::GetCallerFrame(StackFrame *frame,
-  if (!memory_ || !frame)
+                                    const StackFrames *walked_frames) {
+  if (!memory_ || !frame || !walked_frames)
     return false;
 
   // The frame and instruction pointers for previous frames are saved on the
@@ -93,24 +88,26 @@ bool StackwalkerX86::GetCallerFrame(StackFrame* frame) {
   // considerably more difficult, requiring debugging information.  This
   // stackwalker doesn't attempt to solve that problem (at this point).
 
+  u_int32_t last_frame_pointer = walked_frames->back().frame_pointer;
+
   // Don't pass frame.frame_pointer or frame.instruction directly
   // ReadMemory, because their types are too wide (64-bit), and we
   // specifically want to read 32-bit quantities for both.
   u_int32_t frame_pointer;
-  if (!memory_->GetMemoryAtAddress(last_frame_pointer_, &frame_pointer))
+  if (!memory_->GetMemoryAtAddress(last_frame_pointer, &frame_pointer))
     return false;
 
   // A caller frame must reside higher in memory than its callee frames.
   // Anything else is an error, or an indication that we've reached the
   // end of the stack.
-  if (frame_pointer <= last_frame_pointer_)
+  if (frame_pointer <= last_frame_pointer)
     return false;
 
   u_int32_t instruction;
-  if (!memory_->GetMemoryAtAddress(last_frame_pointer_ + 4, &instruction))
+  if (!memory_->GetMemoryAtAddress(last_frame_pointer + 4, &instruction))
     return false;
 
-  frame->frame_pointer = last_frame_pointer_ = frame_pointer;
+  frame->frame_pointer = frame_pointer;
   frame->instruction = instruction;
 
   return true;

src/processor/stackwalker_x86.h

@@ -43,10 +43,8 @@
 #include "processor/stackwalker.h"
 #include "processor/minidump_format.h"
 
-
 namespace google_airbag {
 
-
 class MinidumpContext;
 class MinidumpModuleList;
 
@@ -57,7 +55,7 @@ class StackwalkerX86 : public Stackwalker {
   // register state corresponding to the innermost called frame to be
   // included in the stack.  The other arguments are passed directly through
   // to the base Stackwalker constructor.
-  StackwalkerX86(MinidumpContext*    context,
+  StackwalkerX86(const MDRawContextX86 *context,
                  MemoryRegion *memory,
                  MinidumpModuleList *modules,
                  SymbolSupplier *supplier);
@@ -66,15 +64,12 @@ class StackwalkerX86 : public Stackwalker {
   // Implementation of Stackwalker, using x86 context (%ebp, %eip) and
   // stack conventions (saved %ebp at [%ebp], saved %eip at 4[%ebp]).
   virtual bool GetContextFrame(StackFrame *frame);
-  virtual bool GetCallerFrame(StackFrame* frame);
+  virtual bool GetCallerFrame(StackFrame *frame,
+                              const StackFrames *walked_frames);
 
   // Stores the CPU context corresponding to the innermost stack frame to
   // be returned by GetContextFrame.
   const MDRawContextX86 *context_;
-
-  // Stores the frame pointer returned in the last stack frame returned by
-  // GetContextFrame or GetCallerFrame.
-  u_int32_t              last_frame_pointer_;
 };