Make stack_frame_info vector hold linked_ptrs instead of objects;

make Stackwalker::Walk create and return a CallStack instead of filling a
caller-supplied one (#54).  r=bryner

Interface change: Stackwalker::Walk and MinidumpProcessor::Process now return
a new CallStack*.

d2bad5d7c1


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

src/google/minidump_processor.h

@@ -46,9 +46,9 @@ class MinidumpProcessor {
   MinidumpProcessor(SymbolSupplier *supplier);
   ~MinidumpProcessor();
 
-  // Fills in the given CallStack by processing the minidump file.  Returns
+  // Returns a new CallStack produced by processing the minidump file.  The
-  // true on success.
+  // caller takes ownership of the CallStack.  Returns NULL on failure.
-  bool Process(const string &minidump_file, CallStack *stack);
+  CallStack* Process(const string &minidump_file);
 
  private:
   SymbolSupplier *supplier_;

src/processor/minidump_processor.cc

@@ -44,43 +44,41 @@ MinidumpProcessor::MinidumpProcessor(SymbolSupplier *supplier)
 MinidumpProcessor::~MinidumpProcessor() {
 }
 
-bool MinidumpProcessor::Process(const string &minidump_file,
+CallStack* MinidumpProcessor::Process(const string &minidump_file) {
-                                CallStack *stack) {
   Minidump dump(minidump_file);
   if (!dump.Read()) {
-    return false;
+    return NULL;
   }
 
   MinidumpException *exception = dump.GetException();
   if (!exception) {
-    return false;
+    return NULL;
   }
 
   MinidumpThreadList *threads = dump.GetThreadList();
   if (!threads) {
-    return false;
+    return NULL;
   }
 
   // TODO(bryner): get all the threads
   MinidumpThread *thread = threads->GetThreadByID(exception->GetThreadID());
   if (!thread) {
-    return false;
+    return NULL;
   }
 
   MinidumpMemoryRegion *thread_memory = thread->GetMemory();
   if (!thread_memory) {
-    return false;
+    return NULL;
   }
 
   auto_ptr<Stackwalker> walker(
     Stackwalker::StackwalkerForCPU(exception->GetContext(), thread_memory,
                                    dump.GetModuleList(), supplier_));
   if (!walker.get()) {
-    return false;
+    return NULL;
   }
 
-  walker->Walk(stack);
+  return walker->Walk();
-  return true;
 }
 
 }  // namespace google_airbag

src/processor/minidump_processor_unittest.cc

@@ -30,6 +30,7 @@
 // Unit test for MinidumpProcessor.  Uses a pre-generated minidump and
 // corresponding symbol file, and checks the stack frames for correctness.
 
+#include <memory>
 #include <string>
 #include "google/call_stack.h"
 #include "google/minidump_processor.h"
@@ -37,6 +38,7 @@
 #include "google/symbol_supplier.h"
 #include "processor/minidump.h"
 
+using std::auto_ptr;
 using std::string;
 using google_airbag::CallStack;
 using google_airbag::MinidumpProcessor;
@@ -74,40 +76,40 @@ static bool RunTests() {
   TestSymbolSupplier supplier;
   MinidumpProcessor processor(&supplier);
 
-  CallStack stack;
   string minidump_file = string(getenv("srcdir") ? getenv("srcdir") : ".") +
                          "/src/processor/testdata/minidump2.dmp";
 
-  ASSERT_TRUE(processor.Process(minidump_file, &stack));
+  auto_ptr<CallStack> stack(processor.Process(minidump_file));
-  ASSERT_EQ(stack.frames()->size(), 4);
+  ASSERT_TRUE(stack.get());
+  ASSERT_EQ(stack->frames()->size(), 4);
 
-  ASSERT_EQ(stack.frames()->at(0)->module_base, 0x400000);
+  ASSERT_EQ(stack->frames()->at(0)->module_base, 0x400000);
-  ASSERT_EQ(stack.frames()->at(0)->module_name, "c:\\test_app.exe");
+  ASSERT_EQ(stack->frames()->at(0)->module_name, "c:\\test_app.exe");
-  ASSERT_EQ(stack.frames()->at(0)->function_name, "CrashFunction()");
+  ASSERT_EQ(stack->frames()->at(0)->function_name, "CrashFunction()");
-  ASSERT_EQ(stack.frames()->at(0)->source_file_name, "c:\\test_app.cc");
+  ASSERT_EQ(stack->frames()->at(0)->source_file_name, "c:\\test_app.cc");
-  ASSERT_EQ(stack.frames()->at(0)->source_line, 65);
+  ASSERT_EQ(stack->frames()->at(0)->source_line, 65);
 
-  ASSERT_EQ(stack.frames()->at(1)->module_base, 0x400000);
+  ASSERT_EQ(stack->frames()->at(1)->module_base, 0x400000);
-  ASSERT_EQ(stack.frames()->at(1)->module_name, "c:\\test_app.exe");
+  ASSERT_EQ(stack->frames()->at(1)->module_name, "c:\\test_app.exe");
-  ASSERT_EQ(stack.frames()->at(1)->function_name, "main");
+  ASSERT_EQ(stack->frames()->at(1)->function_name, "main");
-  ASSERT_EQ(stack.frames()->at(1)->source_file_name, "c:\\test_app.cc");
+  ASSERT_EQ(stack->frames()->at(1)->source_file_name, "c:\\test_app.cc");
-  ASSERT_EQ(stack.frames()->at(1)->source_line, 70);
+  ASSERT_EQ(stack->frames()->at(1)->source_line, 70);
 
   // This comes from the CRT
-  ASSERT_EQ(stack.frames()->at(2)->module_base, 0x400000);
+  ASSERT_EQ(stack->frames()->at(2)->module_base, 0x400000);
-  ASSERT_EQ(stack.frames()->at(2)->module_name, "c:\\test_app.exe");
+  ASSERT_EQ(stack->frames()->at(2)->module_name, "c:\\test_app.exe");
-  ASSERT_EQ(stack.frames()->at(2)->function_name, "__tmainCRTStartup");
+  ASSERT_EQ(stack->frames()->at(2)->function_name, "__tmainCRTStartup");
-  ASSERT_EQ(stack.frames()->at(2)->source_file_name,
+  ASSERT_EQ(stack->frames()->at(2)->source_file_name,
             "f:\\rtm\\vctools\\crt_bld\\self_x86\\crt\\src\\crt0.c");
-  ASSERT_EQ(stack.frames()->at(2)->source_line, 318);
+  ASSERT_EQ(stack->frames()->at(2)->source_line, 318);
 
   // No debug info available for kernel32.dll
-  ASSERT_EQ(stack.frames()->at(3)->module_base, 0x7c800000);
+  ASSERT_EQ(stack->frames()->at(3)->module_base, 0x7c800000);
-  ASSERT_EQ(stack.frames()->at(3)->module_name,
+  ASSERT_EQ(stack->frames()->at(3)->module_name,
             "C:\\WINDOWS\\system32\\kernel32.dll");
-  ASSERT_TRUE(stack.frames()->at(3)->function_name.empty());
+  ASSERT_TRUE(stack->frames()->at(3)->function_name.empty());
-  ASSERT_TRUE(stack.frames()->at(3)->source_file_name.empty());
+  ASSERT_TRUE(stack->frames()->at(3)->source_file_name.empty());
-  ASSERT_EQ(stack.frames()->at(3)->source_line, 0);
+  ASSERT_EQ(stack->frames()->at(3)->source_line, 0);
 
   return true;
 }

src/processor/minidump_stackwalk.cc

@@ -114,12 +114,11 @@ int main(int argc, char **argv) {
     exit(1);
   }
 
-  CallStack stack;
+  auto_ptr<CallStack> stack(stackwalker->Walk());
-  stackwalker->Walk(&stack);
 
   unsigned int index;
-  for (index = 0; index < stack.frames()->size(); ++index) {
+  for (index = 0; index < stack->frames()->size(); ++index) {
-    StackFrame *frame = stack.frames()->at(index);
+    StackFrame *frame = stack->frames()->at(index);
     printf("[%2d]  instruction = 0x%08llx  \"%s\" + 0x%08llx\n",
            index,
            frame->instruction,

src/processor/source_line_resolver.cc

@@ -28,18 +28,20 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <stdio.h>
-#include <map>
 #include <string.h>
+#include <map>
+#include <memory>
 #include <vector>
 #include <utility>
 #include "processor/source_line_resolver.h"
 #include "google/stack_frame.h"
+#include "processor/linked_ptr.h"
 #include "processor/address_map-inl.h"
 #include "processor/contained_range_map-inl.h"
-#include "processor/linked_ptr.h"
 #include "processor/range_map-inl.h"
 #include "processor/stack_frame_info.h"
 
+using std::auto_ptr;
 using std::map;
 using std::vector;
 using std::make_pair;
@@ -104,9 +106,10 @@ class SourceLineResolver::Module {
 
   // Looks up the given relative address, and fills the StackFrame struct
   // with the result.  Additional debugging information, if available, is
-  // placed in frame_info.
+  // returned.  If no additional information is available, returns NULL.
-  void LookupAddress(MemAddr address,
+  // A NULL return value is not an error.  The caller takes ownership of
-                     StackFrame *frame, StackFrameInfo *frame_info) const;
+  // any returned StackFrameInfo object.
+  StackFrameInfo* LookupAddress(MemAddr address, StackFrame *frame) const;
 
  private:
   friend class SourceLineResolver;
@@ -163,7 +166,8 @@ class SourceLineResolver::Module {
   // StackInfoTypes.  These are split by type because there may be overlaps
   // between maps of different types, but some information is only available
   // as certain types.
-  ContainedRangeMap<MemAddr, StackFrameInfo> stack_info_[STACK_INFO_LAST];
+  ContainedRangeMap< MemAddr, linked_ptr<StackFrameInfo> >
+      stack_info_[STACK_INFO_LAST];
 };
 
 SourceLineResolver::SourceLineResolver() : modules_(new ModuleMap) {
@@ -198,13 +202,14 @@ bool SourceLineResolver::HasModule(const string &module_name) const {
   return modules_->find(module_name) != modules_->end();
 }
 
-void SourceLineResolver::FillSourceLineInfo(StackFrame *frame,
+StackFrameInfo* SourceLineResolver::FillSourceLineInfo(
-                                            StackFrameInfo *frame_info) const {
+    StackFrame *frame) const {
   ModuleMap::const_iterator it = modules_->find(frame->module_name);
   if (it != modules_->end()) {
-    it->second->LookupAddress(frame->instruction - frame->module_base,
+    return it->second->LookupAddress(frame->instruction - frame->module_base,
-                              frame, frame_info);
+                                     frame);
   }
+  return NULL;
 }
 
 bool SourceLineResolver::Module::LoadMap(const string &map_file) {
@@ -259,23 +264,24 @@ bool SourceLineResolver::Module::LoadMap(const string &map_file) {
   return true;
 }
 
-void SourceLineResolver::Module::LookupAddress(
+StackFrameInfo* SourceLineResolver::Module::LookupAddress(
-    MemAddr address, StackFrame *frame, StackFrameInfo *frame_info) const {
+    MemAddr address, StackFrame *frame) const {
-  if (frame_info) {
+  linked_ptr<StackFrameInfo> retrieved_info;
-    frame_info->valid = StackFrameInfo::VALID_NONE;
+  // Check for debugging info first, before any possible early returns.
+  //
+  // We only know about STACK_INFO_FRAME_DATA and STACK_INFO_FPO.  Prefer
+  // them in this order.  STACK_INFO_FRAME_DATA is the newer type that
+  // includes its own program string.  STACK_INFO_FPO is the older type
+  // corresponding to the FPO_DATA struct.  See stackwalker_x86.cc.
+  if (!stack_info_[STACK_INFO_FRAME_DATA].RetrieveRange(address,
+                                                        &retrieved_info)) {
+    stack_info_[STACK_INFO_FPO].RetrieveRange(address, &retrieved_info);
+  }
 
-    // Check for debugging info first, before any possible early returns.
+  auto_ptr<StackFrameInfo> frame_info;
-    // The caller will know that frame_info was filled in by checking its
+  if (retrieved_info.get()) {
-    // valid field.
+    frame_info.reset(new StackFrameInfo());
-    //
+    frame_info->CopyFrom(*retrieved_info.get());
-    // We only know about STACK_INFO_FRAME_DATA and STACK_INFO_FPO.  Prefer
-    // them in this order.  STACK_INFO_FRAME_DATA is the newer type that
-    // includes its own program string.  STACK_INFO_FPO is the older type
-    // corresponding to the FPO_DATA struct.  See stackwalker_x86.cc.
-    if (!stack_info_[STACK_INFO_FRAME_DATA].RetrieveRange(address,
-                                                          frame_info)) {
-      stack_info_[STACK_INFO_FPO].RetrieveRange(address, frame_info);
-    }
   }
 
   // First, look for a matching FUNC range.  Use RetrieveNearestRange instead
@@ -324,18 +330,20 @@ void SourceLineResolver::Module::LookupAddress(
     frame->function_base = frame->module_base + public_address;
   } else {
     // No FUNC or PUBLIC data available.
-    return;
+    return frame_info.release();
   }
 
-  if (frame_info &&
+  if (!frame_info.get()) {
-      !(frame_info->valid & StackFrameInfo::VALID_PARAMETER_SIZE)) {
     // Even without a relevant STACK line, many functions contain information
     // about how much space their parameters consume on the stack.  Prefer
     // the STACK stuff (above), but if it's not present, take the
     // information from the FUNC or PUBLIC line.
+    frame_info.reset(new StackFrameInfo());
     frame_info->parameter_size = parameter_size;
     frame_info->valid |= StackFrameInfo::VALID_PARAMETER_SIZE;
   }
+
+  return frame_info.release();
 }
 
 // static
@@ -518,15 +526,16 @@ bool SourceLineResolver::Module::ParseStackInfo(char *stack_info_line) {
   // if ContainedRangeMap were modified to allow replacement of
   // already-stored values.
 
-  stack_info_[type].StoreRange(rva, code_size,
+  linked_ptr<StackFrameInfo> stack_frame_info(
-                               StackFrameInfo(prolog_size,
+      new StackFrameInfo(prolog_size,
-                                              epilog_size,
+                         epilog_size,
-                                              parameter_size,
+                         parameter_size,
-                                              saved_register_size,
+                         saved_register_size,
-                                              local_size,
+                         local_size,
-                                              max_stack_size,
+                         max_stack_size,
-                                              allocates_base_pointer,
+                         allocates_base_pointer,
-                                              program_string));
+                         program_string));
+  stack_info_[type].StoreRange(rva, code_size, stack_frame_info);
 
   return true;
 }

src/processor/source_line_resolver.h

@@ -66,8 +66,11 @@ class SourceLineResolver {
   // Fills in the function_base, function_name, source_file_name,
   // and source_line fields of the StackFrame.  The instruction and
   // module_name fields must already be filled in.  Additional debugging
-  // information, if available, is placed in frame_info.
+  // information, if available, is returned.  If the information is not
-  void FillSourceLineInfo(StackFrame *frame, StackFrameInfo *frame_info) const;
+  // available, returns NULL.  A NULL return value does not indicate an
+  // error.  The caller takes ownership of any returned StackFrameInfo
+  // object.
+  StackFrameInfo* FillSourceLineInfo(StackFrame *frame) const;
 
  private:
   template<class T> class MemAddrMap;

src/processor/source_line_resolver_unittest.cc

@@ -28,12 +28,16 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <stdio.h>
+#include <memory>
 #include <string>
 #include "processor/source_line_resolver.h"
 #include "google/stack_frame.h"
+#include "processor/linked_ptr.h"
 #include "processor/stack_frame_info.h"
 
+using std::auto_ptr;
 using std::string;
+using google_airbag::linked_ptr;
 using google_airbag::SourceLineResolver;
 using google_airbag::StackFrame;
 using google_airbag::StackFrameInfo;
@@ -55,12 +59,10 @@ static bool VerifyEmpty(const StackFrame &frame) {
   return true;
 }
 
-static void ClearSourceLineInfo(StackFrame *frame,
+static void ClearSourceLineInfo(StackFrame *frame) {
-                                StackFrameInfo *frame_info) {
   frame->function_name.clear();
   frame->source_file_name.clear();
   frame->source_line = 0;
-  frame_info->program_string.clear();
 }
 
 static bool RunTests() {
@@ -74,62 +76,64 @@ static bool RunTests() {
   ASSERT_TRUE(resolver.HasModule("module2"));
 
   StackFrame frame;
-  StackFrameInfo frame_info;
   frame.instruction = 0x1000;
   frame.module_name = "module1";
-  resolver.FillSourceLineInfo(&frame, &frame_info);
+  auto_ptr<StackFrameInfo> frame_info(resolver.FillSourceLineInfo(&frame));
   ASSERT_EQ(frame.function_name, "Function1_1");
   ASSERT_EQ(frame.source_file_name, "file1_1.cc");
   ASSERT_EQ(frame.source_line, 44);
-  ASSERT_FALSE(frame_info.allocates_base_pointer);
+  ASSERT_TRUE(frame_info.get());
-  ASSERT_EQ(frame_info.program_string,
+  ASSERT_FALSE(frame_info->allocates_base_pointer);
+  ASSERT_EQ(frame_info->program_string,
             "$eip 4 + ^ = $esp $ebp 8 + = $ebp $ebp ^ =");
 
-  ClearSourceLineInfo(&frame, &frame_info);
+  ClearSourceLineInfo(&frame);
   frame.instruction = 0x800;
-  resolver.FillSourceLineInfo(&frame, &frame_info);
+  frame_info.reset(resolver.FillSourceLineInfo(&frame));
   ASSERT_TRUE(VerifyEmpty(frame));
-  ASSERT_FALSE(frame_info.allocates_base_pointer);
+  ASSERT_FALSE(frame_info.get());
-  ASSERT_TRUE(frame_info.program_string.empty());
 
   frame.instruction = 0x1280;
-  resolver.FillSourceLineInfo(&frame, &frame_info);
+  frame_info.reset(resolver.FillSourceLineInfo(&frame));
   ASSERT_EQ(frame.function_name, "Function1_3");
   ASSERT_TRUE(frame.source_file_name.empty());
   ASSERT_EQ(frame.source_line, 0);
-  ASSERT_FALSE(frame_info.allocates_base_pointer);
+  ASSERT_TRUE(frame_info.get());
-  ASSERT_TRUE(frame_info.program_string.empty());
+  ASSERT_FALSE(frame_info->allocates_base_pointer);
+  ASSERT_TRUE(frame_info->program_string.empty());
 
   frame.instruction = 0x1380;
-  resolver.FillSourceLineInfo(&frame, &frame_info);
+  frame_info.reset(resolver.FillSourceLineInfo(&frame));
   ASSERT_EQ(frame.function_name, "Function1_4");
   ASSERT_TRUE(frame.source_file_name.empty());
   ASSERT_EQ(frame.source_line, 0);
-  ASSERT_FALSE(frame_info.allocates_base_pointer);
+  ASSERT_TRUE(frame_info.get());
-  ASSERT_FALSE(frame_info.program_string.empty());
+  ASSERT_FALSE(frame_info->allocates_base_pointer);
+  ASSERT_FALSE(frame_info->program_string.empty());
 
   frame.instruction = 0x2180;
   frame.module_name = "module2";
-  resolver.FillSourceLineInfo(&frame, &frame_info);
+  frame_info.reset(resolver.FillSourceLineInfo(&frame));
   ASSERT_EQ(frame.function_name, "Function2_2");
   ASSERT_EQ(frame.source_file_name, "file2_2.cc");
   ASSERT_EQ(frame.source_line, 21);
-  ASSERT_EQ(frame_info.prolog_size, 1);
+  ASSERT_TRUE(frame_info.get());
+  ASSERT_EQ(frame_info->prolog_size, 1);
 
   frame.instruction = 0x216f;
   frame.module_name = "module2";
-  resolver.FillSourceLineInfo(&frame, &frame_info);
+  resolver.FillSourceLineInfo(&frame);
   ASSERT_EQ(frame.function_name, "Public2_1");
 
-  ClearSourceLineInfo(&frame, &frame_info);
+  ClearSourceLineInfo(&frame);
   frame.instruction = 0x219f;
   frame.module_name = "module2";
-  resolver.FillSourceLineInfo(&frame, &frame_info);
+  resolver.FillSourceLineInfo(&frame);
   ASSERT_TRUE(frame.function_name.empty());
 
   frame.instruction = 0x21a0;
   frame.module_name = "module2";
-  resolver.FillSourceLineInfo(&frame, &frame_info);
+  resolver.FillSourceLineInfo(&frame);
   ASSERT_EQ(frame.function_name, "Public2_2");
 
   ASSERT_FALSE(resolver.LoadModule("module3",

src/processor/stack_frame_info.h

@@ -80,6 +80,19 @@ struct StackFrameInfo {
         allocates_base_pointer(set_allocates_base_pointer),
         program_string(set_program_string) {}
 
+  // CopyFrom makes "this" StackFrameInfo object identical to "that".
+  void CopyFrom(const StackFrameInfo &that) {
+    valid = that.valid;
+    prolog_size = that.prolog_size;
+    epilog_size = that.epilog_size;
+    parameter_size = that.parameter_size;
+    saved_register_size = that.saved_register_size;
+    local_size = that.local_size;
+    max_stack_size = that.max_stack_size;
+    allocates_base_pointer = that.allocates_base_pointer;
+    program_string = that.program_string;
+  }
+
   // Clears the StackFrameInfo object so that users will see it as though
   // it contains no information.
   void Clear() { valid = VALID_NONE; program_string.erase(); }

src/processor/stackwalker.cc

@@ -40,8 +40,10 @@
 #include "google/call_stack.h"
 #include "google/stack_frame.h"
 #include "google/symbol_supplier.h"
+#include "processor/linked_ptr.h"
 #include "processor/minidump.h"
 #include "processor/source_line_resolver.h"
+#include "processor/stack_frame_info.h"
 #include "processor/stackwalker_ppc.h"
 #include "processor/stackwalker_x86.h"
 
@@ -52,15 +54,20 @@ using std::auto_ptr;
 
 Stackwalker::Stackwalker(MemoryRegion *memory, MinidumpModuleList *modules,
                          SymbolSupplier *supplier)
-    : memory_(memory), stack_frame_info_(), modules_(modules),
+    : memory_(memory), modules_(modules), supplier_(supplier) {
-      supplier_(supplier) {
 }
 
 
-void Stackwalker::Walk(CallStack *stack) {
+CallStack* Stackwalker::Walk() {
-  stack_frame_info_.clear();
   SourceLineResolver resolver;
 
+  auto_ptr<CallStack> stack(new CallStack());
+
+  // stack_frame_info parallels the CallStack.  The vector is passed to the
+  // GetCallerFrame function.  It contains information that may be helpful
+  // for stackwalking.
+  vector< linked_ptr<StackFrameInfo> > stack_frame_info;
+
   // Begin with the context frame, and keep getting callers until there are
   // no more.
 
@@ -72,7 +79,7 @@ void Stackwalker::Walk(CallStack *stack) {
     // frame_pointer fields.  The frame structure comes from either the
     // context frame (above) or a caller frame (below).
 
-    StackFrameInfo frame_info;
+    linked_ptr<StackFrameInfo> frame_info;
 
     // Resolve the module information, if a module map was provided.
     if (modules_) {
@@ -87,7 +94,7 @@ void Stackwalker::Walk(CallStack *stack) {
             resolver.LoadModule(frame->module_name, symbol_file);
           }
         }
-        resolver.FillSourceLineInfo(frame.get(), &frame_info);
+        frame_info.reset(resolver.FillSourceLineInfo(frame.get()));
       }
     }
 
@@ -95,13 +102,15 @@ void Stackwalker::Walk(CallStack *stack) {
     // over the frame, because the stack now owns it.
     stack->frames_.push_back(frame.release());
 
-    // Copy the frame info.
+    // Add the frame info to the parallel stack.
-    stack_frame_info_.push_back(frame_info);
+    stack_frame_info.push_back(frame_info);
-    frame_info.Clear();
+    frame_info.reset(NULL);
 
     // Get the next frame and take ownership.
-    frame.reset(GetCallerFrame(stack));
+    frame.reset(GetCallerFrame(stack.get(), stack_frame_info));
   }
+
+  return stack.release();
 }
 
 

src/processor/stackwalker.h

@@ -43,25 +43,28 @@
 
 #include <vector>
 
-#include "processor/stack_frame_info.h"
-
 namespace google_airbag {
 
 class CallStack;
+template<typename T> class linked_ptr;
 class MemoryRegion;
 class MinidumpContext;
 class MinidumpModuleList;
 struct StackFrame;
+struct StackFrameInfo;
 class SymbolSupplier;
 
+using std::vector;
+
 
 class Stackwalker {
  public:
   virtual ~Stackwalker() {}
 
-  // Fills the given CallStack by calling GetContextFrame and GetCallerFrame,
+  // Creates a new CallStack and populates it by calling GetContextFrame and
-  // and populating the returned frames with all available data.
+  // GetCallerFrame.  The frames are further processed to fill all available
-  void Walk(CallStack* stack);
+  // data.  The caller takes ownership of the CallStack returned by Walk.
+  CallStack* Walk();
 
   // Returns a new concrete subclass suitable for the CPU that a stack was
   // generated on, according to the CPU type indicated by the context
@@ -86,11 +89,6 @@ class Stackwalker {
   // get information from the stack.
   MemoryRegion *memory_;
 
-  // Additional debugging information for each stack frame.  This vector
-  // parallels the CallStack.  Subclasses may use this information to help
-  // walk the stack.
-  std::vector<StackFrameInfo> stack_frame_info_;
-
  private:
   // Obtains the context frame, the innermost called procedure in a stack
   // trace.  Returns NULL on failure.  GetContextFrame allocates a new
@@ -106,7 +104,9 @@ class Stackwalker {
   // the end of the stack has been reached).  GetCallerFrame allocates a new
   // StackFrame (or StackFrame subclass), ownership of which is taken by
   // the caller.
-  virtual StackFrame* GetCallerFrame(const CallStack *stack) = 0;
+  virtual StackFrame* GetCallerFrame(
+      const CallStack *stack,
+      const vector< linked_ptr<StackFrameInfo> > &stack_frame_info) = 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

@@ -73,7 +73,9 @@ StackFrame* StackwalkerPPC::GetContextFrame() {
 }
 
 
-StackFrame* StackwalkerPPC::GetCallerFrame(const CallStack *stack) {
+StackFrame* StackwalkerPPC::GetCallerFrame(
+    const CallStack *stack,
+    const vector< linked_ptr<StackFrameInfo> > &stack_frame_info) {
   if (!memory_ || !stack)
     return NULL;
 

src/processor/stackwalker_ppc.h

@@ -65,7 +65,9 @@ class StackwalkerPPC : public Stackwalker {
   // saved program counter in %srr0) and stack conventions (saved stack
   // pointer at 0(%r1), return address at 8(0(%r1)).
   virtual StackFrame* GetContextFrame();
-  virtual StackFrame* GetCallerFrame(const CallStack *stack);
+  virtual StackFrame* GetCallerFrame(
+      const CallStack *stack,
+      const vector< linked_ptr<StackFrameInfo> > &stack_frame_info);
 
   // Stores the CPU context corresponding to the innermost stack frame to
   // be returned by GetContextFrame.

src/processor/stackwalker_selftest.cc

@@ -38,6 +38,7 @@
 
 
 #include <cstdio>
+#include <memory>
 
 #include "google/airbag_types.h"
 #include "google/call_stack.h"
@@ -46,6 +47,7 @@
 #include "processor/memory_region.h"
 #include "processor/minidump_format.h"
 
+using std::auto_ptr;
 using google_airbag::CallStack;
 using google_airbag::MemoryRegion;
 using google_airbag::StackFrame;
@@ -216,23 +218,22 @@ static unsigned int CountCallerFrames() {
   StackwalkerPPC stackwalker = StackwalkerPPC(&context, &memory, NULL, NULL);
 #endif  // __i386__ || __ppc__
 
-  CallStack stack;
+  auto_ptr<CallStack> stack(stackwalker.Walk());
-  stackwalker.Walk(&stack);
 
 #ifdef PRINT_STACKS
   printf("\n");
   for(unsigned int frame_index = 0;
-      frame_index < stack.Count();
+      frame_index < stack->frames()->size();
       ++frame_index) {
-    StackFrame *frame = stack.FrameAt(frame_index);
+    StackFrame *frame = stack->frames()->at(frame_index);
     printf("frame %-3d  instruction = 0x%08llx",
            frame_index, frame->instruction);
 #if defined(__i386__)
-    StackFrameX86 *frame_x86 = reinterpret_cast<StackFrameX86*>(frame.get());
+    StackFrameX86 *frame_x86 = reinterpret_cast<StackFrameX86*>(frame);
     printf("  esp = 0x%08x  ebp = 0x%08x\n",
            frame_x86->context.esp, frame_x86->context.ebp);
 #elif defined(__ppc__)
-    StackFramePPC *frame_ppc = reinterpret_cast<StackFramePPC*>(frame.get());
+    StackFramePPC *frame_ppc = reinterpret_cast<StackFramePPC*>(frame);
     printf("  gpr[1] = 0x%08x\n", frame_ppc->context.gpr[1]);
 #endif  // __i386__ || __ppc__
   }
@@ -241,7 +242,7 @@ static unsigned int CountCallerFrames() {
   // Subtract 1 because the caller wants the number of frames beneath
   // itself.  Because the caller called us, subract two for our frame and its
   // frame, which are included in stack->size().
-  return stack.frames()->size() - 2;
+  return stack->frames()->size() - 2;
 }
 
 

src/processor/stackwalker_x86.cc

@@ -37,8 +37,10 @@
 #include "processor/stackwalker_x86.h"
 #include "google/call_stack.h"
 #include "google/stack_frame_cpu.h"
+#include "processor/linked_ptr.h"
 #include "processor/minidump.h"
 #include "processor/postfix_evaluator-inl.h"
+#include "processor/stack_frame_info.h"
 
 namespace google_airbag {
 
@@ -73,13 +75,15 @@ StackFrame* StackwalkerX86::GetContextFrame() {
 }
 
 
-StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) {
+StackFrame* StackwalkerX86::GetCallerFrame(
+    const CallStack *stack,
+    const vector< linked_ptr<StackFrameInfo> > &stack_frame_info) {
   if (!memory_ || !stack)
     return NULL;
 
   StackFrameX86 *last_frame = static_cast<StackFrameX86*>(
       stack->frames()->back());
-  StackFrameInfo *last_frame_info = &stack_frame_info_.back();
+  StackFrameInfo *last_frame_info = stack_frame_info.back().get();
 
   // This stackwalker sets each frame's %esp to its value immediately prior
   // to the CALL into the callee.  This means that %esp points to the last
@@ -113,12 +117,13 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) {
   // are unknown, 0 is also used in that case.  When that happens, it should
   // be possible to walk to the next frame without reference to %esp.
 
-  int frames_already_walked = stack_frame_info_.size();
+  int frames_already_walked = stack_frame_info.size();
   u_int32_t last_frame_callee_parameter_size = 0;
   if (frames_already_walked >= 2) {
     StackFrameInfo *last_frame_callee_info =
-        &stack_frame_info_[frames_already_walked - 2];
+        stack_frame_info[frames_already_walked - 2].get();
-    if (last_frame_callee_info->valid & StackFrameInfo::VALID_PARAMETER_SIZE) {
+    if (last_frame_callee_info &&
+        last_frame_callee_info->valid & StackFrameInfo::VALID_PARAMETER_SIZE) {
       last_frame_callee_parameter_size =
           last_frame_callee_info->parameter_size;
     }
@@ -135,7 +140,7 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) {
   dictionary["$esp"] = last_frame->context.esp;
   dictionary[".cbCalleeParams"] = last_frame_callee_parameter_size;
 
-  if (last_frame_info->valid == StackFrameInfo::VALID_ALL) {
+  if (last_frame_info && last_frame_info->valid == StackFrameInfo::VALID_ALL) {
     // FPO debugging data is available.  Initialize constants.
     dictionary[".cbSavedRegs"] = last_frame_info->saved_register_size;
     dictionary[".cbLocals"] = last_frame_info->local_size;
@@ -144,7 +149,8 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) {
                                    last_frame_info->local_size +
                                    last_frame_info->saved_register_size;
   }
-  if (last_frame_info->valid & StackFrameInfo::VALID_PARAMETER_SIZE) {
+  if (last_frame_info &&
+      last_frame_info->valid & StackFrameInfo::VALID_PARAMETER_SIZE) {
     // This is treated separately because it can either come from FPO data or
     // from other debugging data.
     dictionary[".cbParams"] = last_frame_info->parameter_size;
@@ -156,7 +162,7 @@ StackFrame* StackwalkerX86::GetCallerFrame(const CallStack *stack) {
   // the return address and the values of other registers in the calling
   // function.
   string program_string;
-  if (last_frame_info->valid == StackFrameInfo::VALID_ALL) {
+  if (last_frame_info && last_frame_info->valid == StackFrameInfo::VALID_ALL) {
     // FPO data available.
     if (!last_frame_info->program_string.empty()) {
       // The FPO data has its own program string, which will tell us how to

src/processor/stackwalker_x86.h

@@ -65,7 +65,9 @@ class StackwalkerX86 : public Stackwalker {
   // stack conventions (saved %ebp at [%ebp], saved %eip at 4[%ebp], or
   // alternate conventions as guided by stack_frame_info_).
   virtual StackFrame* GetContextFrame();
-  virtual StackFrame* GetCallerFrame(const CallStack *stack);
+  virtual StackFrame* GetCallerFrame(
+      const CallStack *stack,
+      const vector< linked_ptr<StackFrameInfo> > &stack_frame_info);
 
   // Stores the CPU context corresponding to the innermost stack frame to
   // be returned by GetContextFrame.