[patch][rfc] Asserting that we have all use/users in the getters
Rafael Espíndola via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 15 16:02:34 PST 2015
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.
The attached patch adds asserts that all uses are known. I think it
can be reduced a bit by dropping support for dematerializing, which
seems dead. I will send a patch for that, but I just wanted to ask if
this looks like a good idea.
Cheers,
Rafael
-------------- next part --------------
diff --git a/include/llvm/IR/Value.h b/include/llvm/IR/Value.h
index 7f11ba3..7a855e26 100644
--- a/include/llvm/IR/Value.h
+++ b/include/llvm/IR/Value.h
@@ -274,36 +274,103 @@ public:
//----------------------------------------------------------------------
// Methods for handling the chain of uses of this Value.
//
- bool use_empty() const { 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 use_end() { return use_iterator(); }
- const_use_iterator use_end() const { return const_use_iterator(); }
+
+ bool use_empty_unchecked() const { return UseList == nullptr; }
+ use_iterator use_begin_unchecked() { return use_iterator(UseList); }
+ const_use_iterator use_begin_unchecked() const {
+ return const_use_iterator(UseList);
+ }
+ use_iterator use_end() { return use_iterator(); }
+ const_use_iterator use_end() const { return const_use_iterator(); }
+ iterator_range<use_iterator> uses_unchecked() {
+ return make_range(use_begin_unchecked(), use_end());
+ }
+ iterator_range<const_use_iterator> uses_unchecked() const {
+ return make_range(use_begin_unchecked(), use_end());
+ }
+
+#ifdef NDEBUG
+ void assertModuleIsMaterialized() const {}
+#else
+ void assertModuleIsMaterialized() const;
+#endif
+
+ bool use_empty() const {
+ assertModuleIsMaterialized();
+ return use_empty_unchecked();
+ }
+
+ use_iterator use_begin() {
+ assertModuleIsMaterialized();
+ return use_begin_unchecked();
+ }
+ const_use_iterator use_begin() const {
+ assertModuleIsMaterialized();
+ return use_begin_unchecked();
+ }
iterator_range<use_iterator> uses() {
- return make_range(use_begin(), use_end());
+ assertModuleIsMaterialized();
+ return uses_unchecked();
}
iterator_range<const_use_iterator> uses() const {
- return make_range(use_begin(), use_end());
+ assertModuleIsMaterialized();
+ return uses_unchecked();
}
- bool user_empty() const { 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 { return const_user_iterator(UseList); }
- 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(); }
+
+ bool user_empty() const {
+ assertModuleIsMaterialized();
+ return UseList == nullptr;
+ }
+
+ user_iterator user_begin_unchecked() { return user_iterator(UseList); }
+ const_user_iterator user_begin_unchecked() const {
+ return const_user_iterator(UseList);
+ }
+
+ user_iterator user_begin() {
+ assertModuleIsMaterialized();
+ return user_begin_unchecked();
+ }
+ const_user_iterator user_begin() const {
+ assertModuleIsMaterialized();
+ return user_begin_unchecked();
+ }
+ user_iterator user_end() { return user_iterator(); }
+ const_user_iterator user_end() const { return const_user_iterator(); }
+
+ User *user_back_unchecked() { return *user_begin_unchecked(); }
+ const User *user_back_unchecked() const { return *user_begin_unchecked(); }
+
+ User *user_back() {
+ assertModuleIsMaterialized();
+ return user_back_unchecked();
+ }
+ const User *user_back() const {
+ assertModuleIsMaterialized();
+ return user_back_unchecked();
+ }
+
+ iterator_range<user_iterator> users_unchecked() {
+ return make_range(user_begin_unchecked(), user_end());
+ }
+ iterator_range<const_user_iterator> users_unchecked() const {
+ return make_range(user_begin_unchecked(), user_end());
+ }
+
iterator_range<user_iterator> users() {
- return make_range(user_begin(), user_end());
+ assertModuleIsMaterialized();
+ return users_unchecked();
}
iterator_range<const_user_iterator> users() const {
- return make_range(user_begin(), user_end());
+ assertModuleIsMaterialized();
+ return users_unchecked();
}
/// \brief Return true if there is exactly one user of this value.
diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
index d80e70d..b1f026f 100644
--- a/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -3013,7 +3013,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->uses_unchecked()) {
if (++NumUses > Record.size())
break;
Order[&U] = Record[NumUses - 1];
@@ -5227,7 +5227,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->user_begin_unchecked(), UE = I.first->user_end();
+ UI != UE;) {
User *U = *UI;
++UI;
if (CallInst *CI = dyn_cast<CallInst>(U))
@@ -5303,11 +5304,11 @@ std::error_code BitcodeReader::materializeModule(Module *M) {
// module is materialized because there could always be another function body
// with calls to the old function.
for (auto &I : UpgradedIntrinsics) {
- for (auto *U : I.first->users()) {
+ for (auto *U : I.first->users_unchecked()) {
if (CallInst *CI = dyn_cast<CallInst>(U))
UpgradeIntrinsicCall(CI, I.second);
}
- if (!I.first->use_empty())
+ if (!I.first->use_empty_unchecked())
I.first->replaceAllUsesWith(I.second);
I.first->eraseFromParent();
}
diff --git a/lib/IR/DebugInfo.cpp b/lib/IR/DebugInfo.cpp
index a2443be..6116420 100644
--- a/lib/IR/DebugInfo.cpp
+++ b/lib/IR/DebugInfo.cpp
@@ -330,8 +330,8 @@ bool llvm::StripDebugInfo(Module &M) {
}
if (Function *DbgVal = M.getFunction("llvm.dbg.value")) {
- while (!DbgVal->use_empty()) {
- CallInst *CI = cast<CallInst>(DbgVal->user_back());
+ while (!DbgVal->use_empty_unchecked()) {
+ CallInst *CI = cast<CallInst>(DbgVal->user_back_unchecked());
CI->eraseFromParent();
}
DbgVal->eraseFromParent();
diff --git a/lib/IR/Value.cpp b/lib/IR/Value.cpp
index 925f205..c5782e1 100644
--- a/lib/IR/Value.cpp
+++ b/lib/IR/Value.cpp
@@ -313,6 +313,16 @@ void Value::takeName(Value *V) {
ST->reinsertValue(this);
}
+void Value::assertModuleIsMaterialized() const {
+ const GlobalValue *GV = dyn_cast<GlobalValue>(this);
+ if (!GV)
+ return;
+ const Module *M = GV->getParent();
+ if (!M)
+ return;
+ assert(!M->getMaterializer());
+}
+
#ifndef NDEBUG
static bool contains(SmallPtrSetImpl<ConstantExpr *> &Cache, ConstantExpr *Expr,
Constant *C) {
diff --git a/tools/llvm-extract/llvm-extract.cpp b/tools/llvm-extract/llvm-extract.cpp
index 936496c..808c350 100644
--- a/tools/llvm-extract/llvm-extract.cpp
+++ b/tools/llvm-extract/llvm-extract.cpp
@@ -254,13 +254,23 @@ 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: we have to drop the functions above.
+ M->materializeAllPermanently();
+ }
+
// 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
More information about the llvm-commits
mailing list