r368836 - [analyzer][CFG] Don't track the condition of asserts
Kristof Umann via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 14 05:20:08 PDT 2019
Author: szelethus
Date: Wed Aug 14 05:20:08 2019
New Revision: 368836
URL: http://llvm.org/viewvc/llvm-project?rev=368836&view=rev
Log:
[analyzer][CFG] Don't track the condition of asserts
Well, what is says on the tin I guess!
Some more changes:
* Move isInevitablySinking() from BugReporter.cpp to CFGBlock's interface
* Rename and move findBlockForNode() from BugReporter.cpp to
ExplodedNode::getCFGBlock()
Differential Revision: https://reviews.llvm.org/D65287
Modified:
cfe/trunk/include/clang/Analysis/CFG.h
cfe/trunk/lib/Analysis/CFG.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
Modified: cfe/trunk/include/clang/Analysis/CFG.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=368836&r1=368835&r2=368836&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/CFG.h (original)
+++ cfe/trunk/include/clang/Analysis/CFG.h Wed Aug 14 05:20:08 2019
@@ -855,6 +855,10 @@ public:
void setLoopTarget(const Stmt *loopTarget) { LoopTarget = loopTarget; }
void setHasNoReturnElement() { HasNoReturnElement = true; }
+ /// Returns true if the block would eventually end with a sink (a noreturn
+ /// node).
+ bool isInevitablySinking() const;
+
CFGTerminator getTerminator() const { return Terminator; }
Stmt *getTerminatorStmt() { return Terminator.getStmt(); }
Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=368836&r1=368835&r2=368836&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Wed Aug 14 05:20:08 2019
@@ -5652,6 +5652,71 @@ void CFGBlock::printTerminatorJson(raw_o
Out << JsonFormat(TempOut.str(), AddQuotes);
}
+// Returns true if by simply looking at the block, we can be sure that it
+// results in a sink during analysis. This is useful to know when the analysis
+// was interrupted, and we try to figure out if it would sink eventually.
+// There may be many more reasons why a sink would appear during analysis
+// (eg. checkers may generate sinks arbitrarily), but here we only consider
+// sinks that would be obvious by looking at the CFG.
+static bool isImmediateSinkBlock(const CFGBlock *Blk) {
+ if (Blk->hasNoReturnElement())
+ return true;
+
+ // FIXME: Throw-expressions are currently generating sinks during analysis:
+ // they're not supported yet, and also often used for actually terminating
+ // the program. So we should treat them as sinks in this analysis as well,
+ // at least for now, but once we have better support for exceptions,
+ // we'd need to carefully handle the case when the throw is being
+ // immediately caught.
+ if (std::any_of(Blk->begin(), Blk->end(), [](const CFGElement &Elm) {
+ if (Optional<CFGStmt> StmtElm = Elm.getAs<CFGStmt>())
+ if (isa<CXXThrowExpr>(StmtElm->getStmt()))
+ return true;
+ return false;
+ }))
+ return true;
+
+ return false;
+}
+
+bool CFGBlock::isInevitablySinking() const {
+ const CFG &Cfg = *getParent();
+
+ const CFGBlock *StartBlk = this;
+ if (isImmediateSinkBlock(StartBlk))
+ return true;
+
+ llvm::SmallVector<const CFGBlock *, 32> DFSWorkList;
+ llvm::SmallPtrSet<const CFGBlock *, 32> Visited;
+
+ DFSWorkList.push_back(StartBlk);
+ while (!DFSWorkList.empty()) {
+ const CFGBlock *Blk = DFSWorkList.back();
+ DFSWorkList.pop_back();
+ Visited.insert(Blk);
+
+ // If at least one path reaches the CFG exit, it means that control is
+ // returned to the caller. For now, say that we are not sure what
+ // happens next. If necessary, this can be improved to analyze
+ // the parent StackFrameContext's call site in a similar manner.
+ if (Blk == &Cfg.getExit())
+ return false;
+
+ for (const auto &Succ : Blk->succs()) {
+ if (const CFGBlock *SuccBlk = Succ.getReachableBlock()) {
+ if (!isImmediateSinkBlock(SuccBlk) && !Visited.count(SuccBlk)) {
+ // If the block has reachable child blocks that aren't no-return,
+ // add them to the worklist.
+ DFSWorkList.push_back(SuccBlk);
+ }
+ }
+ }
+ }
+
+ // Nothing reached the exit. It can only mean one thing: there's no return.
+ return true;
+}
+
const Expr *CFGBlock::getLastCondition() const {
// If the terminator is a temporary dtor or a virtual base, etc, we can't
// retrieve a meaningful condition, bail out.
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=368836&r1=368835&r2=368836&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Wed Aug 14 05:20:08 2019
@@ -2716,90 +2716,6 @@ struct FRIEC_WLItem {
} // namespace
-static const CFGBlock *findBlockForNode(const ExplodedNode *N) {
- ProgramPoint P = N->getLocation();
- if (auto BEP = P.getAs<BlockEntrance>())
- return BEP->getBlock();
-
- // Find the node's current statement in the CFG.
- if (const Stmt *S = PathDiagnosticLocation::getStmt(N))
- return N->getLocationContext()->getAnalysisDeclContext()
- ->getCFGStmtMap()->getBlock(S);
-
- return nullptr;
-}
-
-// Returns true if by simply looking at the block, we can be sure that it
-// results in a sink during analysis. This is useful to know when the analysis
-// was interrupted, and we try to figure out if it would sink eventually.
-// There may be many more reasons why a sink would appear during analysis
-// (eg. checkers may generate sinks arbitrarily), but here we only consider
-// sinks that would be obvious by looking at the CFG.
-static bool isImmediateSinkBlock(const CFGBlock *Blk) {
- if (Blk->hasNoReturnElement())
- return true;
-
- // FIXME: Throw-expressions are currently generating sinks during analysis:
- // they're not supported yet, and also often used for actually terminating
- // the program. So we should treat them as sinks in this analysis as well,
- // at least for now, but once we have better support for exceptions,
- // we'd need to carefully handle the case when the throw is being
- // immediately caught.
- if (std::any_of(Blk->begin(), Blk->end(), [](const CFGElement &Elm) {
- if (Optional<CFGStmt> StmtElm = Elm.getAs<CFGStmt>())
- if (isa<CXXThrowExpr>(StmtElm->getStmt()))
- return true;
- return false;
- }))
- return true;
-
- return false;
-}
-
-// Returns true if by looking at the CFG surrounding the node's program
-// point, we can be sure that any analysis starting from this point would
-// eventually end with a sink. We scan the child CFG blocks in a depth-first
-// manner and see if all paths eventually end up in an immediate sink block.
-static bool isInevitablySinking(const ExplodedNode *N) {
- const CFG &Cfg = N->getCFG();
-
- const CFGBlock *StartBlk = findBlockForNode(N);
- if (!StartBlk)
- return false;
- if (isImmediateSinkBlock(StartBlk))
- return true;
-
- llvm::SmallVector<const CFGBlock *, 32> DFSWorkList;
- llvm::SmallPtrSet<const CFGBlock *, 32> Visited;
-
- DFSWorkList.push_back(StartBlk);
- while (!DFSWorkList.empty()) {
- const CFGBlock *Blk = DFSWorkList.back();
- DFSWorkList.pop_back();
- Visited.insert(Blk);
-
- // If at least one path reaches the CFG exit, it means that control is
- // returned to the caller. For now, say that we are not sure what
- // happens next. If necessary, this can be improved to analyze
- // the parent StackFrameContext's call site in a similar manner.
- if (Blk == &Cfg.getExit())
- return false;
-
- for (const auto &Succ : Blk->succs()) {
- if (const CFGBlock *SuccBlk = Succ.getReachableBlock()) {
- if (!isImmediateSinkBlock(SuccBlk) && !Visited.count(SuccBlk)) {
- // If the block has reachable child blocks that aren't no-return,
- // add them to the worklist.
- DFSWorkList.push_back(SuccBlk);
- }
- }
- }
- }
-
- // Nothing reached the exit. It can only mean one thing: there's no return.
- return true;
-}
-
static BugReport *
FindReportInEquivalenceClass(BugReportEquivClass& EQ,
SmallVectorImpl<BugReport*> &bugReports) {
@@ -2851,8 +2767,9 @@ FindReportInEquivalenceClass(BugReportEq
// to being post-dominated by a sink. This works better when the analysis
// is incomplete and we have never reached the no-return function call(s)
// that we'd inevitably bump into on this path.
- if (isInevitablySinking(errorNode))
- continue;
+ if (const CFGBlock *ErrorB = errorNode->getCFGBlock())
+ if (ErrorB->isInevitablySinking())
+ continue;
// At this point we know that 'N' is not a sink and it has at least one
// successor. Use a DFS worklist to find a non-sink end-of-path node.
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=368836&r1=368835&r2=368836&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Aug 14 05:20:08 2019
@@ -1710,18 +1710,6 @@ public:
};
} // end of anonymous namespace
-static CFGBlock *GetRelevantBlock(const ExplodedNode *Node) {
- if (auto SP = Node->getLocationAs<StmtPoint>()) {
- const Stmt *S = SP->getStmt();
- assert(S);
-
- return const_cast<CFGBlock *>(Node->getLocationContext()
- ->getAnalysisDeclContext()->getCFGStmtMap()->getBlock(S));
- }
-
- return nullptr;
-}
-
static std::shared_ptr<PathDiagnosticEventPiece>
constructDebugPieceForTrackedCondition(const Expr *Cond,
const ExplodedNode *N,
@@ -1742,24 +1730,60 @@ constructDebugPieceForTrackedCondition(c
(Twine() + "Tracking condition '" + ConditionText + "'").str());
}
+static bool isAssertlikeBlock(const CFGBlock *B, ASTContext &Context) {
+ if (B->succ_size() != 2)
+ return false;
+
+ const CFGBlock *Then = B->succ_begin()->getReachableBlock();
+ const CFGBlock *Else = (B->succ_begin() + 1)->getReachableBlock();
+
+ if (!Then || !Else)
+ return false;
+
+ if (Then->isInevitablySinking() != Else->isInevitablySinking())
+ return true;
+
+ // For the following condition the following CFG would be built:
+ //
+ // ------------->
+ // / \
+ // [B1] -> [B2] -> [B3] -> [sink]
+ // assert(A && B || C); \ \
+ // -----------> [go on with the execution]
+ //
+ // It so happens that CFGBlock::getTerminatorCondition returns 'A' for block
+ // B1, 'A && B' for B2, and 'A && B || C' for B3. Let's check whether we
+ // reached the end of the condition!
+ if (const Stmt *ElseCond = Else->getTerminatorCondition())
+ if (isa<BinaryOperator>(ElseCond)) {
+ assert(cast<BinaryOperator>(ElseCond)->isLogicalOp());
+ return isAssertlikeBlock(Else, Context);
+ }
+
+ return false;
+}
+
PathDiagnosticPieceRef TrackControlDependencyCondBRVisitor::VisitNode(
const ExplodedNode *N, BugReporterContext &BRC, BugReport &BR) {
// We can only reason about control dependencies within the same stack frame.
if (Origin->getStackFrame() != N->getStackFrame())
return nullptr;
- CFGBlock *NB = GetRelevantBlock(N);
+ CFGBlock *NB = const_cast<CFGBlock *>(N->getCFGBlock());
// Skip if we already inspected this block.
if (!VisitedBlocks.insert(NB).second)
return nullptr;
- CFGBlock *OriginB = GetRelevantBlock(Origin);
+ CFGBlock *OriginB = const_cast<CFGBlock *>(Origin->getCFGBlock());
// TODO: Cache CFGBlocks for each ExplodedNode.
if (!OriginB || !NB)
return nullptr;
+ if (isAssertlikeBlock(NB, BRC.getASTContext()))
+ return nullptr;
+
if (ControlDeps.isControlDependent(OriginB, NB)) {
if (const Expr *Condition = NB->getLastCondition()) {
// Keeping track of the already tracked conditions on a visitor level
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp?rev=368836&r1=368835&r2=368836&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExplodedGraph.cpp Wed Aug 14 05:20:08 2019
@@ -16,6 +16,7 @@
#include "clang/AST/ExprObjC.h"
#include "clang/AST/ParentMap.h"
#include "clang/AST/Stmt.h"
+#include "clang/Analysis/CFGStmtMap.h"
#include "clang/Analysis/ProgramPoint.h"
#include "clang/Analysis/Support/BumpVector.h"
#include "clang/Basic/LLVM.h"
@@ -292,6 +293,21 @@ bool ExplodedNode::isTrivial() const {
getFirstPred()->succ_size() == 1;
}
+const CFGBlock *ExplodedNode::getCFGBlock() const {
+ ProgramPoint P = getLocation();
+ if (auto BEP = P.getAs<BlockEntrance>())
+ return BEP->getBlock();
+
+ // Find the node's current statement in the CFG.
+ if (const Stmt *S = PathDiagnosticLocation::getStmt(this))
+ return getLocationContext()
+ ->getAnalysisDeclContext()
+ ->getCFGStmtMap()
+ ->getBlock(S);
+
+ return nullptr;
+}
+
ExplodedNode *ExplodedGraph::getNode(const ProgramPoint &L,
ProgramStateRef State,
bool IsSink,
Modified: cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp?rev=368836&r1=368835&r2=368836&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp (original)
+++ cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp Wed Aug 14 05:20:08 2019
@@ -458,3 +458,242 @@ void f(int flag) {
}
} // end of namespace unimportant_write_before_collapse_point
+
+namespace dont_track_assertlike_conditions {
+
+extern void __assert_fail(__const char *__assertion, __const char *__file,
+ unsigned int __line, __const char *__function)
+ __attribute__((__noreturn__));
+#define assert(expr) \
+ ((expr) ? (void)(0) : __assert_fail(#expr, __FILE__, __LINE__, __func__))
+
+int getInt();
+
+int cond1;
+
+void bar() {
+ cond1 = getInt();
+}
+
+void f(int flag) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ flag = getInt();
+
+ bar();
+ assert(cond1); // expected-note{{Assuming 'cond1' is not equal to 0}}
+ // expected-note at -1{{'?' condition is true}}
+
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note at -1{{Taking true branch}}
+ // debug-note at -2{{Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+
+#undef assert
+} // end of namespace dont_track_assertlike_conditions
+
+namespace dont_track_assertlike_and_conditions {
+
+extern void __assert_fail(__const char *__assertion, __const char *__file,
+ unsigned int __line, __const char *__function)
+ __attribute__((__noreturn__));
+#define assert(expr) \
+ ((expr) ? (void)(0) : __assert_fail(#expr, __FILE__, __LINE__, __func__))
+
+int getInt();
+
+int cond1;
+int cond2;
+
+void bar() {
+ cond1 = getInt();
+ cond2 = getInt();
+}
+
+void f(int flag) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ flag = getInt();
+
+ bar();
+ assert(cond1 && cond2);
+ // expected-note at -1{{Assuming 'cond1' is not equal to 0}}
+ // expected-note at -2{{Assuming 'cond2' is not equal to 0}}
+ // expected-note at -3{{'?' condition is true}}
+ // expected-note at -4{{Left side of '&&' is true}}
+
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note at -1{{Taking true branch}}
+ // debug-note at -2{{Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+
+#undef assert
+} // end of namespace dont_track_assertlike_and_conditions
+
+namespace dont_track_assertlike_or_conditions {
+
+extern void __assert_fail(__const char *__assertion, __const char *__file,
+ unsigned int __line, __const char *__function)
+ __attribute__((__noreturn__));
+#define assert(expr) \
+ ((expr) ? (void)(0) : __assert_fail(#expr, __FILE__, __LINE__, __func__))
+
+int getInt();
+
+int cond1;
+int cond2;
+
+void bar() {
+ cond1 = getInt();
+ cond2 = getInt();
+}
+
+void f(int flag) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ flag = getInt();
+
+ bar();
+ assert(cond1 || cond2);
+ // expected-note at -1{{Assuming 'cond1' is not equal to 0}}
+ // expected-note at -2{{Left side of '||' is true}}
+
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note at -1{{Taking true branch}}
+ // debug-note at -2{{Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+
+#undef assert
+} // end of namespace dont_track_assertlike_or_conditions
+
+namespace dont_track_assert2like_conditions {
+
+extern void __assert_fail(__const char *__assertion, __const char *__file,
+ unsigned int __line, __const char *__function)
+ __attribute__((__noreturn__));
+#define assert(expr) \
+ do { \
+ if (!(expr)) \
+ __assert_fail(#expr, __FILE__, __LINE__, __func__); \
+ } while (0)
+
+int getInt();
+
+int cond1;
+
+void bar() {
+ cond1 = getInt();
+}
+
+void f(int flag) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ flag = getInt();
+
+ bar();
+ assert(cond1); // expected-note{{Assuming 'cond1' is not equal to 0}}
+ // expected-note at -1{{Taking false branch}}
+ // expected-note at -2{{Loop condition is false. Exiting loop}}
+
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note at -1{{Taking true branch}}
+ // debug-note at -2{{Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+
+#undef assert
+} // end of namespace dont_track_assert2like_conditions
+
+namespace dont_track_assert2like_and_conditions {
+
+extern void __assert_fail(__const char *__assertion, __const char *__file,
+ unsigned int __line, __const char *__function)
+ __attribute__((__noreturn__));
+#define assert(expr) \
+ do { \
+ if (!(expr)) \
+ __assert_fail(#expr, __FILE__, __LINE__, __func__); \
+ } while (0)
+
+int getInt();
+
+int cond1;
+int cond2;
+
+void bar() {
+ cond1 = getInt();
+ cond2 = getInt();
+}
+
+void f(int flag) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ flag = getInt();
+
+ bar();
+ assert(cond1 && cond2);
+ // expected-note at -1{{Assuming 'cond1' is not equal to 0}}
+ // expected-note at -2{{Left side of '&&' is true}}
+ // expected-note at -3{{Assuming the condition is false}}
+ // expected-note at -4{{Taking false branch}}
+ // expected-note at -5{{Loop condition is false. Exiting loop}}
+
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note at -1{{Taking true branch}}
+ // debug-note at -2{{Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+
+#undef assert
+} // end of namespace dont_track_assert2like_and_conditions
+
+namespace dont_track_assert2like_or_conditions {
+
+extern void __assert_fail(__const char *__assertion, __const char *__file,
+ unsigned int __line, __const char *__function)
+ __attribute__((__noreturn__));
+#define assert(expr) \
+ do { \
+ if (!(expr)) \
+ __assert_fail(#expr, __FILE__, __LINE__, __func__); \
+ } while (0)
+
+int getInt();
+
+int cond1;
+int cond2;
+
+void bar() {
+ cond1 = getInt();
+ cond2 = getInt();
+}
+
+void f(int flag) {
+ int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
+
+ flag = getInt();
+
+ bar();
+ assert(cond1 || cond2);
+ // expected-note at -1{{Assuming 'cond1' is not equal to 0}}
+ // expected-note at -2{{Left side of '||' is true}}
+ // expected-note at -3{{Taking false branch}}
+ // expected-note at -4{{Loop condition is false. Exiting loop}}
+
+ if (flag) // expected-note{{'flag' is not equal to 0}}
+ // expected-note at -1{{Taking true branch}}
+ // debug-note at -2{{Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+
+#undef assert
+} // end of namespace dont_track_assert2like_or_conditions
More information about the cfe-commits
mailing list