Restrict ownership of symbol data buffers to symbol supplier.

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

18 files changed, 189 insertions, 115 deletions

diff --git a/src/google_breakpad/processor/basic_source_line_resolver.h b/src/google_breakpad/processor/basic_source_line_resolver.h
index 6cf99705..f77b3bbb 100644
--- a/src/google_breakpad/processor/basic_source_line_resolver.h
+++ b/src/google_breakpad/processor/basic_source_line_resolver.h
@@ -55,6 +55,7 @@ class BasicSourceLineResolver : public SourceLineResolverBase {
   using SourceLineResolverBase::LoadModule;
   using SourceLineResolverBase::LoadModuleUsingMapBuffer;
   using SourceLineResolverBase::LoadModuleUsingMemoryBuffer;
+  using SourceLineResolverBase::ShouldDeleteMemoryBufferAfterLoadModule;
   using SourceLineResolverBase::UnloadModule;
   using SourceLineResolverBase::HasModule;
   using SourceLineResolverBase::FillSourceLineInfo;
@@ -73,16 +74,6 @@ class BasicSourceLineResolver : public SourceLineResolverBase {
   // Module implements SourceLineResolverBase::Module interface.
   class Module;
 
-  // Helper methods to manage C-String format symbol data.
-  // See "google_breakpad/processor/source_line_resolver_base.h" for more
-  // comments about these helper methods.
-  virtual void DeleteDataAfterLoad(char *symbol_data);
-  // No-op helper methods.
-  virtual void DeleteDataUnload(const CodeModule *module) { }
-  virtual void ClearLocalMemory() { }
-  virtual void StoreDataBeforeLoad(const CodeModule *module,
-                                   char *symbol_data) { }
-
   // Disallow unwanted copy ctor and assignment operator
   BasicSourceLineResolver(const BasicSourceLineResolver&);
   void operator=(const BasicSourceLineResolver&);
diff --git a/src/google_breakpad/processor/fast_source_line_resolver.h b/src/google_breakpad/processor/fast_source_line_resolver.h
index 4bb74f1a..60f6dfce 100644
--- a/src/google_breakpad/processor/fast_source_line_resolver.h
+++ b/src/google_breakpad/processor/fast_source_line_resolver.h
@@ -84,18 +84,10 @@ class FastSourceLineResolver : public SourceLineResolverBase {
   // Deserialize raw memory data to construct a WindowsFrameInfo object.
   static WindowsFrameInfo CopyWFI(const char *raw_memory);
 
-  // Helper methods to manage C-String format symbol data.
-  // See "google_breakpad/processor/source_line_resolver_base.h" for more
-  // comments about these helper methods.
-  virtual void StoreDataBeforeLoad(const CodeModule *module, char *symbol_data);
-  virtual void DeleteDataUnload(const CodeModule *module);
-  virtual void ClearLocalMemory();
-  // No-op helper method.
-  virtual void DeleteDataAfterLoad(char *symbol_data) { }
-
-  // Store memory data allocated in LoadModule and LoadModuleUsingMapBuffer.
-  typedef std::map<string, char*, CompareString> MemoryMap;
-  MemoryMap memory_chunks_;
+  // FastSourceLineResolver requires the memory buffer stays alive during the
+  // lifetime of a corresponding module, therefore it needs to redefine this
+  // virtual method.
+  virtual bool ShouldDeleteMemoryBufferAfterLoadModule();
 
   // Disallow unwanted copy ctor and assignment operator
   FastSourceLineResolver(const FastSourceLineResolver&);
diff --git a/src/google_breakpad/processor/network_source_line_resolver.h b/src/google_breakpad/processor/network_source_line_resolver.h
index f60ff701..138b2f56 100644
--- a/src/google_breakpad/processor/network_source_line_resolver.h
+++ b/src/google_breakpad/processor/network_source_line_resolver.h
@@ -84,6 +84,10 @@ class NetworkSourceLineResolver : public SourceLineResolverInterface,
   virtual bool LoadModuleUsingMemoryBuffer(const CodeModule *module,
                                            char *memory_buffer);
 
+  // It doesn't matter whether returns true or false, since no memory buffer
+  // will be allocated in GetCStringSymbolData().
+  virtual bool ShouldDeleteMemoryBufferAfterLoadModule() { return true; }
+
   void UnloadModule(const CodeModule *module);
 
   virtual bool HasModule(const CodeModule *module);
@@ -112,6 +116,11 @@ class NetworkSourceLineResolver : public SourceLineResolverInterface,
                                             string *symbol_file,
                                             char **symbol_data);
 
+  // Delete the data buffer allocated in GetCStringSymbolData().
+  // Since the above GetCStringSymbolData() won't allocate any memory at all,
+  // this method is no-op.
+  virtual void FreeSymbolData(const CodeModule *module) { }
+
  private:
   int wait_milliseconds_;
   // if false, some part of our network setup failed.
diff --git a/src/google_breakpad/processor/source_line_resolver_base.h b/src/google_breakpad/processor/source_line_resolver_base.h
index 8113e2ed..d950a736 100644
--- a/src/google_breakpad/processor/source_line_resolver_base.h
+++ b/src/google_breakpad/processor/source_line_resolver_base.h
@@ -73,35 +73,13 @@ class SourceLineResolverBase : public SourceLineResolverInterface {
                                         const string &map_buffer);
   virtual bool LoadModuleUsingMemoryBuffer(const CodeModule *module,
                                            char *memory_buffer);
+  virtual bool ShouldDeleteMemoryBufferAfterLoadModule();
   virtual void UnloadModule(const CodeModule *module);
   virtual bool HasModule(const CodeModule *module);
   virtual void FillSourceLineInfo(StackFrame *frame);
   virtual WindowsFrameInfo *FindWindowsFrameInfo(const StackFrame *frame);
   virtual CFIFrameInfo *FindCFIFrameInfo(const StackFrame *frame);
 
-  // Helper methods to manage C-String format symbol data.
-  // These methods are defined as no-op by default.
-  //
-  // StoreDataBeforeLoad() will be called in LoadModule() and
-  // LoadModuleUsingMapBuffer() to let subclass decide whether or how to store
-  // the dynamicly allocated memory data, before passing the data to
-  // LoadModuleUsingMemoryBuffer() which actually loads the module.
-  virtual void StoreDataBeforeLoad(const CodeModule *module, char *symbol_data);
-
-  // DeleteDataAfterLoad() will be called at the end of
-  // LoadModuleUsingMemoryBuffer() to let subclass decide whether to delete the
-  // allocated memory data or not (which depends on whether the subclass has
-  // ownership of the data or not).
-  virtual void DeleteDataAfterLoad(char *symbol_data);
-
-  // DeleteDataUnload() will be called in UnloadModule() to let subclass clean
-  // up dynamicly allocated data associated with the module, if there is any.
-  virtual void DeleteDataUnload(const CodeModule *module);
-
-  // ClearLocalMemory() will be called in destructor to let subclass clean up
-  // all local memory data it owns, if there is any.
-  virtual void ClearLocalMemory();
-
   // Nested structs and classes.
   struct Line;
   struct Function;
@@ -113,10 +91,14 @@ class SourceLineResolverBase : public SourceLineResolverInterface {
   class Module;
   class AutoFileCloser;
 
-  // All of the modules we've loaded
+  // All of the modules that are loaded.
   typedef map<string, Module*, CompareString> ModuleMap;
   ModuleMap *modules_;
 
+  // All of heap-allocated buffers that are owned locally by resolver.
+  typedef std::map<string, char*, CompareString> MemoryMap;
+  MemoryMap *memory_buffers_;
+
   // Creates a concrete module at run-time.
   ModuleFactory *module_factory_;
 
diff --git a/src/google_breakpad/processor/source_line_resolver_interface.h b/src/google_breakpad/processor/source_line_resolver_interface.h
index bd6a12d6..103f979e 100644
--- a/src/google_breakpad/processor/source_line_resolver_interface.h
+++ b/src/google_breakpad/processor/source_line_resolver_interface.h
@@ -67,9 +67,15 @@ class SourceLineResolverInterface {
   // Add an interface to load symbol using C-String data insteading string.
   // This is useful in the optimization design for avoiding unnecessary copying
   // of symbol data, in order to improve memory efficiency.
+  // LoadModuleUsingMemoryBuffer() does NOT take ownership of memory_buffer.
   virtual bool LoadModuleUsingMemoryBuffer(const CodeModule *module,
                                            char *memory_buffer) = 0;
 
+  // Return true if the memory buffer should be deleted immediately after
+  // LoadModuleUsingMemoryBuffer(). Return false if the memory buffer has to be
+  // alive during the lifetime of the corresponding Module.
+  virtual bool ShouldDeleteMemoryBufferAfterLoadModule() = 0;
+
   // Request that the specified module be unloaded from this resolver.
   // A resolver may choose to ignore such a request.
   virtual void UnloadModule(const CodeModule *module) = 0;
@@ -79,7 +85,7 @@ class SourceLineResolverInterface {
 
   // 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.  
+  // module_name fields must already be filled in.
   virtual void FillSourceLineInfo(StackFrame *frame) = 0;
 
   // If Windows stack walking information is available covering
@@ -87,7 +93,7 @@ class SourceLineResolverInterface {
   // describing it. If the information is not available, returns NULL.
   // A NULL return value does not indicate an error. The caller takes
   // ownership of any returned WindowsFrameInfo object.
-  virtual WindowsFrameInfo *FindWindowsFrameInfo(const StackFrame *frame) = 0; 
+  virtual WindowsFrameInfo *FindWindowsFrameInfo(const StackFrame *frame) = 0;
 
   // If CFI stack walking information is available covering ADDRESS,
   // return a CFIFrameInfo structure describing it. If the information
diff --git a/src/google_breakpad/processor/symbol_supplier.h b/src/google_breakpad/processor/symbol_supplier.h
index 4a41688e..26f5d7fa 100644
--- a/src/google_breakpad/processor/symbol_supplier.h
+++ b/src/google_breakpad/processor/symbol_supplier.h
@@ -76,14 +76,21 @@ class SymbolSupplier {
                                      string *symbol_file,
                                      string *symbol_data) = 0;
 
-  // Same as above, except places symbol data into symbol_data as C-string in
-  // dynamically allocated memory. Using C-string as type of symbol data enables
-  // passing data by pointer, and thus avoids unncessary copying of data (to
-  // improve memory efficiency).
+  // Same as above, except allocates data buffer on heap and then places the
+  // symbol data into the buffer as C-string.
+  // SymbolSupplier is responsible for deleting the data buffer. After the call
+  // to GetCStringSymbolData(), the caller should call FreeSymbolData(const
+  // Module *module) once the data buffer is no longer needed.
+  // If symbol_data is not NULL, symbol supplier won't return FOUND unless it
+  // returns a valid buffer in symbol_data, e.g., returns INTERRUPT on memory
+  // allocation failure.
   virtual SymbolResult GetCStringSymbolData(const CodeModule *module,
                                             const SystemInfo *system_info,
                                             string *symbol_file,
                                             char **symbol_data) = 0;
+
+  // Frees the data buffer allocated for the module in GetCStringSymbolData.
+  virtual void FreeSymbolData(const CodeModule *module) = 0;
 };
 
 }  // namespace google_breakpad
diff --git a/src/processor/basic_source_line_resolver.cc b/src/processor/basic_source_line_resolver.cc
index a795c4ae..ff57140f 100644
--- a/src/processor/basic_source_line_resolver.cc
+++ b/src/processor/basic_source_line_resolver.cc
@@ -60,11 +60,6 @@ static const char *kWhitespace = " \r\n";
 BasicSourceLineResolver::BasicSourceLineResolver() :
     SourceLineResolverBase(new BasicModuleFactory) { }
 
-void BasicSourceLineResolver::DeleteDataAfterLoad(char *symbol_data) {
-  // Always delete allocated memory after loading symbol.
-  delete symbol_data;
-}
-
 bool BasicSourceLineResolver::Module::LoadMapFromMemory(char *memory_buffer) {
   linked_ptr<Function> cur_func;
   int line_number = 0;
diff --git a/src/processor/exploitability_unittest.cc b/src/processor/exploitability_unittest.cc
index b23c2de7..4de6f1d6 100644
--- a/src/processor/exploitability_unittest.cc
+++ b/src/processor/exploitability_unittest.cc
@@ -98,6 +98,7 @@ class TestSymbolSupplier : public SymbolSupplier {
                                             string *symbol_file,
                                             char **symbol_data);
 
+  virtual void FreeSymbolData(const CodeModule *module) { }
   // When set to true, causes the SymbolSupplier to return INTERRUPT
   void set_interrupt(bool interrupt) { interrupt_ = interrupt; }
 
diff --git a/src/processor/fast_source_line_resolver.cc b/src/processor/fast_source_line_resolver.cc
index 5f6cc074..45c1f0f0 100644
--- a/src/processor/fast_source_line_resolver.cc
+++ b/src/processor/fast_source_line_resolver.cc
@@ -54,26 +54,8 @@ namespace google_breakpad {
 FastSourceLineResolver::FastSourceLineResolver()
   : SourceLineResolverBase(new FastModuleFactory) { }
 
-void FastSourceLineResolver::ClearLocalMemory() {
-  for (MemoryMap::iterator it = memory_chunks_.begin();
-       it != memory_chunks_.end();
-       ++it) {
-    delete it->second;
-  }
-}
-
-void FastSourceLineResolver::DeleteDataUnload(const CodeModule *module) {
-  if (module) {
-    MemoryMap::iterator iter = memory_chunks_.find(module->code_file());
-    if (iter != memory_chunks_.end()) {
-      delete iter->second;
-    }
-  }
-}
-
-void FastSourceLineResolver::StoreDataBeforeLoad(const CodeModule *module,
-                                                 char *symbol_data) {
-  memory_chunks_.insert(make_pair(module->code_file(), symbol_data));
+bool FastSourceLineResolver::ShouldDeleteMemoryBufferAfterLoadModule() {
+  return false;
 }
 
 void FastSourceLineResolver::Module::LookupAddress(StackFrame *frame) const {
diff --git a/src/processor/minidump_processor_unittest.cc b/src/processor/minidump_processor_unittest.cc
index ac0368d5..fcac48ff 100644
--- a/src/processor/minidump_processor_unittest.cc
+++ b/src/processor/minidump_processor_unittest.cc
@@ -36,6 +36,7 @@
 #include <iostream>
 #include <fstream>
 #include <map>
+#include <utility>
 
 #include "breakpad_googletest_includes.h"
 #include "google_breakpad/processor/basic_source_line_resolver.h"
@@ -118,11 +119,14 @@ class TestSymbolSupplier : public SymbolSupplier {
                                             string *symbol_file,
                                             char **symbol_data);
 
+  virtual void FreeSymbolData(const CodeModule *module);
+
   // When set to true, causes the SymbolSupplier to return INTERRUPT
   void set_interrupt(bool interrupt) { interrupt_ = interrupt; }
 
  private:
   bool interrupt_;
+  map<string, char *> memory_buffers_;
 };
 
 SymbolSupplier::SymbolResult TestSymbolSupplier::GetSymbolFile(
@@ -181,13 +185,27 @@ SymbolSupplier::SymbolResult TestSymbolSupplier::GetCStringSymbolData(
                                                  &symbol_data_string);
   if (s == FOUND) {
     unsigned int size = symbol_data_string.size() + 1;
-    *symbol_data = reinterpret_cast<char*>(operator new(size));
+    *symbol_data = new char[size];
+    if (*symbol_data == NULL) {
+      BPLOG(ERROR) << "Memory allocation failed for module: "
+                   << module->code_file() << " size: " << size;
+      return INTERRUPT;
+    }
     strcpy(*symbol_data, symbol_data_string.c_str());
+    memory_buffers_.insert(make_pair(module->code_file(), *symbol_data));
   }
 
   return s;
 }
 
+void TestSymbolSupplier::FreeSymbolData(const CodeModule *module) {
+  map<string, char *>::iterator it = memory_buffers_.find(module->code_file());
+  if (it != memory_buffers_.end()) {
+    delete [] it->second;
+    memory_buffers_.erase(it);
+  }
+}
+
 // A mock symbol supplier that always returns NOT_FOUND; one current
 // use for testing the processor's caching of symbol lookups.
 class MockSymbolSupplier : public SymbolSupplier {
@@ -204,6 +222,7 @@ class MockSymbolSupplier : public SymbolSupplier {
                                                   const SystemInfo*,
                                                   string*,
                                                   char**));
+  MOCK_METHOD1(FreeSymbolData, void(const CodeModule*));
 };
 
 class MinidumpProcessorTest : public ::testing::Test {
diff --git a/src/processor/network_source_line_server_unittest.cc b/src/processor/network_source_line_server_unittest.cc
index 50eb8a04..c45f19cc 100644
--- a/src/processor/network_source_line_server_unittest.cc
+++ b/src/processor/network_source_line_server_unittest.cc
@@ -93,6 +93,7 @@ public:
                                                   const SystemInfo *system_info,
                                                   string *symbol_file,
                                                   char **symbol_data));
+  MOCK_METHOD1(FreeSymbolData, void(const CodeModule *module));
 };
 
 class MockSourceLineResolver : public SourceLineResolverInterface {
@@ -106,6 +107,7 @@ class MockSourceLineResolver : public SourceLineResolverInterface {
                                               const string &map_buffer));
   MOCK_METHOD2(LoadModuleUsingMemoryBuffer, bool(const CodeModule *module,
                                                  char *memory_buffer));
+  MOCK_METHOD0(ShouldDeleteMemoryBufferAfterLoadModule, bool());
   MOCK_METHOD1(UnloadModule, void(const CodeModule *module));
   MOCK_METHOD1(HasModule, bool(const CodeModule *module));
   MOCK_METHOD1(FillSourceLineInfo, void(StackFrame *frame));
diff --git a/src/processor/simple_symbol_supplier.cc b/src/processor/simple_symbol_supplier.cc
index df77d72c..76820e12 100644
--- a/src/processor/simple_symbol_supplier.cc
+++ b/src/processor/simple_symbol_supplier.cc
@@ -107,13 +107,34 @@ SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetCStringSymbolData(
 
   if (s == FOUND) {
     unsigned int size = symbol_data_string.size() + 1;
-    *symbol_data = reinterpret_cast<char*>(operator new(size));
+    *symbol_data = new char[size];
+    if (*symbol_data == NULL) {
+      BPLOG(ERROR) << "Memory allocation for size " << size << " failed";
+      return INTERRUPT;
+    }
     memcpy(*symbol_data, symbol_data_string.c_str(), size - 1);
     (*symbol_data)[size - 1] = '\0';
+    memory_buffers_.insert(make_pair(module->code_file(), *symbol_data));
   }
   return s;
 }
 
+void SimpleSymbolSupplier::FreeSymbolData(const CodeModule *module) {
+  if (!module) {
+    BPLOG(INFO) << "Cannot free symbol data buffer for NULL module";
+    return;
+  }
+
+  map<string, char *>::iterator it = memory_buffers_.find(module->code_file());
+  if (it == memory_buffers_.end()) {
+    BPLOG(INFO) << "Cannot find symbol data buffer for module "
+                << module->code_file();
+    return;
+  }
+  delete [] it->second;
+  memory_buffers_.erase(it);
+}
+
 SymbolSupplier::SymbolResult SimpleSymbolSupplier::GetSymbolFileAtPathFromRoot(
     const CodeModule *module, const SystemInfo *system_info,
     const string &root_path, string *symbol_file) {
diff --git a/src/processor/simple_symbol_supplier.h b/src/processor/simple_symbol_supplier.h
index 753bd0cd..e1c16195 100644
--- a/src/processor/simple_symbol_supplier.h
+++ b/src/processor/simple_symbol_supplier.h
@@ -76,6 +76,7 @@
 #ifndef PROCESSOR_SIMPLE_SYMBOL_SUPPLIER_H__
 #define PROCESSOR_SIMPLE_SYMBOL_SUPPLIER_H__
 
+#include <map>
 #include <string>
 #include <vector>
 
@@ -83,6 +84,7 @@
 
 namespace google_breakpad {
 
+using std::map;
 using std::string;
 using std::vector;
 
@@ -111,11 +113,16 @@ class SimpleSymbolSupplier : public SymbolSupplier {
                                      string *symbol_file,
                                      string *symbol_data);
 
+  // Allocates data buffer on heap and writes symbol data into buffer.
+  // Symbol supplier ALWAYS takes ownership of the data buffer.
   virtual SymbolResult GetCStringSymbolData(const CodeModule *module,
                                             const SystemInfo *system_info,
                                             string *symbol_file,
                                             char **symbol_data);
 
+  // Free the data buffer allocated in the above GetCStringSymbolData();
+  virtual void FreeSymbolData(const CodeModule *module);
+
  protected:
   SymbolResult GetSymbolFileAtPathFromRoot(const CodeModule *module,
                                            const SystemInfo *system_info,
@@ -123,6 +130,7 @@ class SimpleSymbolSupplier : public SymbolSupplier {
                                            string *symbol_file);
 
  private:
+  map<string, char *> memory_buffers_;
   vector<string> paths_;
 };
 
diff --git a/src/processor/source_line_resolver_base.cc b/src/processor/source_line_resolver_base.cc
index 14f6ec6d..b8362dcf 100644
--- a/src/processor/source_line_resolver_base.cc
+++ b/src/processor/source_line_resolver_base.cc
@@ -53,6 +53,7 @@ namespace google_breakpad {
 SourceLineResolverBase::SourceLineResolverBase(
     ModuleFactory *module_factory)
   : modules_(new ModuleMap),
+    memory_buffers_(new MemoryMap),
     module_factory_(module_factory) {
 }
 
@@ -65,19 +66,16 @@ SourceLineResolverBase::~SourceLineResolverBase() {
   }
   // Delete the map of modules.
   delete modules_;
-  delete module_factory_;
-
-  // Helper method to be specified by subclasses.
-  ClearLocalMemory();
-}
 
-// Helper methods to be specified by subclasses.
-void SourceLineResolverBase::StoreDataBeforeLoad(const CodeModule *module,
-                                                 char *symbol_data) { }
-void SourceLineResolverBase::DeleteDataAfterLoad(char *symbol_data) { }
-void SourceLineResolverBase::DeleteDataUnload(const CodeModule *module) { }
-void SourceLineResolverBase::ClearLocalMemory() { }
+  MemoryMap::iterator iter = memory_buffers_->begin();
+  for (; iter != memory_buffers_->end(); ++iter) {
+    delete [] iter->second;
+  }
+  // Delete the map of memory buffers.
+  delete memory_buffers_;
 
+  delete module_factory_;
+}
 
 bool SourceLineResolverBase::ReadSymbolFile(char **symbol_data,
                                             const string &map_file) {
@@ -100,7 +98,7 @@ bool SourceLineResolverBase::ReadSymbolFile(char **symbol_data,
 
   // Allocate memory for file contents, plus a null terminator
   // since we may use strtok() on the contents.
-  *symbol_data = reinterpret_cast<char*>(operator new(file_size + 1));
+  *symbol_data = new char[file_size + 1];
 
   if (*symbol_data == NULL) {
     BPLOG(ERROR) << "Could not allocate memory for " << map_file;
@@ -115,7 +113,7 @@ bool SourceLineResolverBase::ReadSymbolFile(char **symbol_data,
     int error_code = ErrnoString(&error_string);
     BPLOG(ERROR) << "Could not open " << map_file <<
         ", error " << error_code << ": " << error_string;
-    delete (*symbol_data);
+    delete [] (*symbol_data);
     *symbol_data = NULL;
     return false;
   }
@@ -131,7 +129,7 @@ bool SourceLineResolverBase::ReadSymbolFile(char **symbol_data,
     int error_code = ErrnoString(&error_string);
     BPLOG(ERROR) << "Could not slurp " << map_file <<
         ", error " << error_code << ": " << error_string;
-    delete (*symbol_data);
+    delete [] (*symbol_data);
     *symbol_data = NULL;
     return false;
   }
@@ -161,59 +159,80 @@ bool SourceLineResolverBase::LoadModule(const CodeModule *module,
 
   BPLOG(INFO) << "Read symbol file " << map_file << " succeeded";
 
-  // Invoke helper method, let the concrete subclass decides its own action.
-  StoreDataBeforeLoad(module, memory_buffer);
+  bool load_result = LoadModuleUsingMemoryBuffer(module, memory_buffer);
+
+  if (load_result && !ShouldDeleteMemoryBufferAfterLoadModule()) {
+    // memory_buffer has to stay alive as long as the module.
+    memory_buffers_->insert(make_pair(module->code_file(), memory_buffer));
+  } else {
+    delete [] memory_buffer;
+  }
 
-  return LoadModuleUsingMemoryBuffer(module, memory_buffer);
+  return load_result;
 }
 
 bool SourceLineResolverBase::LoadModuleUsingMapBuffer(
     const CodeModule *module, const string &map_buffer) {
-  char *memory_buffer = reinterpret_cast<char*>(
-      operator new(map_buffer.size() + 1));
-  if (memory_buffer == NULL)
+  if (module == NULL)
+    return false;
+
+  // Make sure we don't already have a module with the given name.
+  if (modules_->find(module->code_file()) != modules_->end()) {
+    BPLOG(INFO) << "Symbols for module " << module->code_file()
+                << " already loaded";
     return false;
+  }
+
+  char *memory_buffer = new char[map_buffer.size() + 1];
+  if (memory_buffer == NULL) {
+    BPLOG(ERROR) << "Could not allocate memory for " << module->code_file();
+    return false;
+  }
 
   // Can't use strcpy, as the data may contain '\0's before the end.
   memcpy(memory_buffer, map_buffer.c_str(), map_buffer.size());
   memory_buffer[map_buffer.size()] = '\0';
 
-  // Invoke helper method, let the concrete subclass decides its own action.
-  StoreDataBeforeLoad(module, memory_buffer);
+  bool load_result = LoadModuleUsingMemoryBuffer(module, memory_buffer);
+
+  if (load_result && !ShouldDeleteMemoryBufferAfterLoadModule()) {
+    // memory_buffer has to stay alive as long as the module.
+    memory_buffers_->insert(make_pair(module->code_file(), memory_buffer));
+  } else {
+    delete [] memory_buffer;
+  }
 
-  return LoadModuleUsingMemoryBuffer(module, memory_buffer);
+  return load_result;
 }
 
 bool SourceLineResolverBase::LoadModuleUsingMemoryBuffer(
     const CodeModule *module, char *memory_buffer) {
-  if (!module) {
-    // Invoke helper method, let the concrete subclass decides its own action.
-    DeleteDataAfterLoad(memory_buffer);
+  if (!module)
     return false;
-  }
 
   // Make sure we don't already have a module with the given name.
   if (modules_->find(module->code_file()) != modules_->end()) {
     BPLOG(INFO) << "Symbols for module " << module->code_file()
                 << " already loaded";
-    DeleteDataAfterLoad(memory_buffer);
     return false;
   }
 
   BPLOG(INFO) << "Loading symbols for module " << module->code_file()
-             << " from buffer";
+             << " from memory buffer";
 
   Module *basic_module = module_factory_->CreateModule(module->code_file());
 
   // Ownership of memory is NOT transfered to Module::LoadMapFromMemory().
   if (!basic_module->LoadMapFromMemory(memory_buffer)) {
     delete basic_module;
-    DeleteDataAfterLoad(memory_buffer);
     return false;
   }
 
   modules_->insert(make_pair(module->code_file(), basic_module));
-  DeleteDataAfterLoad(memory_buffer);
+  return true;
+}
+
+bool SourceLineResolverBase::ShouldDeleteMemoryBufferAfterLoadModule() {
   return true;
 }
 
@@ -228,7 +247,16 @@ void SourceLineResolverBase::UnloadModule(const CodeModule *code_module) {
     modules_->erase(iter);
   }
 
-  DeleteDataUnload(code_module);
+  if (ShouldDeleteMemoryBufferAfterLoadModule()) {
+    // No-op.  Because we never store any memory buffers.
+  } else {
+    // There may be a buffer stored locally, we need to find and delete it.
+    MemoryMap::iterator iter = memory_buffers_->find(code_module->code_file());
+    if (iter != memory_buffers_->end()) {
+      delete [] iter->second;
+      memory_buffers_->erase(iter);
+    }
+  }
 }
 
 bool SourceLineResolverBase::HasModule(const CodeModule *module) {
diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc
index 296697cb..58de27dc 100644
--- a/src/processor/stackwalker.cc
+++ b/src/processor/stackwalker.cc
@@ -116,6 +116,9 @@ bool Stackwalker::Walk(CallStack *stack) {
             case SymbolSupplier::INTERRUPT:
               return false;
           }
+          // Inform symbol supplier to free the unused data memory buffer.
+          if (resolver_->ShouldDeleteMemoryBufferAfterLoadModule())
+            supplier_->FreeSymbolData(module);
         }
         resolver_->FillSourceLineInfo(frame.get());
       }
diff --git a/src/processor/stackwalker_unittest_utils.h b/src/processor/stackwalker_unittest_utils.h
index 2da71bc0..d2e29f72 100644
--- a/src/processor/stackwalker_unittest_utils.h
+++ b/src/processor/stackwalker_unittest_utils.h
@@ -174,6 +174,7 @@ class MockSymbolSupplier: public google_breakpad::SymbolSupplier {
                                                   const SystemInfo *system_info,
                                                   std::string *symbol_file,
                                                   char **symbol_data));
+  MOCK_METHOD1(FreeSymbolData, void(const CodeModule *module));
 };
 
 #endif // PROCESSOR_STACKWALKER_UNITTEST_UTILS_H_
diff --git a/src/tools/mac/crash_report/on_demand_symbol_supplier.h b/src/tools/mac/crash_report/on_demand_symbol_supplier.h
index 2bad776d..28002c6b 100644
--- a/src/tools/mac/crash_report/on_demand_symbol_supplier.h
+++ b/src/tools/mac/crash_report/on_demand_symbol_supplier.h
@@ -61,10 +61,16 @@ class OnDemandSymbolSupplier : public SymbolSupplier {
                                      const SystemInfo *system_info,
                                      string *symbol_file,
                                      string *symbol_data);
+  // Allocates data buffer on heap, and takes the ownership of
+  // the data buffer.
   virtual SymbolResult GetCStringSymbolData(const CodeModule *module,
                                             const SystemInfo *system_info,
                                             string *symbol_file,
                                             char **symbol_data);
+
+  // Delete the data buffer allocated for module in GetCStringSymbolData().
+  virtual void FreeSymbolData(const CodeModule *module);
+
  protected:
   // Search directory
   string search_dir_;
@@ -74,6 +80,9 @@ class OnDemandSymbolSupplier : public SymbolSupplier {
   // and the path to that module's symbol file.
   map<string, string> module_file_map_;
 
+  // Map of allocated data buffers, keyed by module->code_file().
+  map<string, char *> memory_buffers_;
+
   // Return the name for |module|  This will be the value used as the key
   // to the |module_file_map_|.
   string GetNameForModule(const CodeModule *module);
diff --git a/src/tools/mac/crash_report/on_demand_symbol_supplier.mm b/src/tools/mac/crash_report/on_demand_symbol_supplier.mm
index e1db9203..f71aebd9 100644
--- a/src/tools/mac/crash_report/on_demand_symbol_supplier.mm
+++ b/src/tools/mac/crash_report/on_demand_symbol_supplier.mm
@@ -32,6 +32,7 @@
 #include <string>
 #include <iostream>
 #include <fstream>
+#include <utility>
 
 #include "google_breakpad/processor/basic_source_line_resolver.h"
 #include "google_breakpad/processor/minidump.h"
@@ -49,7 +50,7 @@ using google_breakpad::PathnameStripper;
 using google_breakpad::SymbolSupplier;
 using google_breakpad::SystemInfo;
 
-OnDemandSymbolSupplier::OnDemandSymbolSupplier(const string &search_dir, 
+OnDemandSymbolSupplier::OnDemandSymbolSupplier(const string &search_dir,
                                                const string &symbol_search_dir)
   : search_dir_(search_dir) {
   NSFileManager *mgr = [NSFileManager defaultManager];
@@ -169,10 +170,27 @@ OnDemandSymbolSupplier::GetCStringSymbolData(const CodeModule *module,
                                                       system_info,
                                                       symbol_file,
                                                       &symbol_data_string);
-  strcpy(*symbol_data, symbol_data_string.c_str());
+  if (result == FOUND) {
+    unsigned int size = symbol_data_string.size() + 1;
+    *symbol_data = new char[size];
+    if (*symbol_data == NULL) {
+      // Should return INTERRUPT on memory allocation failure.
+      return INTERRUPT;
+    }
+    strcpy(*symbol_data, symbol_data_string.c_str());
+    memory_buffers_.insert(make_pair(module->code_file(), *symbol_data);
+  }
   return result;
 }
 
+void OnDemandSymbolSupplier::FreeSymbolData(const CodeModule *module) {
+  map<string, char *>::iterator it = memory_buffers_.find(module->code_file());
+  if (it != memory_buffers_.end()) {
+    delete [] it->second;
+    memory_buffers_.erase(it);
+  }
+}
+
 string OnDemandSymbolSupplier::GetLocalModulePath(const CodeModule *module) {
   NSFileManager *mgr = [NSFileManager defaultManager];
   const char *moduleStr = module->code_file().c_str();
@@ -281,9 +299,9 @@ bool OnDemandSymbolSupplier::GenerateSymbolFile(const CodeModule *module,
         } else {
           printf("Unable to open %s (%d)\n", name.c_str(), errno);
           result = false;
-        } 
+        }
       } else {
-        printf("Architecture %s not available for %s\n", 
+        printf("Architecture %s not available for %s\n",
                system_info->cpu.c_str(), name.c_str());
         result = false;
       }