Fixes for memory leaks in SPIR-V library.

In few places in the SPIR-V library a pattern of
“dropAllReferences + removeFromParent” calls was used on values like
functions and instructions, that were not meant to be reused.
These lead to memory leaks, as these functions do not destruct the objects.
The fix is to use “eraseFromParent” instead.
SPIRVDecorate objects were also not deallocated, as they were not added
to owning module’s SPIRVEntry list.
The fix is to add them, as all other similar entries,
e.g. SPIRVDecorationGroup.

9 files changed, 27 insertions, 38 deletions

diff --git a/lib/SPIRV/OCL20ToSPIRV.cpp b/lib/SPIRV/OCL20ToSPIRV.cpp
index 58278f6..32aaed8 100644
--- a/lib/SPIRV/OCL20ToSPIRV.cpp
+++ b/lib/SPIRV/OCL20ToSPIRV.cpp
@@ -1230,14 +1230,12 @@ void OCL20ToSPIRV::transWorkItemBuiltinsToVariables() {
       InstList.push_back(CI);

     }

     for (auto &Inst:InstList) {

-      Inst->dropAllReferences();

-      Inst->removeFromParent();

+      Inst->eraseFromParent();

     }

     WorkList.push_back(I);

   }

   for (auto &I:WorkList) {

-    I->dropAllReferences();

-    I->removeFromParent();

+    I->eraseFromParent();

   }

 }

 

@@ -1386,8 +1384,7 @@ void OCL20ToSPIRV::visitCallDot(CallInst *CI) {
   IRBuilder<> Builder(CI);

   Value *FMulVal = Builder.CreateFMul(CI->getOperand(0), CI->getOperand(1));

   CI->replaceAllUsesWith(FMulVal);

-  CI->dropAllReferences();

-  CI->removeFromParent();

+  CI->eraseFromParent();

 }

 

 void OCL20ToSPIRV::visitCallScalToVec(CallInst *CI, StringRef MangledName,

diff --git a/lib/SPIRV/SPIRVReader.cpp b/lib/SPIRV/SPIRVReader.cpp
index 2e5c49f..4f7deaa 100644
--- a/lib/SPIRV/SPIRVReader.cpp
+++ b/lib/SPIRV/SPIRVReader.cpp
@@ -427,10 +427,8 @@ private:
           "A value is translated twice");

       // Replaces placeholders for PHI nodes

       LD->replaceAllUsesWith(V);

-      LD->dropAllReferences();

-      LD->removeFromParent();

-      Placeholder->dropAllReferences();

-      Placeholder->removeFromParent();

+      LD->eraseFromParent();

+      Placeholder->eraseFromParent();

     }

     ValueMap[BV] = V;

     return V;

@@ -567,8 +565,7 @@ SPIRVToLLVM::transOCLBuiltinsFromVariables(){
     WorkList.push_back(I);

   }

   for (auto &I:WorkList) {

-    I->dropAllReferences();

-    I->removeFromParent();

+    I->eraseFromParent();

   }

   return true;

 }

@@ -636,8 +633,7 @@ SPIRVToLLVM::transOCLBuiltinFromVariable(GlobalVariable *GV,
     I->replaceAllUsesWith(Call);

   }

   for (auto &I:Deletes) {

-    I->dropAllReferences();

-    I->removeFromParent();

+    I->eraseFromParent();

   }

   return true;

 }

@@ -1103,14 +1099,11 @@ SPIRVToLLVM::postProcessOCLBuiltinReturnStruct(Function *F) {
       Args.insert(Args.begin(), ST->getPointerOperand());

       auto NewCI = CallInst::Create(newF, Args, CI->getName(), CI);

       NewCI->setCallingConv(CI->getCallingConv());

-      ST->dropAllReferences();

-      ST->removeFromParent();

-      CI->dropAllReferences();

-      CI->removeFromParent();

+      ST->eraseFromParent();

+      CI->eraseFromParent();

     }

   }

-  F->dropAllReferences();

-  F->removeFromParent();

+  F->eraseFromParent();

   return true;

 }

 

diff --git a/lib/SPIRV/SPIRVUtil.cpp b/lib/SPIRV/SPIRVUtil.cpp
index 5208aa2..0c75e11 100644
--- a/lib/SPIRV/SPIRVUtil.cpp
+++ b/lib/SPIRV/SPIRVUtil.cpp
@@ -655,8 +655,7 @@ mutateCallInst(Module *M, CallInst *CI,
       InstName, TakeFuncName);
   DEBUG(dbgs() << " => " << *NewCI << '\n');
   CI->replaceAllUsesWith(NewCI);
-  CI->dropAllReferences();
-  CI->removeFromParent();
+  CI->eraseFromParent();
   return NewCI;
 }
 
@@ -682,8 +681,7 @@ mutateCallInst(Module *M, CallInst *CI,
   NewI->takeName(CI);
   DEBUG(dbgs() << " => " << *NewI << '\n');
   CI->replaceAllUsesWith(NewI);
-  CI->dropAllReferences();
-  CI->removeFromParent();
+  CI->eraseFromParent();
   return NewI;
 }
 
diff --git a/lib/SPIRV/libSPIRV/SPIRVDecorate.cpp b/lib/SPIRV/libSPIRV/SPIRVDecorate.cpp
index 9790b18..087efbf 100644
--- a/lib/SPIRV/libSPIRV/SPIRVDecorate.cpp
+++ b/lib/SPIRV/libSPIRV/SPIRVDecorate.cpp
@@ -171,7 +171,7 @@ SPIRVGroupDecorate::decorateTargets() {
     auto Target = getOrCreate(I);

     for (auto &Dec:DecorationGroup->getDecorations()) {

       assert(Dec->isDecorate());

-      Target->addDecorate(static_cast<const SPIRVDecorate *const>(Dec));

+      Target->addDecorate(static_cast<SPIRVDecorate *const>(Dec));

     }

   }

 }

@@ -182,7 +182,7 @@ SPIRVGroupMemberDecorate::decorateTargets() {
     auto Target = getOrCreate(I);

     for (auto &Dec:DecorationGroup->getDecorations()) {

       assert(Dec->isMemberDecorate());

-      Target->addMemberDecorate(static_cast<const SPIRVMemberDecorate*>(Dec));

+      Target->addMemberDecorate(static_cast<SPIRVMemberDecorate*>(Dec));

     }

   }

 }

diff --git a/lib/SPIRV/libSPIRV/SPIRVDecorate.h b/lib/SPIRV/libSPIRV/SPIRVDecorate.h
index f4cacf0..1683d45 100644
--- a/lib/SPIRV/libSPIRV/SPIRVDecorate.h
+++ b/lib/SPIRV/libSPIRV/SPIRVDecorate.h
@@ -105,12 +105,12 @@ protected:
   SPIRVDecorationGroup *Owner; // Owning decorate group

 };

 

-class SPIRVDecorateSet: public std::multiset<const SPIRVDecorateGeneric *,

+class SPIRVDecorateSet: public std::multiset<SPIRVDecorateGeneric *,

     SPIRVDecorateGeneric::Comparator> {

     public:

-  typedef std::multiset<const SPIRVDecorateGeneric *,

+  typedef std::multiset<SPIRVDecorateGeneric *,

       SPIRVDecorateGeneric::Comparator> BaseType;

-  iterator insert(const value_type& Dec) {

+  iterator insert(value_type& Dec) {

     auto ER = BaseType::equal_range(Dec);

     for (auto I = ER.first, E = ER.second; I != E; ++I) {

       SPIRVDBG(spvdbgs() << "[compare decorate] " << *Dec

diff --git a/lib/SPIRV/libSPIRV/SPIRVEntry.cpp b/lib/SPIRV/libSPIRV/SPIRVEntry.cpp
index 8e1f70c..3bd8818 100644
--- a/lib/SPIRV/libSPIRV/SPIRVEntry.cpp
+++ b/lib/SPIRV/libSPIRV/SPIRVEntry.cpp
@@ -282,7 +282,7 @@ SPIRVEntry::validateBuiltin(SPIRVWord TheSet, SPIRVWord Index)const {
 }

 

 void

-SPIRVEntry::addDecorate(const SPIRVDecorate *Dec) {

+SPIRVEntry::addDecorate(SPIRVDecorate *Dec) {

   auto Kind = Dec->getDecorateKind();

   Decorates.insert(std::make_pair(Dec->getDecorateKind(), Dec));

   Module->addDecorate(Dec);

@@ -321,7 +321,7 @@ SPIRVEntry::setLine(const std::shared_ptr<const SPIRVLine>& L){
 }

 

 void

-SPIRVEntry::addMemberDecorate(const SPIRVMemberDecorate *Dec){

+SPIRVEntry::addMemberDecorate(SPIRVMemberDecorate *Dec){

   assert(canHaveMemberDecorates() && MemberDecorates.find(Dec->getPair()) ==

       MemberDecorates.end());

   MemberDecorates[Dec->getPair()] = Dec;

diff --git a/lib/SPIRV/libSPIRV/SPIRVEntry.h b/lib/SPIRV/libSPIRV/SPIRVEntry.h
index 8cdb3dd..b994bf1 100644
--- a/lib/SPIRV/libSPIRV/SPIRVEntry.h
+++ b/lib/SPIRV/libSPIRV/SPIRVEntry.h
@@ -273,11 +273,11 @@ public:
     return false;

   }

 

-  void addDecorate(const SPIRVDecorate *);

+  void addDecorate(SPIRVDecorate *);

   void addDecorate(Decoration Kind);

   void addDecorate(Decoration Kind, SPIRVWord Literal);

   void eraseDecorate(Decoration);

-  void addMemberDecorate(const SPIRVMemberDecorate *);

+  void addMemberDecorate(SPIRVMemberDecorate *);

   void addMemberDecorate(SPIRVWord MemberNumber, Decoration Kind);

   void addMemberDecorate(SPIRVWord MemberNumber, Decoration Kind,

       SPIRVWord Literal);

diff --git a/lib/SPIRV/libSPIRV/SPIRVModule.cpp b/lib/SPIRV/libSPIRV/SPIRVModule.cpp
index 1cec3f7..b1887c0 100644
--- a/lib/SPIRV/libSPIRV/SPIRVModule.cpp
+++ b/lib/SPIRV/libSPIRV/SPIRVModule.cpp
@@ -168,7 +168,7 @@ public:
   virtual void setCurrentLine(const std::shared_ptr<const SPIRVLine> &Line);

   virtual void addCapability(SPIRVCapabilityKind);

   virtual void addCapabilityInternal(SPIRVCapabilityKind);

-  virtual const SPIRVDecorateGeneric *addDecorate(const SPIRVDecorateGeneric *);

+  virtual const SPIRVDecorateGeneric *addDecorate(SPIRVDecorateGeneric *);

   virtual SPIRVDecorationGroup *addDecorationGroup();

   virtual SPIRVDecorationGroup *addDecorationGroup(SPIRVDecorationGroup *Group);

   virtual SPIRVGroupDecorate *addGroupDecorate(SPIRVDecorationGroup *Group,

@@ -433,7 +433,7 @@ SPIRVModuleImpl::optimizeDecorates() {
       continue;

     }

     SPIRVDBG(spvdbgs() << "  add deco group. erase equal range\n");

-    auto G = new SPIRVDecorationGroup(this, getId());

+    auto G = add(new SPIRVDecorationGroup(this, getId()));

     std::vector<SPIRVId> Targets;

     Targets.push_back(D->getTargetId());

     const_cast<SPIRVDecorateGeneric*>(D)->setTargetId(G->getId());

@@ -450,7 +450,7 @@ SPIRVModuleImpl::optimizeDecorates() {
     // For now, just skip using a group if the number of targets to too big

     if (Targets.size() < 65530) {

       DecorateSet.erase(ER.first, ER.second);

-      auto GD = new SPIRVGroupDecorate(G, Targets);

+      auto GD = add(new SPIRVGroupDecorate(G, Targets));

       DecGroupVec.push_back(G);

       GroupDecVec.push_back(GD);

     }

@@ -841,7 +841,8 @@ SPIRVModuleImpl::addBasicBlock(SPIRVFunction *Func, SPIRVId Id) {
 }

 

 const SPIRVDecorateGeneric *

-SPIRVModuleImpl::addDecorate(const SPIRVDecorateGeneric *Dec) {

+SPIRVModuleImpl::addDecorate(SPIRVDecorateGeneric *Dec) {

+  add(Dec);

   SPIRVId Id = Dec->getTargetId();

   SPIRVEntry *Target = nullptr;

   bool Found = exist(Id, &Target);

diff --git a/lib/SPIRV/libSPIRV/SPIRVModule.h b/lib/SPIRV/libSPIRV/SPIRVModule.h
index a5e3f28..577893a 100644
--- a/lib/SPIRV/libSPIRV/SPIRVModule.h
+++ b/lib/SPIRV/libSPIRV/SPIRVModule.h
@@ -170,7 +170,7 @@ public:
       SPIRVWord Column) = 0;

   virtual const std::shared_ptr<const SPIRVLine>& getCurrentLine() const = 0;

   virtual void setCurrentLine(const std::shared_ptr<const SPIRVLine>&) = 0;

-  virtual const SPIRVDecorateGeneric *addDecorate(const SPIRVDecorateGeneric*)

+  virtual const SPIRVDecorateGeneric *addDecorate(SPIRVDecorateGeneric*)

     = 0;

   virtual SPIRVDecorationGroup *addDecorationGroup() = 0;

   virtual SPIRVDecorationGroup *addDecorationGroup(SPIRVDecorationGroup *Group)