This fixes PR531, a crash when running the CBE on a bytecode file.

The problem is that Function::renameLocalSymbols is iterating through
the symbol table planes, occasionally calling setName to rename a value
(which used to do a symbol table remove/insert pair).

The problem is that if there is only a single value in a particular type
plane that the remove will nuke the symbol table plane, and the insert
will create and insert a new one.  This hoses Function::renameLocalSymbols
because it has an iterator to the old plane, under the (very reasonable)
assumption that simply renaming a value won't cause the type plane to
disappear.

This patch fixes the bug by making the rename operation a single atomic
operation, which has a side effect of making the whole thing faster too. :)


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@20469 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Chris Lattner 2005-03-06 02:14:28 +00:00
parent eaadf58276
commit 04cb800c32
2 changed files with 50 additions and 4 deletions

View File

@ -102,6 +102,42 @@ void SymbolTable::remove(Value *N) {
removeEntry(PI, PI->second.find(N->getName()));
}
/// changeName - Given a value with a non-empty name, remove its existing entry
/// from the symbol table and insert a new one for Name. This is equivalent to
/// doing "remove(V), V->Name = Name, insert(V)", but is faster, and will not
/// temporarily remove the symbol table plane if V is the last value in the
/// symtab with that name (which could invalidate iterators to that plane).
void SymbolTable::changeName(Value *V, const std::string &name) {
assert(!V->getName().empty() && !name.empty() && V->getName() != name &&
"Illegal use of this method!");
plane_iterator PI = pmap.find(V->getType());
assert(PI != pmap.end() && "Value doesn't have an entry in this table?");
ValueMap &VM = PI->second;
value_iterator VI;
if (!InternallyInconsistent) {
VI = VM.find(V->getName());
assert(VI != VM.end() && "Value does have an entry in this table?");
// Remove the old entry.
VM.erase(VI);
}
// See if we can insert the new name.
VI = VM.lower_bound(name);
// Is there a naming conflict?
if (VI != VM.end() && VI->first == name) {
V->Name = getUniqueName(V->getType(), name);
VM.insert(make_pair(V->Name, V));
} else {
V->Name = name;
VM.insert(VI, make_pair(name, V));
}
}
// removeEntry - Remove a value from the symbol table...
Value *SymbolTable::removeEntry(plane_iterator Plane, value_iterator Entry) {

View File

@ -96,8 +96,8 @@ unsigned Value::getNumUses() const {
void Value::setName(const std::string &name) {
if (Name == name) return; // Name is already set.
// Get the symbol table to update for this object.
SymbolTable *ST = 0;
if (Instruction *I = dyn_cast<Instruction>(this)) {
if (BasicBlock *P = I->getParent())
if (Function *PP = P->getParent())
@ -113,9 +113,19 @@ void Value::setName(const std::string &name) {
return; // no name is setable for this.
}
if (ST && hasName()) ST->remove(this);
Name = name;
if (ST && hasName()) ST->insert(this);
if (!ST) // No symbol table to update? Just do the change.
Name = name;
else if (hasName()) {
if (!name.empty()) { // Replacing name.
ST->changeName(this, name);
} else { // Transitioning from hasName -> noname.
ST->remove(this);
Name.clear();
}
} else { // Transitioning from noname -> hasName.
Name = name;
ST->insert(this);
}
}
// uncheckedReplaceAllUsesWith - This is exactly the same as replaceAllUsesWith,