Skip to content

Commit

Permalink
Assert that we have all use/users in the getters.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
espindola committed Dec 19, 2015
1 parent 25b4ccf commit fdb838f
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 23 deletions.
2 changes: 1 addition & 1 deletion include/llvm/IR/Module.h
Expand Up @@ -439,14 +439,14 @@ class Module {
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
/// problem. If successful, this returns false.
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();

Expand Down
77 changes: 65 additions & 12 deletions include/llvm/IR/Value.h
Expand Up @@ -273,38 +273,91 @@ class Value {
//----------------------------------------------------------------------
// 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> users() {
return make_range(user_begin(), user_end());
assertModuleIsMaterialized();
return make_range(materialized_user_begin(), user_end());
}
iterator_range<const_user_iterator> 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.
Expand Down
5 changes: 3 additions & 2 deletions lib/Bitcode/Reader/BitcodeReader.cpp
Expand Up @@ -3028,7 +3028,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];
Expand Down Expand Up @@ -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<CallInst>(U))
Expand Down
6 changes: 2 additions & 4 deletions lib/IR/Module.cpp
Expand Up @@ -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() {
Expand Down
10 changes: 10 additions & 0 deletions lib/IR/Value.cpp
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion lib/IR/Verifier.cpp
Expand Up @@ -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);
Expand Down
15 changes: 12 additions & 3 deletions tools/llvm-extract/llvm-extract.cpp
Expand Up @@ -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
Expand Down

0 comments on commit fdb838f

Please sign in to comment.