[BitcodeReader] Fix for PR23310: llvm-dis crashes when trying to upgrade an intrinsic.

[philip.pfaffe]

Summary

When trying to upgrade @llvm.x86.sse2.psrl.dq while parsing a module, BitcodeReader adds the function to its worklist twice, resulting in a crash when accessing it the second time.

This patch replaces the worklist vector by a map.

[filcab]

lib/Bitcode/Reader/BitcodeReader.cpp

4464

Please do the range-loop patch as a separate one (before the switch from vector to DenseMap).

4534

Do the range-for change separately.

[philip.pfaffe]

Moved range-loops into seperate patch.

[• rafael]

LGTM

[philip.pfaffe]

Added git diff --binary

[philip.pfaffe]

Commited in rL241281

lib/Bitcode/Reader/BitcodeReader.cpp

class BitcodeReader : public GVMaterializer {
   std::vector<BasicBlock*> FunctionBBs;
 
   // When reading the module header, this list is populated with functions that
   // have bodies later in the file.
   std::vector<Function*> FunctionsWithBodies;
 
   // When intrinsic functions are encountered which require upgrading they are
   // stored here with their replacement function.
-  typedef std::vector<std::pair<Function*, Function*> > UpgradedIntrinsicMap;
+  typedef DenseMap<Function*, Function*> UpgradedIntrinsicMap;
   UpgradedIntrinsicMap UpgradedIntrinsics;
 
   // Map the bitcode's custom MDKind ID to the Module's MDKind ID.
   DenseMap<unsigned, unsigned> MDKindMap;
 
   // Several operations happen after the module header has been read, but
   // before function bodies are processed. This keeps track of whether
   // we've done this yet.
std::error_code BitcodeReader::globalCleanup() {
   resolveGlobalAndAliasInits();
   if (!GlobalInits.empty() || !AliasInits.empty())
     return error("Malformed global initializer set");
 
   // Look for intrinsic functions which need to be upgraded at some point
   for (Function &F : *TheModule) {
     Function *NewFn;
     if (UpgradeIntrinsicFunction(&F, NewFn))
-      UpgradedIntrinsics.push_back(std::make_pair(&F, NewFn));
+      UpgradedIntrinsics[&F] = NewFn;
   }
 
   // Look for global variables which need to be renamed.
   for (GlobalVariable &GV : TheModule->globals())
     UpgradeGlobalVariable(&GV);
 
   // Force deallocation of memory for these vectors to favor the client that
   // want lazy deserialization.
std::error_code BitcodeReader::materialize(GlobalValue *GV) {
   if (StripDebugInfo)
     stripDebugInfo(*F);
 
   // Upgrade any old intrinsic calls in the function.
   for (auto &I : UpgradedIntrinsics) {
     if (I.first != I.second) {
       for (auto *U : I.first->users()) {
         if (CallInst *CI = dyn_cast<CallInst>(U))
           UpgradeIntrinsicCall(CI, I.second);
       }
     }
   }
 
   // Bring in any functions that this function forward-referenced via
   // blockaddresses.
   return materializeForwardReferencedFunctions();
 }
if (!BasicBlockFwdRefs.empty())
     return error("Never resolved function from blockaddress");
 
   // Upgrade any intrinsic calls that slipped through (should not happen!) and
   // delete the old functions to clean up. We can't do this unless the entire
   // module is materialized because there could always be another function body
   // with calls to the old function.
   for (auto &I : UpgradedIntrinsics) {
     if (I.first != I.second) {
       for (auto *U : I.first->users()) {
         if (CallInst *CI = dyn_cast<CallInst>(U))
           UpgradeIntrinsicCall(CI, I.second);
       }
       if (!I.first->use_empty())
         I.first->replaceAllUsesWith(I.second);
       I.first->eraseFromParent();
     }
   }
-  std::vector<std::pair<Function*, Function*> >().swap(UpgradedIntrinsics);
+  UpgradedIntrinsics.clear();
 
   for (unsigned I = 0, E = InstsWithTBAATag.size(); I < E; I++)
     UpgradeInstWithTBAATag(InstsWithTBAATag[I]);
 
   UpgradeDebugInfo(*M);
   return std::error_code();
 }
 

test/Bitcode/PR23310.test

+RUN: llvm-dis -disable-output %p/Inputs/PR23310.bc