Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix a bug: 'getExitBB' of SVFFunction may get incorrect exit block. #1262

Merged
merged 12 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions svf-llvm/lib/LLVMModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ void LLVMModuleSet::initSVFFunction()

void LLVMModuleSet::initSVFBasicBlock(const Function* func)
{
SVFFunction *svfFun = getSVFFunction(func);
for (Function::const_iterator bit = func->begin(), ebit = func->end(); bit != ebit; ++bit)
{
const BasicBlock* bb = &*bit;
Expand All @@ -319,6 +320,17 @@ void LLVMModuleSet::initSVFBasicBlock(const Function* func)
const SVFBasicBlock* svf_pred_bb = getSVFBasicBlock(*pred_it);
svfbb->addPredBasicBlock(svf_pred_bb);
}

// set entry / exit block
if (svfbb->getPredecessors().empty())
{
svfFun->setEntryBlock(svfbb);
}
if (svfbb->getSuccessors().empty())
{
svfFun->setExitBlock(svfbb);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should consider the unreachable blocks introduced by instructions such as exit(0). These blocks are not function exit blocks and cannot be connected to the unique exit block. A valid function exit should end with a return instruction.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my knowledge, each llvm function can only have one return instruction. This means there should be only one function exit block? In that way, no extra unique function exit block is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we do not need an extra unique exit block theoretically.

I would like to improve robustness of SVF: when someone generate a valid bitcode file like above, we should handle it correctly.

And I have a question, why my first pr can't pass the testcases. I guess the following assert leads to fail to pass the testcase (maybe multi exit basic block)

inline void setExitBlock(SVFBasicBlock *bb)
{
    assert(!exitBlock && "function already has a exit Basicblock.");
    exitBlock = bb;
}

Because I have no testcases, so I can only guess the reason...

}

for (BasicBlock::const_iterator iit = bb->begin(), eiit = bb->end(); iit != eiit; ++iit)
{
const Instruction* inst = &*iit;
Expand Down
27 changes: 25 additions & 2 deletions svf/include/SVFIR/SVFValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,15 @@ class SVFFunction : public SVFValue
std::vector<const SVFBasicBlock*> allBBs; /// all BasicBlocks of this function
std::vector<const SVFArgument*> allArgs; /// all formal arguments of this function
std::vector<std::string> annotations; /// annotations of this function
SVFBasicBlock *entryBlock; /// entry BasicBlock of this function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need an entryBlock because it is always the first block in allBBs. This is consistent with LLVM's API called getEntryBlock(), which simply returns the front of its basic blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your review. Yeah, here is redundant.

SVFBasicBlock *exitBlock; /// exit BasicBlock of this function
SVFBasicBlock *uniqueExitBlock; /// multi exit BasicBlocks connect to a single unique Exit Block

private:
/// get dummy exit basic block. if not exits, then create it, else return
/// @param exitBB one of multi exit Basic Blocks
/// @return the dummy exit basic block
SVFBasicBlock* getUniqueExitBlock(SVFBasicBlock* exitBB);

protected:
///@{ attributes to be set only through Module builders e.g., LLVMModule
Expand Down Expand Up @@ -409,15 +418,28 @@ class SVFFunction : public SVFValue
inline const SVFBasicBlock* getEntryBlock() const
{
assert(hasBasicBlock() && "function does not have any Basicblock, external function?");
return allBBs.front();
assert(entryBlock && "have not yet set entry Basicblock");
return entryBlock;
}

inline void setEntryBlock(SVFBasicBlock *bb)
{
assert(!entryBlock && "function already has entry Basicblock.");
entryBlock = bb;
}

inline const SVFBasicBlock* getExitBB() const
{
assert(hasBasicBlock() && "function does not have any Basicblock, external function?");
return allBBs.back();
assert(exitBlock && "have not yet set exit Basicblock");
assert((!uniqueExitBlock || uniqueExitBlock == exitBlock) && "unique exit basic block must be null or just equal to exit block");
return exitBlock;
}

/// set exit basic block, if multi exit basic blocks exist,
/// then create a unique basic block, and set the unique exit basic block
void setExitBlock(SVFBasicBlock *bb);

inline const SVFBasicBlock* front() const
{
return getEntryBlock();
Expand Down Expand Up @@ -520,6 +542,7 @@ class SVFBasicBlock : public SVFValue
friend class SVFIRWriter;
friend class SVFIRReader;
friend class SVFIRBuilder;
friend class SVFFunction;

public:
typedef std::vector<const SVFInstruction*>::const_iterator const_iterator;
Expand Down
30 changes: 29 additions & 1 deletion svf/lib/SVFIR/SVFValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ SVFFunction::SVFFunction(const SVFType* ty, const SVFFunctionType* ft,
SVFLoopAndDomInfo* ld, std::vector<std::string> annos)
: SVFValue(ty, SVFValue::SVFFunc), isDecl(declare), intrinsic(intrinsic),
addrTaken(adt), isUncalled(false), isNotRet(false), varArg(varg),
funcType(ft), loopAndDom(ld), realDefFun(nullptr), annotations(std::move(annos))
funcType(ft), loopAndDom(ld), realDefFun(nullptr), annotations(std::move(annos)),
entryBlock(nullptr), exitBlock(nullptr), uniqueExitBlock(nullptr)
{
}

Expand Down Expand Up @@ -189,6 +190,33 @@ bool SVFFunction::isVarArg() const
return varArg;
}

SVFBasicBlock* SVFFunction::getUniqueExitBlock(SVFBasicBlock* exitBB)
{
assert(exitBlock && "we do not need unique exit block when exitBlock is null");
if (!uniqueExitBlock)
{
uniqueExitBlock = new SVFBasicBlock(exitBlock->getType(), this);
// connect every exit basic block to dummy exit block
uniqueExitBlock->addPredBasicBlock(exitBB);
exitBB->addSuccBasicBlock(uniqueExitBlock);
}
return uniqueExitBlock;
}

void SVFFunction::setExitBlock(SVFBasicBlock *bb)
{
if (exitBlock)
{
/// multi exit basic block
exitBlock = getUniqueExitBlock(bb);
}
else
{
/// single exit basic block
exitBlock = bb;
}
}

SVFBasicBlock::SVFBasicBlock(const SVFType* ty, const SVFFunction* f)
: SVFValue(ty, SVFValue::SVFBB), fun(f)
{
Expand Down
Loading