r349320 - [analyzer] Fix some expressions staying live too long. Add a debug checker.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Sun Dec 16 15:44:06 PST 2018
Author: dergachev
Date: Sun Dec 16 15:44:06 2018
New Revision: 349320
URL: http://llvm.org/viewvc/llvm-project?rev=349320&view=rev
Log:
[analyzer] Fix some expressions staying live too long. Add a debug checker.
StaticAnalyzer uses the CFG-based RelaxedLiveVariables analysis in order to,
in particular, figure out values of which expressions are still needed.
When the expression becomes "dead", it is garbage-collected during
the dead binding scan.
Expressions that constitute branches/bodies of control flow statements,
eg. `E1' in `if (C1) E1;' but not `E2' in `if (C2) { E2; }', were kept alive
for too long. This caused false positives in MoveChecker because it relies
on cleaning up loop-local variables when they go out of scope, but some of those
live-for-too-long expressions were keeping a reference to those variables.
Fix liveness analysis to correctly mark these expressions as dead.
Add a debug checker, debug.DumpLiveStmts, in order to test expressions liveness.
Differential Revision: https://reviews.llvm.org/D55566
Added:
cfe/trunk/test/Analysis/live-stmts.cpp
Modified:
cfe/trunk/docs/analyzer/DebugChecks.rst
cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/Analysis/LiveVariables.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
cfe/trunk/test/Analysis/use-after-move.cpp
Modified: cfe/trunk/docs/analyzer/DebugChecks.rst
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/analyzer/DebugChecks.rst?rev=349320&r1=349319&r2=349320&view=diff
==============================================================================
--- cfe/trunk/docs/analyzer/DebugChecks.rst (original)
+++ cfe/trunk/docs/analyzer/DebugChecks.rst Sun Dec 16 15:44:06 2018
@@ -30,9 +30,12 @@ using a 'dot' format viewer (such as Gra
- debug.DumpLiveVars: Show the results of live variable analysis for each
top-level function being analyzed.
+- debug.DumpLiveStmts: Show the results of live statement analysis for each
+ top-level function being analyzed.
+
- debug.ViewExplodedGraph: Show the Exploded Graphs generated for the
analysis of different functions in the input translation unit. When there
- are several functions analyzed, display one graph per function. Beware
+ are several functions analyzed, display one graph per function. Beware
that these graphs may grow very large, even for small functions.
Path Tracking
Modified: cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h?rev=349320&r1=349319&r2=349320&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/LiveVariables.h Sun Dec 16 15:44:06 2018
@@ -88,9 +88,13 @@ public:
/// before the given block-level expression (see runOnAllBlocks).
bool isLive(const Stmt *Loc, const Stmt *StmtVal);
- /// Print to stderr the liveness information associated with
+ /// Print to stderr the variable liveness information associated with
/// each basic block.
- void dumpBlockLiveness(const SourceManager& M);
+ void dumpBlockLiveness(const SourceManager &M);
+
+ /// Print to stderr the statement liveness information associated with
+ /// each basic block.
+ void dumpStmtLiveness(const SourceManager &M);
void runOnAllBlocks(Observer &obs);
Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=349320&r1=349319&r2=349320&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Sun Dec 16 15:44:06 2018
@@ -707,6 +707,9 @@ def DominatorsTreeDumper : Checker<"Dump
def LiveVariablesDumper : Checker<"DumpLiveVars">,
HelpText<"Print results of live variable analysis">;
+def LiveStatementsDumper : Checker<"DumpLiveStmts">,
+ HelpText<"Print results of live statement analysis">;
+
def CFGViewer : Checker<"ViewCFG">,
HelpText<"View Control-Flow Graphs using GraphViz">;
Modified: cfe/trunk/lib/Analysis/LiveVariables.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/LiveVariables.cpp?rev=349320&r1=349319&r2=349320&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/LiveVariables.cpp (original)
+++ cfe/trunk/lib/Analysis/LiveVariables.cpp Sun Dec 16 15:44:06 2018
@@ -93,6 +93,7 @@ public:
LiveVariables::Observer *obs = nullptr);
void dumpBlockLiveness(const SourceManager& M);
+ void dumpStmtLiveness(const SourceManager& M);
LiveVariablesImpl(AnalysisDeclContext &ac, bool KillAtAssign)
: analysisContext(ac),
@@ -327,6 +328,35 @@ void TransferFunctions::Visit(Stmt *S) {
// No need to unconditionally visit subexpressions.
return;
}
+ case Stmt::IfStmtClass: {
+ // If one of the branches is an expression rather than a compound
+ // statement, it will be bad if we mark it as live at the terminator
+ // of the if-statement (i.e., immediately after the condition expression).
+ AddLiveStmt(val.liveStmts, LV.SSetFact, cast<IfStmt>(S)->getCond());
+ return;
+ }
+ case Stmt::WhileStmtClass: {
+ // If the loop body is an expression rather than a compound statement,
+ // it will be bad if we mark it as live at the terminator of the loop
+ // (i.e., immediately after the condition expression).
+ AddLiveStmt(val.liveStmts, LV.SSetFact, cast<WhileStmt>(S)->getCond());
+ return;
+ }
+ case Stmt::DoStmtClass: {
+ // If the loop body is an expression rather than a compound statement,
+ // it will be bad if we mark it as live at the terminator of the loop
+ // (i.e., immediately after the condition expression).
+ AddLiveStmt(val.liveStmts, LV.SSetFact, cast<DoStmt>(S)->getCond());
+ return;
+ }
+ case Stmt::ForStmtClass: {
+ // If the loop body is an expression rather than a compound statement,
+ // it will be bad if we mark it as live at the terminator of the loop
+ // (i.e., immediately after the condition expression).
+ AddLiveStmt(val.liveStmts, LV.SSetFact, cast<ForStmt>(S)->getCond());
+ return;
+ }
+
}
for (Stmt *Child : S->children()) {
@@ -632,5 +662,23 @@ void LiveVariablesImpl::dumpBlockLivenes
llvm::errs() << "\n";
}
+void LiveVariables::dumpStmtLiveness(const SourceManager &M) {
+ getImpl(impl).dumpStmtLiveness(M);
+}
+
+void LiveVariablesImpl::dumpStmtLiveness(const SourceManager &M) {
+ // Don't iterate over blockEndsToLiveness directly because it's not sorted.
+ for (auto I : *analysisContext.getCFG()) {
+
+ llvm::errs() << "\n[ B" << I->getBlockID()
+ << " (live statements at block exit) ]\n";
+ for (auto S : blocksEndToLiveness[I].liveStmts) {
+ llvm::errs() << "\n";
+ S->dump();
+ }
+ llvm::errs() << "\n";
+ }
+}
+
const void *LiveVariables::getTag() { static int x; return &x; }
const void *RelaxedLiveVariables::getTag() { static int x; return &x; }
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp?rev=349320&r1=349319&r2=349320&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp Sun Dec 16 15:44:06 2018
@@ -69,6 +69,25 @@ void ento::registerLiveVariablesDumper(C
}
//===----------------------------------------------------------------------===//
+// LiveStatementsDumper
+//===----------------------------------------------------------------------===//
+
+namespace {
+class LiveStatementsDumper : public Checker<check::ASTCodeBody> {
+public:
+ void checkASTCodeBody(const Decl *D, AnalysisManager& Mgr,
+ BugReporter &BR) const {
+ if (LiveVariables *L = Mgr.getAnalysis<RelaxedLiveVariables>(D))
+ L->dumpStmtLiveness(Mgr.getSourceManager());
+ }
+};
+}
+
+void ento::registerLiveStatementsDumper(CheckerManager &mgr) {
+ mgr.registerChecker<LiveStatementsDumper>();
+}
+
+//===----------------------------------------------------------------------===//
// CFGViewer
//===----------------------------------------------------------------------===//
Added: cfe/trunk/test/Analysis/live-stmts.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/live-stmts.cpp?rev=349320&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/live-stmts.cpp (added)
+++ cfe/trunk/test/Analysis/live-stmts.cpp Sun Dec 16 15:44:06 2018
@@ -0,0 +1,167 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=debug.DumpLiveStmts %s 2>&1\
+// RUN: | FileCheck %s
+
+int coin();
+
+
+int testThatDumperWorks(int x, int y, int z) {
+ return x ? y : z;
+}
+// CHECK: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: ImplicitCastExpr {{.*}} <IntegralToBoolean>
+// CHECK-NEXT: `-ImplicitCastExpr {{.*}} <LValueToRValue>
+// CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int'
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B3 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: ImplicitCastExpr {{.*}} <IntegralToBoolean>
+// CHECK-NEXT: `-ImplicitCastExpr {{.*}} <LValueToRValue>
+// CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int'
+// CHECK: [ B4 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: ImplicitCastExpr {{.*}} <IntegralToBoolean>
+// CHECK-NEXT: `-ImplicitCastExpr {{.*}} <LValueToRValue>
+// CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int'
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B5 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int'
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int'
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+
+
+void testIfBranchExpression(bool flag) {
+ // No expressions should be carried over from one block to another here.
+ while (flag) {
+ int e = 1;
+ if (true)
+ e;
+ }
+}
+// CHECK: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B3 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B4 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B5 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+
+
+void testWhileBodyExpression(bool flag) {
+ // No expressions should be carried over from one block to another here.
+ while (flag) {
+ int e = 1;
+ while (coin())
+ e;
+ }
+}
+// CHECK: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B3 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B4 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B5 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+
+
+void testDoWhileBodyExpression(bool flag) {
+ // No expressions should be carried over from one block to another here.
+ while (flag) {
+ int e = 1;
+ do
+ e;
+ while (coin());
+ }
+}
+// CHECK: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B3 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B4 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B5 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+
+
+void testForBodyExpression(bool flag) {
+ // No expressions should be carried over from one block to another here.
+ while (flag) {
+ int e = 1;
+ for (; coin();)
+ e;
+ }
+}
+// CHECK: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B3 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B4 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK: [ B5 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+
Modified: cfe/trunk/test/Analysis/use-after-move.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/use-after-move.cpp?rev=349320&r1=349319&r2=349320&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/use-after-move.cpp (original)
+++ cfe/trunk/test/Analysis/use-after-move.cpp Sun Dec 16 15:44:06 2018
@@ -778,6 +778,45 @@ void checkLoopZombies() {
}
}
+void checkMoreLoopZombies1(bool flag) {
+ while (flag) {
+ Empty e{};
+ if (true)
+ e; // expected-warning {{expression result unused}}
+ Empty f = std::move(e); // no-warning
+ }
+}
+
+bool coin();
+
+void checkMoreLoopZombies2(bool flag) {
+ while (flag) {
+ Empty e{};
+ while (coin())
+ e; // expected-warning {{expression result unused}}
+ Empty f = std::move(e); // no-warning
+ }
+}
+
+void checkMoreLoopZombies3(bool flag) {
+ while (flag) {
+ Empty e{};
+ do
+ e; // expected-warning {{expression result unused}}
+ while (coin());
+ Empty f = std::move(e); // no-warning
+ }
+}
+
+void checkMoreLoopZombies4(bool flag) {
+ while (flag) {
+ Empty e{};
+ for (; coin();)
+ e; // expected-warning {{expression result unused}}
+ Empty f = std::move(e); // no-warning
+ }
+}
+
struct MoveOnlyWithDestructor {
MoveOnlyWithDestructor();
~MoveOnlyWithDestructor();
More information about the cfe-commits
mailing list