Bring back "Assert that we have all use/users in the getters."

This reverts commit r257751, bringing back r256105.

The problem the assert found was fixed in r257915.

Original commit message:

Assert that we have all use/users in the getters.

An error that is pretty easy to make is to use the lazy bitcode reader
and then do something like

if (V.use_empty())

The problem is that uses in unmaterialized functions are not accounted
for.

This patch adds asserts that all uses are known.

llvm-svn: 257920

include/llvm/IR/Module.h

@@ -439,6 +439,7 @@ public:
   void setMaterializer(GVMaterializer *GVM);
   /// Retrieves the GVMaterializer, if any, for this Module.
   GVMaterializer *getMaterializer() const { return Materializer.get(); }
+  bool isMaterialized() const { return !getMaterializer(); }
 
   /// Make sure the GlobalValue is fully read. If the module is corrupt, this
   /// returns true and fills in the optional string with information about the
@@ -446,7 +447,6 @@ public:
   std::error_code materialize(GlobalValue *GV);
 
   /// Make sure all GlobalValues in this Module are fully read and clear the
-  /// Materializer. If the module is corrupt, this DOES NOT clear the old
   /// Materializer.
   std::error_code materializeAll();
 

include/llvm/IR/Value.h

@@ -273,38 +273,97 @@ public:
   //----------------------------------------------------------------------
   // Methods for handling the chain of uses of this Value.
   //
-  bool use_empty() const { return UseList == nullptr; }
+  // Materializing a function can introduce new uses, so these methods come in
+  // two variants:
+  // The methods that start with materialized_ check the uses that are
+  // currently known given which functions are materialized. Be very careful
+  // when using them since you might not get all uses.
+  // The methods that don't start with materialized_ assert that modules is
+  // fully materialized.
+#ifdef NDEBUG
+  void assertModuleIsMaterialized() const {}
+#else
+  void assertModuleIsMaterialized() const;
+#endif
+
+  bool use_empty() const {
+    assertModuleIsMaterialized();
+    return UseList == nullptr;
+  }
 
   typedef use_iterator_impl<Use> use_iterator;
   typedef use_iterator_impl<const Use> const_use_iterator;
-  use_iterator use_begin() { return use_iterator(UseList); }
-  const_use_iterator use_begin() const { return const_use_iterator(UseList); }
+  use_iterator materialized_use_begin() { return use_iterator(UseList); }
+  const_use_iterator materialized_use_begin() const {
+    return const_use_iterator(UseList);
+  }
+  use_iterator use_begin() {
+    assertModuleIsMaterialized();
+    return materialized_use_begin();
+  }
+  const_use_iterator use_begin() const {
+    assertModuleIsMaterialized();
+    return materialized_use_begin();
+  }
   use_iterator use_end() { return use_iterator(); }
   const_use_iterator use_end() const { return const_use_iterator(); }
+  iterator_range<use_iterator> materialized_uses() {
+    return make_range(materialized_use_begin(), use_end());
+  }
+  iterator_range<const_use_iterator> materialized_uses() const {
+    return make_range(materialized_use_begin(), use_end());
+  }
   iterator_range<use_iterator> uses() {
-    return make_range(use_begin(), use_end());
+    assertModuleIsMaterialized();
+    return materialized_uses();
   }
   iterator_range<const_use_iterator> uses() const {
-    return make_range(use_begin(), use_end());
+    assertModuleIsMaterialized();
+    return materialized_uses();
   }
 
-  bool user_empty() const { return UseList == nullptr; }
+  bool user_empty() const {
+    assertModuleIsMaterialized();
+    return UseList == nullptr;
+  }
 
   typedef user_iterator_impl<User> user_iterator;
   typedef user_iterator_impl<const User> const_user_iterator;
-  user_iterator user_begin() { return user_iterator(UseList); }
-  const_user_iterator user_begin() const {
+  user_iterator materialized_user_begin() { return user_iterator(UseList); }
+  const_user_iterator materialized_user_begin() const {
     return const_user_iterator(UseList);
   }
+  user_iterator user_begin() {
+    assertModuleIsMaterialized();
+    return materialized_user_begin();
+  }
+  const_user_iterator user_begin() const {
+    assertModuleIsMaterialized();
+    return materialized_user_begin();
+  }
   user_iterator user_end() { return user_iterator(); }
   const_user_iterator user_end() const { return const_user_iterator(); }
-  User *user_back() { return *user_begin(); }
-  const User *user_back() const { return *user_begin(); }
+  User *user_back() {
+    assertModuleIsMaterialized();
+    return *materialized_user_begin();
+  }
+  const User *user_back() const {
+    assertModuleIsMaterialized();
+    return *materialized_user_begin();
+  }
+  iterator_range<user_iterator> materialized_users() {
+    return make_range(materialized_user_begin(), user_end());
+  }
+  iterator_range<const_user_iterator> materialized_users() const {
+    return make_range(materialized_user_begin(), user_end());
+  }
   iterator_range<user_iterator> users() {
-    return make_range(user_begin(), user_end());
+    assertModuleIsMaterialized();
+    return materialized_users();
   }
   iterator_range<const_user_iterator> users() const {
-    return make_range(user_begin(), user_end());
+    assertModuleIsMaterialized();
+    return materialized_users();
   }
 
   /// \brief Return true if there is exactly one user of this value.

lib/Bitcode/Reader/BitcodeReader.cpp

@@ -3022,7 +3022,7 @@ std::error_code BitcodeReader::parseUseLists() {
         V = ValueList[ID];
       unsigned NumUses = 0;
       SmallDenseMap<const Use *, unsigned, 16> Order;
-      for (const Use &U : V->uses()) {
+      for (const Use &U : V->materialized_uses()) {
         if (++NumUses > Record.size())
           break;
         Order[&U] = Record[NumUses - 1];
@@ -5267,7 +5267,8 @@ std::error_code BitcodeReader::materialize(GlobalValue *GV) {
 
   // Upgrade any old intrinsic calls in the function.
   for (auto &I : UpgradedIntrinsics) {
-    for (auto UI = I.first->user_begin(), UE = I.first->user_end(); UI != UE;) {
+    for (auto UI = I.first->materialized_user_begin(), UE = I.first->user_end();
+         UI != UE;) {
       User *U = *UI;
       ++UI;
       if (CallInst *CI = dyn_cast<CallInst>(U))

lib/IR/Module.cpp

@@ -394,10 +394,8 @@ std::error_code Module::materialize(GlobalValue *GV) {
 std::error_code Module::materializeAll() {
   if (!Materializer)
     return std::error_code();
-  if (std::error_code EC = Materializer->materializeModule())
-    return EC;
-  Materializer.reset();
-  return std::error_code();
+  std::unique_ptr<GVMaterializer> M = std::move(Materializer);
+  return M->materializeModule();
 }
 
 std::error_code Module::materializeMetadata() {

lib/IR/Value.cpp

@@ -314,6 +314,16 @@ void Value::takeName(Value *V) {
 }
 
 #ifndef NDEBUG
+void Value::assertModuleIsMaterialized() const {
+  const GlobalValue *GV = dyn_cast<GlobalValue>(this);
+  if (!GV)
+    return;
+  const Module *M = GV->getParent();
+  if (!M)
+    return;
+  assert(M->isMaterialized());
+}
+
 static bool contains(SmallPtrSetImpl<ConstantExpr *> &Cache, ConstantExpr *Expr,
                      Constant *C) {
   if (!Cache.insert(Expr).second)

lib/IR/Verifier.cpp

@@ -470,7 +470,7 @@ static void forEachUser(const Value *User,
                         llvm::function_ref<bool(const Value *)> Callback) {
   if (!Visited.insert(User).second)
     return;
-  for (const Value *TheNextUser : User->users())
+  for (const Value *TheNextUser : User->materialized_users())
     if (Callback(TheNextUser))
       forEachUser(TheNextUser, Visited, Callback);
 }
@@ -1944,7 +1944,9 @@ void Verifier::visitFunction(const Function &F) {
 
   // If this function is actually an intrinsic, verify that it is only used in
   // direct call/invokes, never having its "address taken".
-  if (F.getIntrinsicID()) {
+  // Only do this if the module is materialized, otherwise we don't have all the
+  // uses.
+  if (F.getIntrinsicID() && F.getParent()->isMaterialized()) {
     const User *U;
     if (F.hasAddressTaken(&U))
       Assert(0, "Invalid user of intrinsic instruction!", U);

tools/llvm-extract/llvm-extract.cpp

@@ -242,13 +242,22 @@ int main(int argc, char **argv) {
     }
   }
 
+  {
+    std::vector<GlobalValue *> Gvs(GVs.begin(), GVs.end());
+    legacy::PassManager Extract;
+    Extract.add(createGVExtractionPass(Gvs, DeleteFn));
+    Extract.run(*M);
+
+    // Now that we have all the GVs we want, mark the module as fully
+    // materialized.
+    // FIXME: should the GVExtractionPass handle this?
+    M->materializeAll();
+  }
+
   // In addition to deleting all other functions, we also want to spiff it
   // up a little bit.  Do this now.
   legacy::PassManager Passes;
 
-  std::vector<GlobalValue*> Gvs(GVs.begin(), GVs.end());
-
-  Passes.add(createGVExtractionPass(Gvs, DeleteFn));
   if (!DeleteFn)
     Passes.add(createGlobalDCEPass());           // Delete unreachable globals
   Passes.add(createStripDeadDebugInfoPass());    // Remove dead debug info