Populate stack frames with unloaded module info.

This CL hits lots of source files because:
 1. An update to the CodeModule virtual class. I added an is_loaded
  method to specify whether the module is loaded. There were several
  mocks/test classes that needed to be updated with an implementation.
  An alternative to this route would be to modify
  MinidumpUnloadedModule::code_file to prepend "Unloaded_" to the
  module name.

 2. Added an unloaded_modules parameter to
  StackFrameSymbolizer::FillSourceLineInfo.

BUG=

Change-Id: Ic9c7f7c7b7e932a154a5d4ccf292c1527d8da09f
Reviewed-on: https://chromium-review.googlesource.com/430241
Reviewed-by: Ivan Penkov <ivanpe@chromium.org>

15 files changed, 161 insertions, 13 deletions

diff --git a/src/google_breakpad/processor/code_module.h b/src/google_breakpad/processor/code_module.h
index b139907c..29b8d9c9 100644
--- a/src/google_breakpad/processor/code_module.h
+++ b/src/google_breakpad/processor/code_module.h
@@ -94,6 +94,9 @@ class CodeModule {
   // should always reflect the original values (reported in the minidump).
   virtual uint64_t shrink_down_delta() const = 0;
   virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) = 0;
+
+  // Whether the module was unloaded from memory.
+  virtual bool is_unloaded() const = 0;
 };
 
 }  // namespace google_breakpad
diff --git a/src/google_breakpad/processor/minidump.h b/src/google_breakpad/processor/minidump.h
index 1bfc7d9e..bff38bf3 100644
--- a/src/google_breakpad/processor/minidump.h
+++ b/src/google_breakpad/processor/minidump.h
@@ -399,6 +399,7 @@ class MinidumpModule : public MinidumpObject,
   virtual string debug_identifier() const;
   virtual string version() const;
   virtual CodeModule* Copy() const;
+  virtual bool is_unloaded() const { return false; }
 
   // Getter and setter for shrink_down_delta.  This is used when the address
   // range for a module is shrunk down due to address range conflicts with
@@ -775,6 +776,7 @@ class MinidumpUnloadedModule : public MinidumpObject,
   string debug_identifier() const override;
   string version() const override;
   CodeModule* Copy() const override;
+  bool is_unloaded() const override { return true; }
   uint64_t shrink_down_delta() const override;
   void SetShrinkDownDelta(uint64_t shrink_down_delta) override;
 
diff --git a/src/google_breakpad/processor/process_state.h b/src/google_breakpad/processor/process_state.h
index 9f12b0c6..21bef42c 100644
--- a/src/google_breakpad/processor/process_state.h
+++ b/src/google_breakpad/processor/process_state.h
@@ -91,7 +91,7 @@ enum ExploitabilityRating {
 
 class ProcessState {
  public:
-  ProcessState() : modules_(NULL) { Clear(); }
+  ProcessState() : modules_(NULL), unloaded_modules_(NULL) { Clear(); }
   ~ProcessState();
 
   // Resets the ProcessState to its default values
@@ -111,6 +111,7 @@ class ProcessState {
   }
   const SystemInfo* system_info() const { return &system_info_; }
   const CodeModules* modules() const { return modules_; }
+  const CodeModules* unloaded_modules() const { return unloaded_modules_; }
   const vector<linked_ptr<const CodeModule> >* shrunk_range_modules() const {
     return &shrunk_range_modules_;
   }
@@ -177,6 +178,10 @@ class ProcessState {
   // ProcessState.
   const CodeModules *modules_;
 
+  // The modules that have been unloaded from the process represented by the
+  // ProcessState.
+  const CodeModules *unloaded_modules_;
+
   // The modules which virtual address ranges were shrunk down due to
   // virtual address conflicts.
   vector<linked_ptr<const CodeModule> > shrunk_range_modules_;
diff --git a/src/google_breakpad/processor/stack_frame_symbolizer.h b/src/google_breakpad/processor/stack_frame_symbolizer.h
index 074907cb..0bbaae0a 100644
--- a/src/google_breakpad/processor/stack_frame_symbolizer.h
+++ b/src/google_breakpad/processor/stack_frame_symbolizer.h
@@ -75,9 +75,11 @@ class StackFrameSymbolizer {
 
   // Encapsulate the step of resolving source line info for a stack frame.
   // "frame" must not be NULL.
-  virtual SymbolizerResult FillSourceLineInfo(const CodeModules* modules,
-                                              const SystemInfo* system_info,
-                                              StackFrame* stack_frame);
+  virtual SymbolizerResult FillSourceLineInfo(
+      const CodeModules* modules,
+      const CodeModules* unloaded_modules,
+      const SystemInfo* system_info,
+      StackFrame* stack_frame);
 
   virtual WindowsFrameInfo* FindWindowsFrameInfo(const StackFrame* frame);
 
diff --git a/src/google_breakpad/processor/stackwalker.h b/src/google_breakpad/processor/stackwalker.h
index a1bd3e7f..4378f75b 100644
--- a/src/google_breakpad/processor/stackwalker.h
+++ b/src/google_breakpad/processor/stackwalker.h
@@ -89,8 +89,10 @@ class Stackwalker {
      DumpContext* context,
      MemoryRegion* memory,
      const CodeModules* modules,
+     const CodeModules* unloaded_modules,
      StackFrameSymbolizer* resolver_helper);
 
+
   static void set_max_frames(uint32_t max_frames) {
     max_frames_ = max_frames;
     max_frames_set_ = true;
@@ -189,6 +191,11 @@ class Stackwalker {
   // This field is optional and may be NULL.
   const CodeModules* modules_;
 
+  // A list of unloaded modules, for populating frames which aren't matched
+  // to any loaded modules.
+  // This field is optional and may be NULL.
+  const CodeModules* unloaded_modules_;
+
  protected:
   // The StackFrameSymbolizer implementation.
   StackFrameSymbolizer* frame_symbolizer_;
diff --git a/src/processor/basic_code_module.h b/src/processor/basic_code_module.h
index 0f7b3e43..35d66a60 100644
--- a/src/processor/basic_code_module.h
+++ b/src/processor/basic_code_module.h
@@ -62,14 +62,16 @@ class BasicCodeModule : public CodeModule {
         code_identifier_(that->code_identifier()),
         debug_file_(that->debug_file()),
         debug_identifier_(that->debug_identifier()),
-        version_(that->version()) {}
+        version_(that->version()),
+        is_unloaded_(that->is_unloaded()) {}
 
   BasicCodeModule(uint64_t base_address, uint64_t size,
                   const string &code_file,
                   const string &code_identifier,
                   const string &debug_file,
                   const string &debug_identifier,
-                  const string &version)
+                  const string &version,
+                  const bool is_unloaded = false)
       : base_address_(base_address),
         size_(size),
         shrink_down_delta_(0),
@@ -77,7 +79,8 @@ class BasicCodeModule : public CodeModule {
         code_identifier_(code_identifier),
         debug_file_(debug_file),
         debug_identifier_(debug_identifier),
-        version_(version)
+        version_(version),
+        is_unloaded_(is_unloaded)
     {}
   virtual ~BasicCodeModule() {}
 
@@ -95,6 +98,7 @@ class BasicCodeModule : public CodeModule {
   virtual string debug_identifier() const { return debug_identifier_; }
   virtual string version() const { return version_; }
   virtual CodeModule* Copy() const { return new BasicCodeModule(this); }
+  virtual bool is_unloaded() const { return is_unloaded_; }
 
  private:
   uint64_t base_address_;
@@ -105,6 +109,7 @@ class BasicCodeModule : public CodeModule {
   string debug_file_;
   string debug_identifier_;
   string version_;
+  bool is_unloaded_;
 
   // Disallow copy constructor and assignment operator.
   BasicCodeModule(const BasicCodeModule &that);
diff --git a/src/processor/basic_source_line_resolver_unittest.cc b/src/processor/basic_source_line_resolver_unittest.cc
index a75044c7..9fab8ca6 100644
--- a/src/processor/basic_source_line_resolver_unittest.cc
+++ b/src/processor/basic_source_line_resolver_unittest.cc
@@ -71,6 +71,7 @@ class TestCodeModule : public CodeModule {
   virtual CodeModule* Copy() const {
     return new TestCodeModule(code_file_);
   }
+  virtual bool is_unloaded() const { return false; }
   virtual uint64_t shrink_down_delta() const { return 0; }
   virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {}
 
diff --git a/src/processor/fast_source_line_resolver_unittest.cc b/src/processor/fast_source_line_resolver_unittest.cc
index c7215228..87b13c52 100644
--- a/src/processor/fast_source_line_resolver_unittest.cc
+++ b/src/processor/fast_source_line_resolver_unittest.cc
@@ -82,6 +82,7 @@ class TestCodeModule : public CodeModule {
   virtual CodeModule* Copy() const {
     return new TestCodeModule(code_file_);
   }
+  virtual bool is_unloaded() const { return false; }
   virtual uint64_t shrink_down_delta() const { return 0; }
   virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {}
 
diff --git a/src/processor/microdump_processor.cc b/src/processor/microdump_processor.cc
index 366e3f30..2d67f861 100644
--- a/src/processor/microdump_processor.cc
+++ b/src/processor/microdump_processor.cc
@@ -73,6 +73,7 @@ ProcessResult MicrodumpProcessor::Process(const string &microdump_contents,
                             microdump.GetContext(),
                             microdump.GetMemory(),
                             process_state->modules_,
+                            /* unloaded_modules= */ NULL,
                             frame_symbolizer_));
 
   scoped_ptr<CallStack> stack(new CallStack());
diff --git a/src/processor/minidump_processor.cc b/src/processor/minidump_processor.cc
index e80ebc38..c522adff 100644
--- a/src/processor/minidump_processor.cc
+++ b/src/processor/minidump_processor.cc
@@ -141,6 +141,12 @@ ProcessResult MinidumpProcessor::Process(
     }
   }
 
+  MinidumpUnloadedModuleList *unloaded_module_list =
+      dump->GetUnloadedModuleList();
+  if (unloaded_module_list) {
+    process_state->unloaded_modules_ = unloaded_module_list->Copy();
+  }
+
   MinidumpMemoryList *memory_list = dump->GetMemoryList();
   if (memory_list) {
     BPLOG(INFO) << "Found " << memory_list->region_count()
@@ -262,6 +268,7 @@ ProcessResult MinidumpProcessor::Process(
                                        context,
                                        thread_memory,
                                        process_state->modules_,
+                                       process_state->unloaded_modules_,
                                        frame_symbolizer_));
 
     scoped_ptr<CallStack> stack(new CallStack());
diff --git a/src/processor/minidump_processor_unittest.cc b/src/processor/minidump_processor_unittest.cc
index d43c1fc9..d9d974df 100644
--- a/src/processor/minidump_processor_unittest.cc
+++ b/src/processor/minidump_processor_unittest.cc
@@ -71,9 +71,24 @@ class MockMinidump : public Minidump {
   MOCK_METHOD0(GetException, MinidumpException*());
   MOCK_METHOD0(GetAssertion, MinidumpAssertion*());
   MOCK_METHOD0(GetModuleList, MinidumpModuleList*());
+  MOCK_METHOD0(GetUnloadedModuleList, MinidumpUnloadedModuleList*());
   MOCK_METHOD0(GetMemoryList, MinidumpMemoryList*());
 };
 
+class MockMinidumpUnloadedModule : public MinidumpUnloadedModule {
+ public:
+  MockMinidumpUnloadedModule() : MinidumpUnloadedModule(NULL) {}
+};
+
+class MockMinidumpUnloadedModuleList : public MinidumpUnloadedModuleList {
+ public:
+  MockMinidumpUnloadedModuleList() : MinidumpUnloadedModuleList(NULL) {}
+
+  MOCK_CONST_METHOD0(Copy, CodeModules*());
+  MOCK_CONST_METHOD1(GetModuleForAddress,
+                     const MinidumpUnloadedModule*(uint64_t));
+};
+
 class MockMinidumpThreadList : public MinidumpThreadList {
  public:
   MockMinidumpThreadList() : MinidumpThreadList(NULL) {}
@@ -157,6 +172,8 @@ using google_breakpad::MockMinidumpMemoryList;
 using google_breakpad::MockMinidumpMemoryRegion;
 using google_breakpad::MockMinidumpThread;
 using google_breakpad::MockMinidumpThreadList;
+using google_breakpad::MockMinidumpUnloadedModule;
+using google_breakpad::MockMinidumpUnloadedModuleList;
 using google_breakpad::ProcessState;
 using google_breakpad::scoped_ptr;
 using google_breakpad::SymbolSupplier;
@@ -320,6 +337,88 @@ class TestMinidumpContext : public MinidumpContext {
 class MinidumpProcessorTest : public ::testing::Test {
 };
 
+TEST_F(MinidumpProcessorTest, TestUnloadedModules) {
+  MockMinidump dump;
+
+  EXPECT_CALL(dump, path()).WillRepeatedly(Return("mock minidump"));
+  EXPECT_CALL(dump, Read()).WillRepeatedly(Return(true));
+
+  MDRawHeader fake_header;
+  fake_header.time_date_stamp = 0;
+  EXPECT_CALL(dump, header()).WillRepeatedly(Return(&fake_header));
+
+  MDRawSystemInfo raw_system_info;
+  memset(&raw_system_info, 0, sizeof(raw_system_info));
+  raw_system_info.processor_architecture = MD_CPU_ARCHITECTURE_X86;
+  raw_system_info.platform_id = MD_OS_WIN32_NT;
+  TestMinidumpSystemInfo dump_system_info(raw_system_info);
+
+  EXPECT_CALL(dump, GetSystemInfo()).
+      WillRepeatedly(Return(&dump_system_info));
+
+  // No loaded modules
+
+  MockMinidumpUnloadedModuleList unloaded_module_list;
+  EXPECT_CALL(dump, GetUnloadedModuleList()).
+      WillOnce(Return(&unloaded_module_list));
+
+  MockMinidumpMemoryList memory_list;
+  EXPECT_CALL(dump, GetMemoryList()).
+      WillOnce(Return(&memory_list));
+
+  MockMinidumpThreadList thread_list;
+  EXPECT_CALL(dump, GetThreadList()).
+      WillOnce(Return(&thread_list));
+
+  EXPECT_CALL(thread_list, thread_count()).
+    WillRepeatedly(Return(1));
+
+  MockMinidumpThread thread;
+  EXPECT_CALL(thread_list, GetThreadAtIndex(0)).
+    WillOnce(Return(&thread));
+
+  EXPECT_CALL(thread, GetThreadID(_)).
+    WillRepeatedly(DoAll(SetArgumentPointee<0>(1),
+                         Return(true)));
+
+  MDRawContextX86 thread_raw_context;
+  memset(&thread_raw_context, 0,
+         sizeof(thread_raw_context));
+  thread_raw_context.context_flags = MD_CONTEXT_X86_FULL;
+  const uint32_t kExpectedEIP = 0xabcd1234;
+  thread_raw_context.eip = kExpectedEIP;
+  TestMinidumpContext thread_context(thread_raw_context);
+  EXPECT_CALL(thread, GetContext()).
+    WillRepeatedly(Return(&thread_context));
+
+  // The memory contents don't really matter here, since it won't be used.
+  MockMinidumpMemoryRegion thread_memory(0x1234, "xxx");
+  EXPECT_CALL(thread, GetMemory()).
+    WillRepeatedly(Return(&thread_memory));
+  EXPECT_CALL(thread, GetStartOfStackMemoryRange()).
+    Times(0);
+  EXPECT_CALL(memory_list, GetMemoryRegionForAddress(_)).
+    Times(0);
+
+  EXPECT_CALL(unloaded_module_list, Copy()).
+      WillOnce(Return(&unloaded_module_list));
+
+  MockMinidumpUnloadedModule unloaded_module;
+  EXPECT_CALL(unloaded_module_list, GetModuleForAddress(kExpectedEIP)).
+      WillOnce(Return(&unloaded_module));
+
+  MinidumpProcessor processor(reinterpret_cast<SymbolSupplier*>(NULL), NULL);
+  ProcessState state;
+  EXPECT_EQ(processor.Process(&dump, &state),
+            google_breakpad::PROCESS_OK);
+
+  // The single frame should be populated with the unloaded module.
+  ASSERT_EQ(1U, state.threads()->size());
+  ASSERT_EQ(1U, state.threads()->at(0)->frames()->size());
+  ASSERT_EQ(kExpectedEIP, state.threads()->at(0)->frames()->at(0)->instruction);
+  ASSERT_EQ(&unloaded_module, state.threads()->at(0)->frames()->at(0)->module);
+}
+
 TEST_F(MinidumpProcessorTest, TestCorruptMinidumps) {
   MockMinidump dump;
   TestSymbolSupplier supplier;
diff --git a/src/processor/process_state.cc b/src/processor/process_state.cc
index 5a5cd7f6..90ae93d7 100644
--- a/src/processor/process_state.cc
+++ b/src/processor/process_state.cc
@@ -64,6 +64,7 @@ void ProcessState::Clear() {
   modules_with_corrupt_symbols_.clear();
   delete modules_;
   modules_ = NULL;
+  unloaded_modules_ = NULL;
 }
 
 }  // namespace google_breakpad
diff --git a/src/processor/stack_frame_symbolizer.cc b/src/processor/stack_frame_symbolizer.cc
index 5c8dbe5e..7a44f243 100644
--- a/src/processor/stack_frame_symbolizer.cc
+++ b/src/processor/stack_frame_symbolizer.cc
@@ -55,12 +55,19 @@ StackFrameSymbolizer::StackFrameSymbolizer(
 
 StackFrameSymbolizer::SymbolizerResult StackFrameSymbolizer::FillSourceLineInfo(
     const CodeModules* modules,
+    const CodeModules* unloaded_modules,
     const SystemInfo* system_info,
     StackFrame* frame) {
   assert(frame);
 
-  if (!modules) return kError;
-  const CodeModule* module = modules->GetModuleForAddress(frame->instruction);
+  const CodeModule* module = NULL;
+  if (modules) {
+    module = modules->GetModuleForAddress(frame->instruction);
+  }
+  if (!module && unloaded_modules) {
+    module = unloaded_modules->GetModuleForAddress(frame->instruction);
+  }
+
   if (!module) return kError;
   frame->module = module;
 
diff --git a/src/processor/stackwalker.cc b/src/processor/stackwalker.cc
index 98cb0b5b..c85ce5e8 100644
--- a/src/processor/stackwalker.cc
+++ b/src/processor/stackwalker.cc
@@ -72,6 +72,7 @@ Stackwalker::Stackwalker(const SystemInfo* system_info,
     : system_info_(system_info),
       memory_(memory),
       modules_(modules),
+      unloaded_modules_(NULL),
       frame_symbolizer_(frame_symbolizer) {
   assert(frame_symbolizer_);
 }
@@ -134,8 +135,9 @@ bool Stackwalker::Walk(
 
     // Resolve the module information, if a module map was provided.
     StackFrameSymbolizer::SymbolizerResult symbolizer_result =
-        frame_symbolizer_->FillSourceLineInfo(modules_, system_info_,
-                                             frame.get());
+        frame_symbolizer_->FillSourceLineInfo(modules_, unloaded_modules_,
+                                              system_info_,
+                                              frame.get());
     switch (symbolizer_result) {
       case StackFrameSymbolizer::kInterrupt:
         BPLOG(INFO) << "Stack walk is interrupted.";
@@ -186,13 +188,13 @@ bool Stackwalker::Walk(
   return true;
 }
 
-
 // static
 Stackwalker* Stackwalker::StackwalkerForCPU(
     const SystemInfo* system_info,
     DumpContext* context,
     MemoryRegion* memory,
     const CodeModules* modules,
+    const CodeModules* unloaded_modules,
     StackFrameSymbolizer* frame_symbolizer) {
   if (!context) {
     BPLOG(ERROR) << "Can't choose a stackwalker implementation without context";
@@ -263,6 +265,9 @@ Stackwalker* Stackwalker::StackwalkerForCPU(
   BPLOG_IF(ERROR, !cpu_stackwalker) << "Unknown CPU type " << HexString(cpu) <<
                                        ", can't choose a stackwalker "
                                        "implementation";
+  if (cpu_stackwalker) {
+    cpu_stackwalker->unloaded_modules_ = unloaded_modules;
+  }
   return cpu_stackwalker;
 }
 
@@ -270,7 +275,8 @@ bool Stackwalker::InstructionAddressSeemsValid(uint64_t address) {
   StackFrame frame;
   frame.instruction = address;
   StackFrameSymbolizer::SymbolizerResult symbolizer_result =
-      frame_symbolizer_->FillSourceLineInfo(modules_, system_info_, &frame);
+      frame_symbolizer_->FillSourceLineInfo(modules_, unloaded_modules_,
+                                            system_info_, &frame);
 
   if (!frame.module) {
     // not inside any loaded module
diff --git a/src/processor/stackwalker_unittest_utils.h b/src/processor/stackwalker_unittest_utils.h
index 1523b247..d7f34755 100644
--- a/src/processor/stackwalker_unittest_utils.h
+++ b/src/processor/stackwalker_unittest_utils.h
@@ -118,6 +118,7 @@ class MockCodeModule: public google_breakpad::CodeModule {
   google_breakpad::CodeModule *Copy() const {
     abort(); // Tests won't use this.
   }
+  virtual bool is_unloaded() const { return false; }
   virtual uint64_t shrink_down_delta() const { return 0; }
   virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {}