Bug 1370786 - use UniquePtr for SecMap in LUL; r=froydnj
authorTom Tromey <tom@tromey.com>
Wed, 07 Jun 2017 11:40:24 -0600
changeset 413708 bf13425e15f781c4616e23aee4484dcac66b1d21
parent 413707 1efa46c863a0fcc79dc66757d7e3a22c88475e18
child 413709 68f271f79ce6630fc2c75c0d8f291a20e004b83e
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1370786
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1370786 - use UniquePtr for SecMap in LUL; r=froydnj This avoids a memory leak. MozReview-Commit-ID: LmZdWd6ym56
tools/profiler/lul/LulMain.cpp
--- a/tools/profiler/lul/LulMain.cpp
+++ b/tools/profiler/lul/LulMain.cpp
@@ -13,17 +13,19 @@
 #include <algorithm>  // std::sort
 #include <string>
 
 #include "mozilla/Assertions.h"
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/CheckedInt.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/MemoryChecking.h"
+#include "mozilla/Move.h"
 #include "mozilla/Sprintf.h"
+#include "mozilla/UniquePtr.h"
 
 #include "LulCommonExt.h"
 #include "LulElfExt.h"
 
 #include "LulMainInt.h"
 
 #include "platform-linux-lul.h"  // for gettid()
 
@@ -509,68 +511,59 @@ class SegArray {
 ////////////////////////////////////////////////////////////////
 
 class PriMap {
  public:
   explicit PriMap(void (*aLog)(const char*))
     : mLog(aLog)
   {}
 
-  ~PriMap() {
-    for (std::vector<SecMap*>::iterator iter = mSecMaps.begin();
-         iter != mSecMaps.end();
-         ++iter) {
-      delete *iter;
-    }
-    mSecMaps.clear();
-  }
-
   // RUNS IN NO-MALLOC CONTEXT
   pair<const RuleSet*, const vector<PfxInstr>*>
   Lookup(uintptr_t ia)
   {
     SecMap* sm = FindSecMap(ia);
     return pair<const RuleSet*, const vector<PfxInstr>*>
              (sm ? sm->FindRuleSet(ia) : nullptr,
               sm ? sm->GetPfxInstrs() : nullptr);
   }
 
   // Add a secondary map.  No overlaps allowed w.r.t. existing
   // secondary maps.
-  void AddSecMap(SecMap* aSecMap) {
+  void AddSecMap(mozilla::UniquePtr<SecMap>&& aSecMap) {
     // We can't add an empty SecMap to the PriMap.  But that's OK
     // since we'd never be able to find anything in it anyway.
     if (aSecMap->IsEmpty()) {
       return;
     }
 
     // Iterate through the SecMaps and find the right place for this
     // one.  At the same time, ensure that the in-order
     // non-overlapping invariant is preserved (and, generally, holds).
     // FIXME: this gives a cost that is O(N^2) in the total number of
     // shared objects in the system.  ToDo: better.
     MOZ_ASSERT(aSecMap->mSummaryMinAddr <= aSecMap->mSummaryMaxAddr);
 
     size_t num_secMaps = mSecMaps.size();
     uintptr_t i;
     for (i = 0; i < num_secMaps; ++i) {
-      SecMap* sm_i = mSecMaps[i];
+      mozilla::UniquePtr<SecMap>& sm_i = mSecMaps[i];
       MOZ_ASSERT(sm_i->mSummaryMinAddr <= sm_i->mSummaryMaxAddr);
       if (aSecMap->mSummaryMinAddr < sm_i->mSummaryMaxAddr) {
         // |aSecMap| needs to be inserted immediately before mSecMaps[i].
         break;
       }
     }
     MOZ_ASSERT(i <= num_secMaps);
     if (i == num_secMaps) {
       // It goes at the end.
-      mSecMaps.push_back(aSecMap);
+      mSecMaps.push_back(mozilla::Move(aSecMap));
     } else {
-      std::vector<SecMap*>::iterator iter = mSecMaps.begin() + i;
-      mSecMaps.insert(iter, aSecMap);
+      std::vector<mozilla::UniquePtr<SecMap>>::iterator iter = mSecMaps.begin() + i;
+      mSecMaps.insert(iter, mozilla::Move(aSecMap));
     }
     char buf[100];
     SprintfLiteral(buf, "AddSecMap: now have %d SecMaps\n",
                    (int)mSecMaps.size());
     buf[sizeof(buf)-1] = 0;
     mLog(buf);
   }
 
@@ -581,26 +574,25 @@ class PriMap {
     size_t num_secMaps = mSecMaps.size();
     if (num_secMaps > 0) {
       intptr_t i;
       // Iterate from end to start over the vector, so as to ensure
       // that the special case where |avma_min| and |avma_max| denote
       // the entire address space, can be completed in time proportional
       // to the number of elements in the map.
       for (i = (intptr_t)num_secMaps-1; i >= 0; i--) {
-        SecMap* sm_i = mSecMaps[i];
+        mozilla::UniquePtr<SecMap>& sm_i = mSecMaps[i];
         if (sm_i->mSummaryMaxAddr < avma_min ||
             avma_max < sm_i->mSummaryMinAddr) {
           // There's no overlap.  Move on.
           continue;
         }
         // We need to remove mSecMaps[i] and slide all those above it
         // downwards to cover the hole.
         mSecMaps.erase(mSecMaps.begin() + i);
-        delete sm_i;
       }
     }
   }
 
   // Return the number of currently contained SecMaps.
   size_t CountSecMaps() {
     return mSecMaps.size();
   }
@@ -823,30 +815,30 @@ class PriMap {
     long int hi = (long int)mSecMaps.size() - 1;
     while (true) {
       // current unsearched space is from lo to hi, inclusive.
       if (lo > hi) {
         // not found
         return nullptr;
       }
       long int  mid         = lo + ((hi - lo) / 2);
-      SecMap*   mid_secMap  = mSecMaps[mid];
+      mozilla::UniquePtr<SecMap>& mid_secMap = mSecMaps[mid];
       uintptr_t mid_minAddr = mid_secMap->mSummaryMinAddr;
       uintptr_t mid_maxAddr = mid_secMap->mSummaryMaxAddr;
       if (ia < mid_minAddr) { hi = mid-1; continue; }
       if (ia > mid_maxAddr) { lo = mid+1; continue; }
       MOZ_ASSERT(mid_minAddr <= ia && ia <= mid_maxAddr);
-      return mid_secMap;
+      return mid_secMap.get();
     }
     // NOTREACHED
   }
 
  private:
   // sorted array of per-object ranges, non overlapping, non empty
-  std::vector<SecMap*> mSecMaps;
+  std::vector<mozilla::UniquePtr<SecMap>> mSecMaps;
 
   // a logging sink, for debugging.
   void (*mLog)(const char*);
 };
 
 
 ////////////////////////////////////////////////////////////////
 // LUL                                                        //
@@ -951,41 +943,41 @@ LUL::NotifyAfterMap(uintptr_t aRXavma, s
                  aFileName);
   buf[sizeof(buf)-1] = 0;
   mLog(buf);
 
   // Ignore obviously-stupid notifications.
   if (aSize > 0) {
 
     // Here's a new mapping, for this object.
-    SecMap* smap = new SecMap(mLog);
+    mozilla::UniquePtr<SecMap> smap = mozilla::MakeUnique<SecMap>(mLog);
 
     // Read CFI or EXIDX unwind data into |smap|.
     if (!aMappedImage) {
       (void)lul::ReadSymbolData(
-              string(aFileName), std::vector<string>(), smap,
+              string(aFileName), std::vector<string>(), smap.get(),
               (void*)aRXavma, aSize, mUSU, mLog);
     } else {
       (void)lul::ReadSymbolDataInternal(
               (const uint8_t*)aMappedImage,
-              string(aFileName), std::vector<string>(), smap,
+              string(aFileName), std::vector<string>(), smap.get(),
               (void*)aRXavma, aSize, mUSU, mLog);
     }
 
     mLog("NotifyMap .. preparing entries\n");
 
     smap->PrepareRuleSets(aRXavma, aSize);
 
     SprintfLiteral(buf,
                    "NotifyMap got %lld entries\n", (long long int)smap->Size());
     buf[sizeof(buf)-1] = 0;
     mLog(buf);
 
     // Add it to the primary map (the top level set of mapped objects).
-    mPriMap->AddSecMap(smap);
+    mPriMap->AddSecMap(mozilla::Move(smap));
 
     // Tell the segment array about the mapping, so that the stack
     // scan and __kernel_syscall mechanisms know where valid code is.
     mSegArray->add(aRXavma, aRXavma + aSize - 1, true);
   }
 }