~SPIRVModuleImpl() crashes when delete no-id entries (#222)

* ~SPIRVModuleImpl() crashes when delete no-id entries

It crashes because it tries to delete some no-id entry (OpStore) twice, which is
pushed into vector "EntryNoId" twice.

Do not add entries of SPIRVBasicBlock and SPIRVInstruction into Module when call
Decoder.getEntry(). They will be added at addBasicBlock() and addInstruction().

* ~SPIRVModuleImpl() crashes when delete no-id entries

No-id entries of OpGroupDecorate and OpGroupMemberDecorate are pushed into vector
"EntryNoId" twice. See SPIRVGroupDecorateGeneric::decode(std::istream &I).
No-id entry of OpLine will be deleted by std::shared_ptr automatically.

* ~SPIRVModuleImpl() crashes when delete no-id entries

Use type std::set for "EntryNoId" to avoid duplicate elements.The elements
in std:set are sorted by memory pointer, not by creation order. An entry of
OpLine (A) can be deleted because of deletion of another entry (B) which refers
to A. If A is placed after B, it will try to access invalid A when it frees
"EntryNoId". Therefore, not insert entry of OpLine to "EntryNoId", and it will
be deleted by std::shared_ptr automatically.

3 files changed, 15 insertions, 13 deletions

diff --git a/lib/SPIRV/libSPIRV/SPIRVFunction.cpp b/lib/SPIRV/libSPIRV/SPIRVFunction.cpp
index c6e1351..9c5c74b 100644
--- a/lib/SPIRV/libSPIRV/SPIRVFunction.cpp
+++ b/lib/SPIRV/libSPIRV/SPIRVFunction.cpp
@@ -110,6 +110,7 @@ SPIRVFunction::decode(std::istream &I) {
     case OpFunctionParameter: {

       auto Param = static_cast<SPIRVFunctionParameter *>(Decoder.getEntry());

       assert(Param);

+      Module->add(Param);

       Param->setParent(this);

       Parameters.push_back(Param);

       Decoder.getWordCountAndOpCode();

@@ -143,7 +144,7 @@ SPIRVFunction::decodeBB(SPIRVDecoder &Decoder) {
     }

 

     if (Decoder.OpCode == OpLine) {

-      Decoder.getEntry();

+      Module->add(Decoder.getEntry());

       continue;

     }

 

@@ -151,6 +152,8 @@ SPIRVFunction::decodeBB(SPIRVDecoder &Decoder) {
     assert(Inst);

     if (Inst->getOpCode() != OpUndef)

       BB->addInstruction(Inst);

+    else

+      Module->add(Inst);

   }

   Decoder.setScope(this);

 }

diff --git a/lib/SPIRV/libSPIRV/SPIRVModule.cpp b/lib/SPIRV/libSPIRV/SPIRVModule.cpp
index 308c7da..d4cfa71 100644
--- a/lib/SPIRV/libSPIRV/SPIRVModule.cpp
+++ b/lib/SPIRV/libSPIRV/SPIRVModule.cpp
@@ -332,7 +332,7 @@ private:
   SPIRVMemoryModelKind MemoryModel;

 

   typedef std::map<SPIRVId, SPIRVEntry *> SPIRVIdToEntryMap;

-  typedef std::vector<SPIRVEntry *> SPIRVEntryVector;

+  typedef std::set<SPIRVEntry *> SPIRVEntrySet;

   typedef std::set<SPIRVId> SPIRVIdSet;

   typedef std::vector<SPIRVId> SPIRVIdVec;

   typedef std::vector<SPIRVFunction *> SPIRVFunctionVector;

@@ -357,7 +357,7 @@ private:
   SPIRVFunctionVector FuncVec;

   SPIRVConstantVector ConstVec;

   SPIRVVariableVec VariableVec;

-  SPIRVEntryVector EntryNoId;         // Entries without id

+  SPIRVEntrySet EntryNoId;          // Entries without id

   SPIRVIdToBuiltinSetMap IdBuiltinMap;

   SPIRVIdSet NamedId;

   SPIRVStringVec StringVec;

@@ -382,11 +382,8 @@ SPIRVModuleImpl::~SPIRVModuleImpl() {
   //for (auto I:IdEntryMap)

   //  delete I.second;

 

-  // ToDo: Fix bug causing crash

-  //for (auto I:EntryNoId) {

-  //  bildbgs() << "[delete] " << *I;

-  //  delete I;

-  //}

+  for (auto I : EntryNoId)

+    delete I;

 

   for (auto C : CapMap)

     delete C.second;

@@ -552,7 +549,9 @@ SPIRVModuleImpl::addEntry(SPIRVEntry *Entry) {
     } else

       IdEntryMap[Id] = Entry;

   } else {

-    EntryNoId.push_back(Entry);

+    // Entry of OpLine will be deleted by std::shared_ptr automatically.

+    if (Entry->getOpCode() != OpLine)

+      EntryNoId.insert(Entry);

   }

 

   Entry->setModule(this);

@@ -1507,7 +1506,7 @@ operator>> (std::istream &I, SPIRVModule &M) {
   assert(MI.InstSchema == SPIRVISCH_Default && "Unsupported instruction schema");

 

   while(Decoder.getWordCountAndOpCode())

-    Decoder.getEntry();

+    M.add(Decoder.getEntry());

 

   MI.optimizeDecorates();

   MI.resolveUnknownStructFields();

diff --git a/lib/SPIRV/libSPIRV/SPIRVStream.cpp b/lib/SPIRV/libSPIRV/SPIRVStream.cpp
index 6cb2f3d..c78c2c1 100644
--- a/lib/SPIRV/libSPIRV/SPIRVStream.cpp
+++ b/lib/SPIRV/libSPIRV/SPIRVStream.cpp
@@ -241,12 +241,12 @@ SPIRVDecoder::getEntry() {
   else

     Entry->setScope(Scope);

   Entry->setWordCount(WordCount);

-  Entry->setLine(M.getCurrentLine());

+  if (OpCode != OpLine)

+    Entry->setLine(M.getCurrentLine());

   IS >> *Entry;

-  if(Entry->isEndOfBlock() || OpCode == OpNoLine)

+  if (Entry->isEndOfBlock() || OpCode == OpNoLine)

     M.setCurrentLine(nullptr);

   assert(!IS.bad() && !IS.fail() && "SPIRV stream fails");

-  M.add(Entry);

   return Entry;

 }