BitcodeReader: Only create one basic block for each blockaddress

Block address forward-references are implemented by creating a
`BasicBlock` ahead of time that gets inserted in the `Function` when
it's eventually encountered.

However, if the same blockaddress was used in two separate functions
that were parsed *before* the referenced function (and the blockaddress
was never used at global scope), two separate basic blocks would get
created, one of which would be forgotten creating invalid IR.

This commit changes the forward-reference logic to create only one basic
block (and always return the same blockaddress).

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@215805 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
index 66426c83c6693e68aa2307ce332e740933367da7..e5dfa723b04df5c610c4a74767ed15bbe7774774 100644 (file)
--- a/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1635,11 +1635,14 @@ std::error_code BitcodeReader::ParseConstants() {
       } else {
         // Otherwise insert a placeholder and remember it so it can be inserted
         // when the function is parsed.
-        BB = BasicBlock::Create(Context);
         auto &FwdBBs = BasicBlockFwdRefs[Fn];
         if (FwdBBs.empty())
           BasicBlockFwdRefQueue.push_back(Fn);
-        FwdBBs.emplace_back(BBID, BB);
+        if (FwdBBs.size() < BBID + 1)
+          FwdBBs.resize(BBID + 1);
+        if (!FwdBBs[BBID])
+          FwdBBs[BBID] = BasicBlock::Create(Context);
+        BB = FwdBBs[BBID];
       }
       V = BlockAddress::get(Fn, BB);
       break;
@@ -2392,24 +2395,19 @@ std::error_code BitcodeReader::ParseFunctionBody(Function *F) {
           FunctionBBs[i] = BasicBlock::Create(Context, "", F);
       } else {
         auto &BBRefs = BBFRI->second;
-        std::sort(BBRefs.begin(), BBRefs.end(),
-                  [](const std::pair<unsigned, BasicBlock *> &LHS,
-                     const std::pair<unsigned, BasicBlock *> &RHS) {
-          return LHS.first < RHS.first;
-        });
-        unsigned R = 0, RE = BBRefs.size();
-        for (unsigned I = 0, E = FunctionBBs.size(); I != E; ++I)
-          if (R != RE && BBRefs[R].first == I) {
-            assert(I != 0 && "Invalid reference to entry block");
-            BasicBlock *BB = BBRefs[R++].second;
-            BB->insertInto(F);
-            FunctionBBs[I] = BB;
+        // Check for invalid basic block references.
+        if (BBRefs.size() > FunctionBBs.size())
+          return Error(BitcodeError::InvalidID);
+        assert(!BBRefs.empty() && "Unexpected empty array");
+        assert(!BBRefs.front() && "Invalid reference to entry block");
+        for (unsigned I = 0, E = FunctionBBs.size(), RE = BBRefs.size(); I != E;
+             ++I)
+          if (I < RE && BBRefs[I]) {
+            BBRefs[I]->insertInto(F);
+            FunctionBBs[I] = BBRefs[I];
           } else {
             FunctionBBs[I] = BasicBlock::Create(Context, "", F);
           }
-        // Check for invalid basic block references.
-        if (R != RE)
-          return Error(BitcodeError::InvalidID);
 
         // Erase from the table.
         BasicBlockFwdRefs.erase(BBFRI);

diff --git a/lib/Bitcode/Reader/BitcodeReader.h b/lib/Bitcode/Reader/BitcodeReader.h
index 5bbcaf4c614ac4398e7d5622a73c4a3f2ca7d48d..6d4e0a2dfe90bf47985a104f7e065b7217caa2f3 100644 (file)
--- a/lib/Bitcode/Reader/BitcodeReader.h
+++ b/lib/Bitcode/Reader/BitcodeReader.h
@@ -181,9 +181,9 @@ class BitcodeReader : public GVMaterializer {
   DenseMap<Function*, uint64_t> DeferredFunctionInfo;
 
   /// These are basic blocks forward-referenced by block addresses.  They are
-  /// inserted lazily into functions when they're loaded.
-  typedef std::pair<unsigned, BasicBlock *> BasicBlockRefTy;
-  DenseMap<Function *, std::vector<BasicBlockRefTy>> BasicBlockFwdRefs;
+  /// inserted lazily into functions when they're loaded.  The basic block ID is
+  /// its index into the vector.
+  DenseMap<Function *, std::vector<BasicBlock *>> BasicBlockFwdRefs;
   std::deque<Function *> BasicBlockFwdRefQueue;
 
   /// UseRelativeIDs - Indicates that we are using a new encoding for

diff --git a/test/Bitcode/blockaddress.ll b/test/Bitcode/blockaddress.ll
index 305118c83b8e6a6dda5d2e4b69c7ab6d1d16ca97..83fae48bf2fc51ba3d9df901e1a03558f98bd0c3 100644 (file)
--- a/test/Bitcode/blockaddress.ll
+++ b/test/Bitcode/blockaddress.ll
@@ -44,3 +44,17 @@ here:
 end:
   ret void
 }
+
+; Check a blockaddress taken in two separate functions before the referenced
+; function.
+define i8* @take1() {
+  ret i8* blockaddress(@taken, %bb)
+}
+define i8* @take2() {
+  ret i8* blockaddress(@taken, %bb)
+}
+define void @taken() {
+  unreachable
+bb:
+  unreachable
+}