Server-side workaround to handle overlapping modules.

This change is resolving an issue that was caused by the combination of:
 - Android system libraries being relro packed in N+.
 - Breakpad dealing with relro packed libraries in a hack way.

This is a fix for http://crbug/611824.

I also found an use-after-free issue (bug in Minidump::SeekToStreamType).  I disallowed the MinidumpStreamInfo copy and assign constructors and the compiler detected another similar issue in Minidump::Print.  Then I disabled the copy and assign constructors for most classes in minidump.h (just in case).  There are a couple of classes where I couldn't disallow them (since assign is used).  This will require a small refactor so I left it out of this CL.

R=mark@chromium.org

Review URL: https://codereview.chromium.org/2060663002 .

12 files changed, 158 insertions, 36 deletions

diff --git a/src/processor/basic_code_module.h b/src/processor/basic_code_module.h
index 3fe782bb..0f7b3e43 100644
--- a/src/processor/basic_code_module.h
+++ b/src/processor/basic_code_module.h
@@ -57,6 +57,7 @@ class BasicCodeModule : public CodeModule {
   explicit BasicCodeModule(const CodeModule *that)
       : base_address_(that->base_address()),
         size_(that->size()),
+        shrink_down_delta_(that->shrink_down_delta()),
         code_file_(that->code_file()),
         code_identifier_(that->code_identifier()),
         debug_file_(that->debug_file()),
@@ -64,18 +65,19 @@ class BasicCodeModule : public CodeModule {
         version_(that->version()) {}
 
   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)
-    : base_address_(base_address),
-      size_(size),
-      code_file_(code_file),
-      code_identifier_(code_identifier),
-      debug_file_(debug_file),
-      debug_identifier_(debug_identifier),
-      version_(version)
+                  const string &code_file,
+                  const string &code_identifier,
+                  const string &debug_file,
+                  const string &debug_identifier,
+                  const string &version)
+      : base_address_(base_address),
+        size_(size),
+        shrink_down_delta_(0),
+        code_file_(code_file),
+        code_identifier_(code_identifier),
+        debug_file_(debug_file),
+        debug_identifier_(debug_identifier),
+        version_(version)
     {}
   virtual ~BasicCodeModule() {}
 
@@ -83,16 +85,21 @@ class BasicCodeModule : public CodeModule {
   // members.
   virtual uint64_t base_address() const { return base_address_; }
   virtual uint64_t size() const { return size_; }
+  virtual uint64_t shrink_down_delta() const { return shrink_down_delta_; }
+  virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {
+    shrink_down_delta_ = shrink_down_delta;
+  }
   virtual string code_file() const { return code_file_; }
   virtual string code_identifier() const { return code_identifier_; }
   virtual string debug_file() const { return debug_file_; }
   virtual string debug_identifier() const { return debug_identifier_; }
   virtual string version() const { return version_; }
-  virtual const CodeModule* Copy() const { return new BasicCodeModule(this); }
+  virtual CodeModule* Copy() const { return new BasicCodeModule(this); }
 
  private:
   uint64_t base_address_;
   uint64_t size_;
+  uint64_t shrink_down_delta_;
   string code_file_;
   string code_identifier_;
   string debug_file_;
diff --git a/src/processor/basic_code_modules.cc b/src/processor/basic_code_modules.cc
index 00dc6fcb..48d97167 100644
--- a/src/processor/basic_code_modules.cc
+++ b/src/processor/basic_code_modules.cc
@@ -38,6 +38,8 @@
 
 #include <assert.h>
 
+#include <vector>
+
 #include "google_breakpad/processor/code_module.h"
 #include "processor/linked_ptr.h"
 #include "processor/logging.h"
@@ -45,31 +47,50 @@
 
 namespace google_breakpad {
 
+using std::vector;
+
 BasicCodeModules::BasicCodeModules(const CodeModules *that)
     : main_address_(0), map_() {
   BPLOG_IF(ERROR, !that) << "BasicCodeModules::BasicCodeModules requires "
                             "|that|";
   assert(that);
 
+  map_.SetEnableShrinkDown(that->IsModuleShrinkEnabled());
+
   const CodeModule *main_module = that->GetMainModule();
   if (main_module)
     main_address_ = main_module->base_address();
 
   unsigned int count = that->module_count();
-  for (unsigned int module_sequence = 0;
-       module_sequence < count;
-       ++module_sequence) {
+  for (unsigned int i = 0; i < count; ++i) {
     // Make a copy of the module and insert it into the map.  Use
     // GetModuleAtIndex because ordering is unimportant when slurping the
     // entire list, and GetModuleAtIndex may be faster than
     // GetModuleAtSequence.
-    linked_ptr<const CodeModule> module(
-        that->GetModuleAtIndex(module_sequence)->Copy());
+    linked_ptr<const CodeModule> module(that->GetModuleAtIndex(i)->Copy());
     if (!map_.StoreRange(module->base_address(), module->size(), module)) {
-      BPLOG(ERROR) << "Module " << module->code_file() <<
-                      " could not be stored";
+      BPLOG(ERROR) << "Module " << module->code_file()
+                   << " could not be stored";
+    }
+  }
+
+  // Report modules with shrunk ranges.
+  for (unsigned int i = 0; i < count; ++i) {
+    linked_ptr<const CodeModule> module(that->GetModuleAtIndex(i)->Copy());
+    uint64_t delta = 0;
+    if (map_.RetrieveRange(module->base_address() + module->size() - 1,
+                           &module, NULL /* base */, &delta, NULL /* size */) &&
+        delta > 0) {
+      BPLOG(INFO) << "The range for module " << module->code_file()
+                  << " was shrunk down by " << HexString(delta) << " bytes.";
+      linked_ptr<CodeModule> shrunk_range_module(module->Copy());
+      shrunk_range_module->SetShrinkDownDelta(delta);
+      shrunk_range_modules_.push_back(shrunk_range_module);
     }
   }
+
+  // TODO(ivanpe): Report modules with conflicting ranges.  The list of such
+  // modules should be copied from |that|.
 }
 
 BasicCodeModules::BasicCodeModules() : main_address_(0), map_() { }
@@ -122,4 +143,13 @@ const CodeModules* BasicCodeModules::Copy() const {
   return new BasicCodeModules(this);
 }
 
+vector<linked_ptr<const CodeModule> >
+BasicCodeModules::GetShrunkRangeModules() const {
+  return shrunk_range_modules_;
+}
+
+bool BasicCodeModules::IsModuleShrinkEnabled() const {
+  return map_.IsShrinkDownEnabled();
+}
+
 }  // namespace google_breakpad
diff --git a/src/processor/basic_code_modules.h b/src/processor/basic_code_modules.h
index 97579b4d..50f8a03d 100644
--- a/src/processor/basic_code_modules.h
+++ b/src/processor/basic_code_modules.h
@@ -43,6 +43,8 @@
 
 #include <stddef.h>
 
+#include <vector>
+
 #include "google_breakpad/processor/code_modules.h"
 #include "processor/linked_ptr.h"
 #include "processor/range_map.h"
@@ -67,6 +69,9 @@ class BasicCodeModules : public CodeModules {
   virtual const CodeModule* GetModuleAtSequence(unsigned int sequence) const;
   virtual const CodeModule* GetModuleAtIndex(unsigned int index) const;
   virtual const CodeModules* Copy() const;
+  virtual std::vector<linked_ptr<const CodeModule> >
+  GetShrunkRangeModules() const;
+  virtual bool IsModuleShrinkEnabled() const;
 
  protected:
   BasicCodeModules();
@@ -78,6 +83,10 @@ class BasicCodeModules : public CodeModules {
   // address range.
   RangeMap<uint64_t, linked_ptr<const CodeModule> > map_;
 
+  // A vector of all CodeModules that were shrunk downs due to
+  // address range conflicts.
+  std::vector<linked_ptr<const CodeModule> > shrunk_range_modules_;
+
  private:
   // Disallow copy constructor and assignment operator.
   BasicCodeModules(const BasicCodeModules &that);
diff --git a/src/processor/basic_source_line_resolver_unittest.cc b/src/processor/basic_source_line_resolver_unittest.cc
index 7d4cd5c5..a75044c7 100644
--- a/src/processor/basic_source_line_resolver_unittest.cc
+++ b/src/processor/basic_source_line_resolver_unittest.cc
@@ -68,9 +68,11 @@ class TestCodeModule : public CodeModule {
   virtual string debug_file() const { return ""; }
   virtual string debug_identifier() const { return ""; }
   virtual string version() const { return ""; }
-  virtual const CodeModule* Copy() const {
+  virtual CodeModule* Copy() const {
     return new TestCodeModule(code_file_);
   }
+  virtual uint64_t shrink_down_delta() const { return 0; }
+  virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {}
 
  private:
   string code_file_;
diff --git a/src/processor/fast_source_line_resolver_unittest.cc b/src/processor/fast_source_line_resolver_unittest.cc
index 72632f84..c7215228 100644
--- a/src/processor/fast_source_line_resolver_unittest.cc
+++ b/src/processor/fast_source_line_resolver_unittest.cc
@@ -79,9 +79,11 @@ class TestCodeModule : public CodeModule {
   virtual string debug_file() const { return ""; }
   virtual string debug_identifier() const { return ""; }
   virtual string version() const { return ""; }
-  virtual const CodeModule* Copy() const {
+  virtual CodeModule* Copy() const {
     return new TestCodeModule(code_file_);
   }
+  virtual uint64_t shrink_down_delta() const { return 0; }
+  virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {}
 
  private:
   string code_file_;
diff --git a/src/processor/microdump.cc b/src/processor/microdump.cc
index 9073fe10..4af62f56 100644
--- a/src/processor/microdump.cc
+++ b/src/processor/microdump.cc
@@ -110,6 +110,9 @@ void MicrodumpModules::Add(const CodeModule* module) {
   }
 }
 
+void MicrodumpModules::SetEnableModuleShrink(bool is_enabled) {
+  map_.SetEnableShrinkDown(is_enabled);
+}
 
 //
 // MicrodumpContext
@@ -262,6 +265,7 @@ Microdump::Microdump(const string& contents)
       } else if (os_id == "A") {
         system_info_->os = "Android";
         system_info_->os_short = "android";
+        modules_->SetEnableModuleShrink(true);
       }
 
       // OS line also contains release and version for future use.
diff --git a/src/processor/microdump_processor_unittest.cc b/src/processor/microdump_processor_unittest.cc
index 898b65b2..af897f7d 100644
--- a/src/processor/microdump_processor_unittest.cc
+++ b/src/processor/microdump_processor_unittest.cc
@@ -201,7 +201,7 @@ TEST_F(MicrodumpProcessorTest, TestProcessX86) {
   AnalyzeDump("microdump-x86.dmp", false /* omit_symbols */,
               4 /* expected_cpu_count */, &state);
 
-  ASSERT_EQ(105U, state.modules()->module_count());
+  ASSERT_EQ(124U, state.modules()->module_count());
   ASSERT_EQ("x86", state.system_info()->cpu);
   ASSERT_EQ("asus/WW_Z00A/Z00A:5.0/LRX21V/2.19.40.22_20150627_5104_user:user/"
       "release-keys", state.system_info()->os_version);
@@ -216,11 +216,11 @@ TEST_F(MicrodumpProcessorTest, TestProcessMultiple) {
   ProcessState state;
   AnalyzeDump("microdump-multiple.dmp", false /* omit_symbols */,
               6 /* expected_cpu_count */, &state);
-  ASSERT_EQ(133U, state.modules()->module_count());
+  ASSERT_EQ(156U, state.modules()->module_count());
   ASSERT_EQ("arm", state.system_info()->cpu);
   ASSERT_EQ("lge/p1_tmo_us/p1:6.0/MRA58K/1603210524c8d:user/release-keys",
             state.system_info()->os_version);
-  ASSERT_EQ(2U, state.threads()->at(0)->frames()->size());
+  ASSERT_EQ(5U, state.threads()->at(0)->frames()->size());
 }
 
 TEST_F(MicrodumpProcessorTest, TestProcessMips) {
@@ -249,7 +249,7 @@ TEST_F(MicrodumpProcessorTest, TestProcessMips64) {
   AnalyzeDump("microdump-mips64.dmp", false /* omit_symbols */,
               1 /* expected_cpu_count */, &state);
 
-  ASSERT_EQ(7U, state.modules()->module_count());
+  ASSERT_EQ(8U, state.modules()->module_count());
   ASSERT_EQ("mips64", state.system_info()->cpu);
   ASSERT_EQ("3.10.0-gf185e20 #112 PREEMPT Mon Oct 5 11:12:49 PDT 2015",
         state.system_info()->os_version);
diff --git a/src/processor/minidump.cc b/src/processor/minidump.cc
index 81c3d11a..f94a409a 100644
--- a/src/processor/minidump.cc
+++ b/src/processor/minidump.cc
@@ -2109,11 +2109,21 @@ string MinidumpModule::version() const {
 }
 
 
-const CodeModule* MinidumpModule::Copy() const {
+CodeModule* MinidumpModule::Copy() const {
   return new BasicCodeModule(this);
 }
 
 
+uint64_t MinidumpModule::shrink_down_delta() const {
+  return 0;
+}
+
+void MinidumpModule::SetShrinkDownDelta(uint64_t shrink_down_delta) {
+  // Not implemented
+  assert(false);
+}
+
+
 const uint8_t* MinidumpModule::GetCVRecord(uint32_t* size) {
   if (!module_valid_) {
     BPLOG(ERROR) << "Invalid MinidumpModule for GetCVRecord";
@@ -2497,6 +2507,7 @@ MinidumpModuleList::MinidumpModuleList(Minidump* minidump)
       range_map_(new RangeMap<uint64_t, unsigned int>()),
       modules_(NULL),
       module_count_(0) {
+  range_map_->SetEnableShrinkDown(minidump_->IsAndroid());
 }
 
 
@@ -2709,7 +2720,7 @@ const MinidumpModule* MinidumpModuleList::GetModuleAtSequence(
   }
 
   unsigned int module_index;
-  if (!range_map_->RetrieveRangeAtIndex(sequence, &module_index, 
+  if (!range_map_->RetrieveRangeAtIndex(sequence, &module_index,
                                         NULL /* base */, NULL /* delta */,
                                         NULL /* size */)) {
     BPLOG(ERROR) << "MinidumpModuleList has no module at sequence " << sequence;
@@ -2741,6 +2752,14 @@ const CodeModules* MinidumpModuleList::Copy() const {
   return new BasicCodeModules(this);
 }
 
+vector<linked_ptr<const CodeModule> >
+MinidumpModuleList::GetShrunkRangeModules() const {
+  return vector<linked_ptr<const CodeModule> >();
+}
+
+bool MinidumpModuleList::IsModuleShrinkEnabled() const {
+  return range_map_->IsShrinkDownEnabled();
+}
 
 void MinidumpModuleList::Print() {
   if (!valid_) {
@@ -4532,6 +4551,24 @@ MinidumpLinuxMapsList *Minidump::GetLinuxMapsList() {
   return GetStream(&linux_maps_list);
 }
 
+bool Minidump::IsAndroid() {
+  // Save the current stream position
+  off_t saved_position = Tell();
+  if (saved_position == -1) {
+    return false;
+  }
+  const MDRawSystemInfo* system_info =
+    GetSystemInfo() ? GetSystemInfo()->system_info() : NULL;
+
+  // Restore position and return
+  if (!SeekSet(saved_position)) {
+    BPLOG(ERROR) << "Couldn't seek back to saved position";
+    return false;
+  }
+
+  return system_info && system_info->platform_id == MD_OS_ANDROID;
+}
+
 static const char* get_stream_name(uint32_t stream_type) {
   switch (stream_type) {
   case MD_UNUSED_STREAM:
@@ -4645,7 +4682,7 @@ void Minidump::Print() {
        iterator != stream_map_->end();
        ++iterator) {
     uint32_t stream_type = iterator->first;
-    MinidumpStreamInfo info = iterator->second;
+    const MinidumpStreamInfo& info = iterator->second;
     printf("  stream type 0x%x (%s) at index %d\n", stream_type,
            get_stream_name(stream_type),
            info.stream_index);
@@ -4802,7 +4839,7 @@ bool Minidump::SeekToStreamType(uint32_t  stream_type,
     return false;
   }
 
-  MinidumpStreamInfo info = iterator->second;
+  const MinidumpStreamInfo& info = iterator->second;
   if (info.stream_index >= header_.stream_count) {
     BPLOG(ERROR) << "SeekToStreamType: type " << stream_type <<
                     " out of range: " <<
diff --git a/src/processor/minidump_processor.cc b/src/processor/minidump_processor.cc
index 06a8916d..3ff19518 100644
--- a/src/processor/minidump_processor.cc
+++ b/src/processor/minidump_processor.cc
@@ -126,8 +126,20 @@ ProcessResult MinidumpProcessor::Process(
   // Put a copy of the module list into ProcessState object.  This is not
   // necessarily a MinidumpModuleList, but it adheres to the CodeModules
   // interface, which is all that ProcessState needs to expose.
-  if (module_list)
+  if (module_list) {
     process_state->modules_ = module_list->Copy();
+    process_state->shrunk_range_modules_ =
+        process_state->modules_->GetShrunkRangeModules();
+    for (unsigned int i = 0;
+         i < process_state->shrunk_range_modules_.size();
+         i++) {
+      linked_ptr<const CodeModule> module =
+          process_state->shrunk_range_modules_[i];
+      BPLOG(INFO) << "The range for module " << module->code_file()
+                  << " was shrunk down by " << HexString(
+                      module->shrink_down_delta()) << " bytes. ";
+    }
+  }
 
   MinidumpMemoryList *memory_list = dump->GetMemoryList();
   if (memory_list) {
diff --git a/src/processor/range_map-inl.h b/src/processor/range_map-inl.h
index 082733e1..9fe74c50 100644
--- a/src/processor/range_map-inl.h
+++ b/src/processor/range_map-inl.h
@@ -53,6 +53,11 @@ void RangeMap<AddressType, EntryType>::SetEnableShrinkDown(
 }
 
 template<typename AddressType, typename EntryType>
+bool RangeMap<AddressType, EntryType>::IsShrinkDownEnabled() const {
+  return enable_shrink_down_;
+}
+
+template<typename AddressType, typename EntryType>
 bool RangeMap<AddressType, EntryType>::StoreRange(const AddressType &base,
                                                   const AddressType &size,
                                                   const EntryType &entry) {
diff --git a/src/processor/range_map.h b/src/processor/range_map.h
index 985d992f..d90a6732 100644
--- a/src/processor/range_map.h
+++ b/src/processor/range_map.h
@@ -60,6 +60,7 @@ class RangeMap {
   // will be shrunk down by moving its start position to a higher address so
   // that it does not overlap anymore.
   void SetEnableShrinkDown(bool enable_shrink_down);
+  bool IsShrinkDownEnabled() const;
 
   // Inserts a range into the map.  Returns false for a parameter error,
   // or if the location of the range would conflict with a range already
diff --git a/src/processor/stackwalker_unittest_utils.h b/src/processor/stackwalker_unittest_utils.h
index 73ceb199..ee22a8fe 100644
--- a/src/processor/stackwalker_unittest_utils.h
+++ b/src/processor/stackwalker_unittest_utils.h
@@ -48,6 +48,7 @@
 #include "google_breakpad/processor/memory_region.h"
 #include "google_breakpad/processor/symbol_supplier.h"
 #include "google_breakpad/processor/system_info.h"
+#include "processor/linked_ptr.h"
 
 class MockMemoryRegion: public google_breakpad::MemoryRegion {
  public:
@@ -114,9 +115,11 @@ class MockCodeModule: public google_breakpad::CodeModule {
   string debug_file()       const { return code_file_; }
   string debug_identifier() const { return code_file_; }
   string version()          const { return version_; }
-  const google_breakpad::CodeModule *Copy() const {
+  google_breakpad::CodeModule *Copy() const {
     abort(); // Tests won't use this.
   }
+  virtual uint64_t shrink_down_delta() const { return 0; }
+  virtual void SetShrinkDownDelta(uint64_t shrink_down_delta) {}
 
  private:
   uint64_t base_address_;
@@ -126,11 +129,11 @@ class MockCodeModule: public google_breakpad::CodeModule {
 };
 
 class MockCodeModules: public google_breakpad::CodeModules {
- public:  
+ public:
   typedef google_breakpad::CodeModule CodeModule;
   typedef google_breakpad::CodeModules CodeModules;
 
-  void Add(const MockCodeModule *module) { 
+  void Add(const MockCodeModule *module) {
     modules_.push_back(module);
   }
 
@@ -157,9 +160,19 @@ class MockCodeModules: public google_breakpad::CodeModules {
     return modules_.at(index);
   }
 
-  const CodeModules *Copy() const { abort(); } // Tests won't use this.
+  CodeModules *Copy() const { abort(); }  // Tests won't use this
+
+  virtual std::vector<google_breakpad::linked_ptr<const CodeModule> >
+  GetShrunkRangeModules() const {
+    return std::vector<google_breakpad::linked_ptr<const CodeModule> >();
+  }
+
+  // Returns true, if module address range shrink is enabled.
+  bool IsModuleShrinkEnabled() const {
+    return false;
+  }
 
- private:  
+ private:
   typedef std::vector<const MockCodeModule *> ModuleVector;
   ModuleVector modules_;
 };