CodeSource destructor cleanup (#883)

* removeRegion should take a pointer This makes the interface types consistent. * Move destruction of CodeSource::_regions to ~CodeSource * Put 'removeRegion' in CodeSource * Code cleanup * Whitespace cleanup * Move dtor implementation to source file
dyninst · Oct 7, 2020 · 5829baa · 5829baa
1 parent f0b23c2
commit 5829baa
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 39 deletions.
diff --git a/parseAPI/h/CodeSource.h b/parseAPI/h/CodeSource.h
@@ -205,13 +205,14 @@ class PARSER_EXPORT CodeSource : public Dyninst::InstructionSource {
     virtual void startTimer(const std::string& /*name*/) const { return; } 
     virtual void stopTimer(const std::string& /*name*/) const { return; }
     virtual bool findCatchBlockByTryRange(Address /*given try address*/, std::set<Address> & /* catch start */)  const { return false; }
-
+
+    virtual ~CodeSource();
  protected:
     CodeSource() : _regions_overlap(false),
                    _table_of_contents(0) {}
-    virtual ~CodeSource() {}
 
     void addRegion(CodeRegion *);
+    void removeRegion(CodeRegion *);
 
  private: 
     // statistics
@@ -334,7 +335,7 @@ class PARSER_EXPORT SymtabCodeSource : public CodeSource, public boost::lockable
     void init_try_blocks();
 
     CodeRegion * lookup_region(const Address addr) const;
-    void removeRegion(CodeRegion &); // removes from region tree
+    void removeRegion(CodeRegion *); // removes from region tree
 
     void overlapping_warn(const char * file, unsigned line) const;
 

diff --git a/parseAPI/h/SymLiteCodeSource.h b/parseAPI/h/SymLiteCodeSource.h
@@ -129,7 +129,7 @@ class SymReaderCodeSource : public CodeSource {
  private:
 
     CodeRegion * lookup_region(const Address addr) const;
-    void removeRegion(CodeRegion &); // removes from region tree
+    void removeRegion(CodeRegion *); // removes from region tree
 
     void overlapping_warn(const char * file, unsigned line) const;
 

diff --git a/parseAPI/src/CodeSource.C b/parseAPI/src/CodeSource.C
@@ -43,6 +43,11 @@ using namespace std;
 using namespace Dyninst;
 using namespace Dyninst::ParseAPI;
 
+CodeSource::~CodeSource() {
+  for(auto *r : _regions)
+    delete r;
+}
+
 /** CodeSource **/
 void
 CodeSource::addRegion(CodeRegion * cr)
@@ -54,19 +59,26 @@ CodeSource::addRegion(CodeRegion * cr)
         set<CodeRegion *> exist;
         _region_tree.find(cr,exist);
         if(!exist.empty()) {
-	    // for(auto i = exist.begin();
-	    // 	i != exist.end();
-	    // 	++i)
-	    // {
-	    // 	std::cerr << "Region " << **i << " overlaps " << *cr << std::endl;
-	    // }
             _regions_overlap = true;
-	}
+        }
     }
 
     _region_tree.insert(cr);
 }
 
+void CodeSource::removeRegion(CodeRegion *cr) {
+  auto pos = std::remove(_regions.begin(), _regions.end(), cr);
+  if (pos != _regions.end()) {
+    // NB: Assume no duplicates
+    delete *pos;
+    _regions.erase(pos);
+
+    // Also remove from the tree
+    _region_tree.remove(*pos);
+  }
+}
+
+
 int
 CodeSource::findRegions(Address addr, set<CodeRegion *> & ret) const
 {

diff --git a/parseAPI/src/SymLiteCodeSource.C b/parseAPI/src/SymLiteCodeSource.C
@@ -194,8 +194,6 @@ SymReaderCodeSource::~SymReaderCodeSource()
     free(stats_parse);
     if(owns_symtab && _symtab)
       getSymReaderFactory()->closeSymbolReader(_symtab);
-    for(unsigned i=0;i<_regions.size();++i)
-        delete _regions[i];
 }
 
 SymReaderCodeSource::SymReaderCodeSource(SymReader * st) : 
@@ -500,18 +498,9 @@ SymReaderCodeSource::length() const
 
 
 void 
-SymReaderCodeSource::removeRegion(CodeRegion &cr)
+SymReaderCodeSource::removeRegion(CodeRegion *cr)
 {
-    _region_tree.remove( &cr );
-
-    for (vector<CodeRegion*>::iterator rit = _regions.begin(); 
-         rit != _regions.end(); rit++) 
-    {
-        if ( &cr == *rit ) {
-            _regions.erase( rit );
-            break;
-        }
-    }
+	CodeSource::removeRegion(cr);
 }
 
 // fails and returns false if it can't find a CodeRegion
@@ -538,7 +527,7 @@ SymReaderCodeSource::resizeRegion(SymSegment *sr, Address newDiskSize)
     }
 
     // remove, resize, reinsert
-    removeRegion( **rit );
+    removeRegion( *rit );
     sr->file_size = newDiskSize;
     addRegion( *rit );
     return true;

diff --git a/parseAPI/src/SymtabCodeSource.C b/parseAPI/src/SymtabCodeSource.C
@@ -235,8 +235,6 @@ SymtabCodeSource::~SymtabCodeSource()
     delete stats_parse;
     if(owns_symtab && _symtab)
         SymtabAPI::Symtab::closeSymtab(_symtab);
-    for(unsigned i=0;i<_regions.size();++i)
-        delete _regions[i];
 }
 
 SymtabCodeSource::SymtabCodeSource(SymtabAPI::Symtab * st, 
@@ -847,18 +845,9 @@ SymtabCodeSource::length() const
 
 
 void 
-SymtabCodeSource::removeRegion(CodeRegion &cr)
+SymtabCodeSource::removeRegion(CodeRegion *cr)
 {
-    _region_tree.remove( &cr );
-
-    for (vector<CodeRegion*>::iterator rit = _regions.begin(); 
-         rit != _regions.end(); rit++) 
-    {
-        if ( &cr == *rit ) {
-            _regions.erase( rit );
-            break;
-        }
-    }
+	CodeSource::removeRegion(cr);
 }
 
 // fails and returns false if it can't find a CodeRegion
@@ -885,7 +874,7 @@ SymtabCodeSource::resizeRegion(SymtabAPI::Region *sr, Address newDiskSize)
     }
 
     // remove, resize, reinsert
-    removeRegion( **rit );
+    removeRegion( *rit );
     sr->setDiskSize( newDiskSize );
     addRegion( *rit );
     return true;