From 7d83ebcadd725d050cc58962e9b7c4312d676e7f Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Sat, 31 Oct 2009 20:08:37 +0000 Subject: [PATCH] Make blockaddress(@func, null) be valid, and make 'deleting a basic block with a blockaddress still referring to it' replace the invalid blockaddress with a new blockaddress(@func, null) instead of a inttoptr(1). This changes the bitcode encoding format, and still needs codegen support (this should produce a non-zero value, referring to the entry block of the function would also be quite reasonable). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@85678 91177308-0d34-0410-b5e6-96231b3b80d8 --- docs/LangRef.html | 6 ++++-- include/llvm/Constants.h | 4 ++++ lib/AsmParser/LLParser.cpp | 16 +++++++++++----- lib/Bitcode/Reader/BitcodeReader.cpp | 5 +++-- lib/Bitcode/Writer/BitcodeWriter.cpp | 3 ++- lib/Bitcode/Writer/ValueEnumerator.cpp | 8 +++++--- lib/VMCore/AsmWriter.cpp | 5 ++++- lib/VMCore/BasicBlock.cpp | 8 +++----- lib/VMCore/Constants.cpp | 5 +++-- 9 files changed, 39 insertions(+), 21 deletions(-) diff --git a/docs/LangRef.html b/docs/LangRef.html index af0e5640c5b..23c07c70761 100644 --- a/docs/LangRef.html +++ b/docs/LangRef.html @@ -2176,11 +2176,13 @@ has undefined behavior.

Blocks
-

blockaddress(@function, %block)

+

blockaddress(@function, %block)
+ blockaddress(@function, null)

The 'blockaddress' constant computes the address of the specified basic block in the specified function, and always has an i8* type. Taking - the address of the entry block is illegal.

+ the address of the entry block is illegal. The BasicBlock operand may also + be null.

This value only has defined behavior when used as an operand to the 'indirectbr' instruction or for comparisons diff --git a/include/llvm/Constants.h b/include/llvm/Constants.h index 99928d9b855..f493ee68ffe 100644 --- a/include/llvm/Constants.h +++ b/include/llvm/Constants.h @@ -567,6 +567,10 @@ public: DECLARE_TRANSPARENT_OPERAND_ACCESSORS(Value); Function *getFunction() const { return (Function*)Op<0>().get(); } + + /// getBasicBlock - This returns the block associated with this BlockAddress. + /// Note that this can return null if the block this originally referred to + /// was deleted. BasicBlock *getBasicBlock() const { return (BasicBlock*)Op<1>().get(); } /// isNullValue - Return true if this is the value that would be returned by diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp index 07bf261573c..9dbd78c558e 100644 --- a/lib/AsmParser/LLParser.cpp +++ b/lib/AsmParser/LLParser.cpp @@ -123,20 +123,25 @@ bool LLParser::ResolveForwardRefBlockAddresses(Function *TheFn, // Loop over all the references, resolving them. for (unsigned i = 0, e = Refs.size(); i != e; ++i) { BasicBlock *Res; - if (PFS) { + if (Refs[i].first.Kind == ValID::t_Null) + Res = 0; + else if (PFS) { if (Refs[i].first.Kind == ValID::t_LocalName) Res = PFS->GetBB(Refs[i].first.StrVal, Refs[i].first.Loc); - else + else { + assert(Refs[i].first.Kind == ValID::t_LocalID); Res = PFS->GetBB(Refs[i].first.UIntVal, Refs[i].first.Loc); + } } else if (Refs[i].first.Kind == ValID::t_LocalID) { return Error(Refs[i].first.Loc, "cannot take address of numeric label after it the function is defined"); } else { + assert(Refs[i].first.Kind == ValID::t_LocalName); Res = dyn_cast_or_null( TheFn->getValueSymbolTable().lookup(Refs[i].first.StrVal)); } - if (Res == 0) + if (Res == 0 && Refs[i].first.Kind != ValID::t_Null) return Error(Refs[i].first.Loc, "referenced value is not a basic block"); @@ -2055,10 +2060,11 @@ bool LLParser::ParseValID(ValID &ID) { ParseValID(Label) || ParseToken(lltok::rparen, "expected ')' in block address expression")) return true; - + if (Fn.Kind != ValID::t_GlobalID && Fn.Kind != ValID::t_GlobalName) return Error(Fn.Loc, "expected function name in blockaddress"); - if (Label.Kind != ValID::t_LocalID && Label.Kind != ValID::t_LocalName) + if (Label.Kind != ValID::t_LocalID && Label.Kind != ValID::t_LocalName && + Label.Kind != ValID::t_Null) return Error(Label.Loc, "expected basic block name in blockaddress"); // Make a global variable as a placeholder for this reference. diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index 68527e3d474..749f16f7f3b 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -2274,11 +2274,12 @@ bool BitcodeReader::ParseFunctionBody(Function *F) { std::vector &RefList = BAFRI->second; for (unsigned i = 0, e = RefList.size(); i != e; ++i) { unsigned BlockIdx = RefList[i].first; - if (BlockIdx >= FunctionBBs.size()) + if (BlockIdx > FunctionBBs.size()) return Error("Invalid blockaddress block #"); GlobalVariable *FwdRef = RefList[i].second; - FwdRef->replaceAllUsesWith(BlockAddress::get(F, FunctionBBs[BlockIdx])); + BasicBlock *BB = BlockIdx == 0 ? 0 : FunctionBBs[BlockIdx-1]; + FwdRef->replaceAllUsesWith(BlockAddress::get(F, BB)); FwdRef->eraseFromParent(); } diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp index af0b8acd44c..e32977db414 100644 --- a/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -854,7 +854,8 @@ static void WriteConstants(unsigned FirstVal, unsigned LastVal, break; } } else if (const BlockAddress *BA = dyn_cast(C)) { - assert(BA->getFunction() == BA->getBasicBlock()->getParent() && + assert((!BA->getBasicBlock() || + BA->getFunction() == BA->getBasicBlock()->getParent()) && "Malformed blockaddress"); Code = bitc::CST_CODE_BLOCKADDRESS; Record.push_back(VE.getTypeID(BA->getFunction()->getType())); diff --git a/lib/Bitcode/Writer/ValueEnumerator.cpp b/lib/Bitcode/Writer/ValueEnumerator.cpp index d840d4ae9fe..92a9bab4e76 100644 --- a/lib/Bitcode/Writer/ValueEnumerator.cpp +++ b/lib/Bitcode/Writer/ValueEnumerator.cpp @@ -265,6 +265,8 @@ void ValueEnumerator::EnumerateValue(const Value *V) { // Do not enumerate the initializers for an array of simple characters. // The initializers just polute the value table, and we emit the strings // specially. + } else if (isa(C)) { + // Don't enumerate function or block. } else if (C->getNumOperands()) { // If a constant has operands, enumerate them. This makes sure that if a // constant has uses (for example an array of const ints), that they are @@ -276,8 +278,7 @@ void ValueEnumerator::EnumerateValue(const Value *V) { // graph that don't go through a global variable. for (User::const_op_iterator I = C->op_begin(), E = C->op_end(); I != E; ++I) - if (!isa(*I)) // Don't enumerate BB operand to BlockAddress. - EnumerateValue(*I); + EnumerateValue(*I); // Finally, add the value. Doing this could make the ValueID reference be // dangling, don't reuse it. @@ -417,9 +418,10 @@ static void IncorporateFunctionInfoGlobalBBIDs(const Function *F, /// specified basic block. This is relatively expensive information, so it /// should only be used by rare constructs such as address-of-label. unsigned ValueEnumerator::getGlobalBasicBlockID(const BasicBlock *BB) const { + if (BB == 0) return 0; unsigned &Idx = GlobalBasicBlockIDs[BB]; if (Idx != 0) - return Idx-1; + return Idx; IncorporateFunctionInfoGlobalBBIDs(BB->getParent(), GlobalBasicBlockIDs); return getGlobalBasicBlockID(BB); diff --git a/lib/VMCore/AsmWriter.cpp b/lib/VMCore/AsmWriter.cpp index 9a803a16628..df1d19b0fcf 100644 --- a/lib/VMCore/AsmWriter.cpp +++ b/lib/VMCore/AsmWriter.cpp @@ -1065,7 +1065,10 @@ static void WriteConstantInt(raw_ostream &Out, const Constant *CV, Out << "blockaddress("; WriteAsOperandInternal(Out, BA->getFunction(), &TypePrinter, Machine); Out << ", "; - WriteAsOperandInternal(Out, BA->getBasicBlock(), &TypePrinter, Machine); + if (BA->getBasicBlock()) + WriteAsOperandInternal(Out, BA->getBasicBlock(), &TypePrinter, Machine); + else + Out << "null"; Out << ")"; return; } diff --git a/lib/VMCore/BasicBlock.cpp b/lib/VMCore/BasicBlock.cpp index 23d0557dc74..c609ef85ebd 100644 --- a/lib/VMCore/BasicBlock.cpp +++ b/lib/VMCore/BasicBlock.cpp @@ -63,15 +63,13 @@ BasicBlock::~BasicBlock() { // hanging off the block, or an undefined use of the block (source code // expecting the address of a label to keep the block alive even though there // is no indirect branch). Handle these cases by zapping the BlockAddress - // nodes. There are no other possible uses at this point. + // nodes, replacing them with BlockAddress(F, NULL). There are no other + // possible uses at this point. if (hasAddressTaken()) { assert(!use_empty() && "There should be at least one blockaddress!"); - Constant *Replacement = - ConstantInt::get(llvm::Type::getInt32Ty(getContext()), 1); while (!use_empty()) { BlockAddress *BA = cast(use_back()); - BA->replaceAllUsesWith(ConstantExpr::getIntToPtr(Replacement, - BA->getType())); + BA->replaceAllUsesWith(BlockAddress::get(BA->getFunction(), 0)); BA->destroyConstant(); } } diff --git a/lib/VMCore/Constants.cpp b/lib/VMCore/Constants.cpp index 2d3d71b6863..e0adf9d2fd5 100644 --- a/lib/VMCore/Constants.cpp +++ b/lib/VMCore/Constants.cpp @@ -1045,7 +1045,7 @@ BlockAddress::BlockAddress(Function *F, BasicBlock *BB) &Op<0>(), 2) { Op<0>() = F; Op<1>() = BB; - BB->AdjustBlockAddressRefCount(1); + if (BB) BB->AdjustBlockAddressRefCount(1); } @@ -1054,7 +1054,8 @@ BlockAddress::BlockAddress(Function *F, BasicBlock *BB) void BlockAddress::destroyConstant() { getFunction()->getType()->getContext().pImpl ->BlockAddresses.erase(std::make_pair(getFunction(), getBasicBlock())); - getBasicBlock()->AdjustBlockAddressRefCount(-1); + if (BasicBlock *BB = getBasicBlock()) + BB->AdjustBlockAddressRefCount(-1); destroyConstantImpl(); } -- 2.34.1