From fdb838f3f8a8b6896bbbd5285555874eb3b748eb Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Sat, 19 Dec 2015 20:03:23 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@256105 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/Module.h | 2 +- include/llvm/IR/Value.h | 77 +++++++++++++++++++++++----- lib/Bitcode/Reader/BitcodeReader.cpp | 5 +- lib/IR/Module.cpp | 6 +-- lib/IR/Value.cpp | 10 ++++ lib/IR/Verifier.cpp | 4 +- tools/llvm-extract/llvm-extract.cpp | 15 ++++-- 7 files changed, 96 insertions(+), 23 deletions(-) diff --git a/include/llvm/IR/Module.h b/include/llvm/IR/Module.h index bf1447e48af..942f68543cb 100644 --- a/include/llvm/IR/Module.h +++ b/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(); diff --git a/include/llvm/IR/Value.h b/include/llvm/IR/Value.h index be9f14a78de..bb7ff278fde 100644 --- a/include/llvm/IR/Value.h +++ b/include/llvm/IR/Value.h @@ -273,38 +273,91 @@ 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_iterator; typedef use_iterator_impl 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 materialized_uses() { + return make_range(materialized_use_begin(), use_end()); + } + iterator_range materialized_uses() const { + return make_range(materialized_use_begin(), use_end()); + } iterator_range uses() { - return make_range(use_begin(), use_end()); + assertModuleIsMaterialized(); + return materialized_uses(); } iterator_range 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_iterator; typedef user_iterator_impl 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 users() { - return make_range(user_begin(), user_end()); + assertModuleIsMaterialized(); + return make_range(materialized_user_begin(), user_end()); } iterator_range users() const { - return make_range(user_begin(), user_end()); + assertModuleIsMaterialized(); + return make_range(materialized_user_begin(), user_end()); } /// \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 d1f102704d6..7872a7b5849 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -3028,7 +3028,7 @@ std::error_code BitcodeReader::parseUseLists() { V = ValueList[ID]; unsigned NumUses = 0; SmallDenseMap Order; - for (const Use &U : V->uses()) { + for (const Use &U : V->materialized_uses()) { if (++NumUses > Record.size()) break; Order[&U] = Record[NumUses - 1]; @@ -5266,7 +5266,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(U)) diff --git a/lib/IR/Module.cpp b/lib/IR/Module.cpp index 0685c1a206d..ac578d6dba0 100644 --- a/lib/IR/Module.cpp +++ b/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 M = std::move(Materializer); + return M->materializeModule(); } std::error_code Module::materializeMetadata() { diff --git a/lib/IR/Value.cpp b/lib/IR/Value.cpp index 925f2058e55..eb9deb6a07e 100644 --- a/lib/IR/Value.cpp +++ b/lib/IR/Value.cpp @@ -314,6 +314,16 @@ void Value::takeName(Value *V) { } #ifndef NDEBUG +void Value::assertModuleIsMaterialized() const { + const GlobalValue *GV = dyn_cast(this); + if (!GV) + return; + const Module *M = GV->getParent(); + if (!M) + return; + assert(M->isMaterialized()); +} + static bool contains(SmallPtrSetImpl &Cache, ConstantExpr *Expr, Constant *C) { if (!Cache.insert(Expr).second) diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp index 1a22d37ba8a..9ed044638f3 100644 --- a/lib/IR/Verifier.cpp +++ b/lib/IR/Verifier.cpp @@ -1831,7 +1831,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); diff --git a/tools/llvm-extract/llvm-extract.cpp b/tools/llvm-extract/llvm-extract.cpp index de4288dd6ad..1da456d33f5 100644 --- a/tools/llvm-extract/llvm-extract.cpp +++ b/tools/llvm-extract/llvm-extract.cpp @@ -242,13 +242,22 @@ int main(int argc, char **argv) { } } + { + std::vector 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 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 -- 2.34.1