[PATCH] D35670: [StaticAnalyzer] Handle LoopExit CFGElement in the analyzer

2017-08-21 Thread Peter Szecsi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL311344: [StaticAnalyzer] Handle LoopExit CFGElement in the 
analyzer (authored by szepet).

Changed prior to commit:
  https://reviews.llvm.org/D35670?vs=110981=111993#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35670

Files:
  cfe/trunk/include/clang/Analysis/ProgramPoint.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -274,7 +274,8 @@
   assert(Loc.getAs() ||
  Loc.getAs() ||
  Loc.getAs() ||
- Loc.getAs());
+ Loc.getAs() ||
+ Loc.getAs());
   HandlePostStmt(WU.getBlock(), WU.getIndex(), Pred);
   break;
   }
@@ -566,7 +567,8 @@
 
   // Do not create extra nodes. Move to the next CFG element.
   if (N->getLocation().getAs() ||
-  N->getLocation().getAs()) {
+  N->getLocation().getAs()||
+  N->getLocation().getAs()) {
 WList->enqueue(N, Block, Idx+1);
 return;
   }
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -364,8 +364,10 @@
 case CFGElement::TemporaryDtor:
   ProcessImplicitDtor(E.castAs(), Pred);
   return;
-case CFGElement::LifetimeEnds:
 case CFGElement::LoopExit:
+  ProcessLoopExit(E.castAs().getLoopStmt(), Pred);
+  return;
+case CFGElement::LifetimeEnds:
   return;
   }
 }
@@ -510,6 +512,21 @@
   Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
 }
 
+void ExprEngine::ProcessLoopExit(const Stmt* S, ExplodedNode *Pred) {
+  PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
+S->getLocStart(),
+"Error evaluating end of the loop");
+  ExplodedNodeSet Dst;
+  Dst.Add(Pred);
+  NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+
+  LoopExit PP(S, Pred->getLocationContext());
+  Bldr.generateNode(PP, Pred->getState(), Pred);
+
+  // Enqueue the new nodes onto the work list.
+  Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
+}
+
 void ExprEngine::ProcessInitializer(const CFGInitializer Init,
 ExplodedNode *Pred) {
   const CXXCtorInitializer *BMI = Init.getInitializer();
@@ -2689,6 +2706,12 @@
 Out << "Epsilon Point";
 break;
 
+  case ProgramPoint::LoopExitKind: {
+LoopExit LE = Loc.castAs();
+Out << "LoopExit: " << LE.getLoopStmt()->getStmtClassName();
+break;
+  }
+
   case ProgramPoint::PreImplicitCallKind: {
 ImplicitCallPoint PC = Loc.castAs();
 Out << "PreCall: ";
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -196,6 +196,8 @@
 
   void ProcessStmt(const CFGStmt S, ExplodedNode *Pred);
 
+  void ProcessLoopExit(const Stmt* S, ExplodedNode *Pred);
+
   void ProcessInitializer(const CFGInitializer I, ExplodedNode *Pred);
 
   void ProcessImplicitDtor(const CFGImplicitDtor D, ExplodedNode *Pred);
Index: cfe/trunk/include/clang/Analysis/ProgramPoint.h
===
--- cfe/trunk/include/clang/Analysis/ProgramPoint.h
+++ cfe/trunk/include/clang/Analysis/ProgramPoint.h
@@ -83,6 +83,7 @@
   PostImplicitCallKind,
   MinImplicitCallKind = PreImplicitCallKind,
   MaxImplicitCallKind = PostImplicitCallKind,
+  LoopExitKind,
   EpsilonKind};
 
 private:
@@ -654,6 +655,29 @@
   }
 };
 
+/// Represents a point when we exit a loop.
+/// When this ProgramPoint is encountered we can be sure that the symbolic
+/// execution of the corresponding LoopStmt is finished on the given path.
+/// Note: It is possible to encounter a LoopExit element when we haven't even
+/// encountered the loop itself. At the current state not all loop exits will
+/// result in a LoopExit program point.
+class LoopExit : public ProgramPoint {
+public:
+LoopExit(const Stmt *LoopStmt, const LocationContext *LC)
+: ProgramPoint(LoopStmt, nullptr, LoopExitKind, LC) {}
+
+const Stmt *getLoopStmt() const {
+  return static_cast(getData1());
+}
+
+private:
+friend class ProgramPoint;
+LoopExit() {}
+static bool isKind(const ProgramPoint ) {
+  return Location.getKind() == LoopExitKind;

[PATCH] D35670: [StaticAnalyzer] Handle LoopExit CFGElement in the analyzer

2017-08-14 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin added a comment.

Also, please mention in the commit message that tests will be added in a 
following commit.


https://reviews.llvm.org/D35670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35670: [StaticAnalyzer] Handle LoopExit CFGElement in the analyzer

2017-08-14 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

Looks good to me! (Please expand the comment, though.)




Comment at: include/clang/Analysis/ProgramPoint.h:658
 
+class LoopExit : public ProgramPoint {
+public:

szepet wrote:
> dcoughlin wrote:
> > Can you add a comment explaining what meaning of this program point is.
> Can you help me with that? I am not sure what is important to say about this 
> point to understand it better than from its name.
My recommendation would be to think about it from the perspective of someone 
new to the codebase trying to understand when the analyzer will reach such a 
program point. What additional context will they need that is not conveyed in 
the name alone?

At a minimum, I think it is worth mentioning that a LoopExit point can arise 
even when a loop is not entered and that not all loop exits will result in a 
LoopExit program point.


https://reviews.llvm.org/D35670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35670: [StaticAnalyzer] Handle LoopExit CFGElement in the analyzer

2017-08-14 Thread Peter Szecsi via Phabricator via cfe-commits
szepet added inline comments.



Comment at: include/clang/Analysis/ProgramPoint.h:658
 
+class LoopExit : public ProgramPoint {
+public:

dcoughlin wrote:
> Can you add a comment explaining what meaning of this program point is.
Can you help me with that? I am not sure what is important to say about this 
point to understand it better than from its name.



Comment at: lib/StaticAnalyzer/Core/CoreEngine.cpp:586
 
+  if ((*Block)[Idx].getKind() == CFGElement::LoopExit) {
+WList->enqueue(N, Block, Idx+1);

dcoughlin wrote:
> I'm surprised both this and the checks for N's location above are needed. How 
> does this arise?
We don't need both of the checks, I just left it by mistake.


https://reviews.llvm.org/D35670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35670: [StaticAnalyzer] Handle LoopExit CFGElement in the analyzer

2017-08-14 Thread Peter Szecsi via Phabricator via cfe-commits
szepet updated this revision to Diff 110981.
szepet marked an inline comment as done.
szepet added a comment.

Updates based on comments.


https://reviews.llvm.org/D35670

Files:
  include/clang/Analysis/ProgramPoint.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -364,8 +364,10 @@
 case CFGElement::TemporaryDtor:
   ProcessImplicitDtor(E.castAs(), Pred);
   return;
-case CFGElement::LifetimeEnds:
 case CFGElement::LoopExit:
+  ProcessLoopExit(E.castAs().getLoopStmt(), Pred);
+  return;
+case CFGElement::LifetimeEnds:
   return;
   }
 }
@@ -510,6 +512,21 @@
   Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
 }
 
+void ExprEngine::ProcessLoopExit(const Stmt* S, ExplodedNode *Pred) {
+  PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
+S->getLocStart(),
+"Error evaluating end of the loop");
+  ExplodedNodeSet Dst;
+  Dst.Add(Pred);
+  NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+
+  LoopExit PP(S, Pred->getLocationContext());
+  Bldr.generateNode(PP, Pred->getState(), Pred);
+
+  // Enqueue the new nodes onto the work list.
+  Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
+}
+
 void ExprEngine::ProcessInitializer(const CFGInitializer Init,
 ExplodedNode *Pred) {
   const CXXCtorInitializer *BMI = Init.getInitializer();
@@ -2689,6 +2706,12 @@
 Out << "Epsilon Point";
 break;
 
+  case ProgramPoint::LoopExitKind: {
+LoopExit LE = Loc.castAs();
+Out << "LoopExit: " << LE.getLoopStmt()->getStmtClassName();
+break;
+  }
+
   case ProgramPoint::PreImplicitCallKind: {
 ImplicitCallPoint PC = Loc.castAs();
 Out << "PreCall: ";
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -274,7 +274,8 @@
   assert(Loc.getAs() ||
  Loc.getAs() ||
  Loc.getAs() ||
- Loc.getAs());
+ Loc.getAs() ||
+ Loc.getAs());
   HandlePostStmt(WU.getBlock(), WU.getIndex(), Pred);
   break;
   }
@@ -566,7 +567,8 @@
 
   // Do not create extra nodes. Move to the next CFG element.
   if (N->getLocation().getAs() ||
-  N->getLocation().getAs()) {
+  N->getLocation().getAs()||
+  N->getLocation().getAs()) {
 WList->enqueue(N, Block, Idx+1);
 return;
   }
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -196,6 +196,8 @@
 
   void ProcessStmt(const CFGStmt S, ExplodedNode *Pred);
 
+  void ProcessLoopExit(const Stmt* S, ExplodedNode *Pred);
+
   void ProcessInitializer(const CFGInitializer I, ExplodedNode *Pred);
 
   void ProcessImplicitDtor(const CFGImplicitDtor D, ExplodedNode *Pred);
Index: include/clang/Analysis/ProgramPoint.h
===
--- include/clang/Analysis/ProgramPoint.h
+++ include/clang/Analysis/ProgramPoint.h
@@ -83,6 +83,7 @@
   PostImplicitCallKind,
   MinImplicitCallKind = PreImplicitCallKind,
   MaxImplicitCallKind = PostImplicitCallKind,
+  LoopExitKind,
   EpsilonKind};
 
 private:
@@ -654,6 +655,24 @@
   }
 };
 
+/// Represents a point when we exit a loop.
+class LoopExit : public ProgramPoint {
+public:
+LoopExit(const Stmt *LoopStmt, const LocationContext *LC)
+: ProgramPoint(LoopStmt, nullptr, LoopExitKind, LC) {}
+
+const Stmt *getLoopStmt() const {
+  return static_cast(getData1());
+}
+
+private:
+friend class ProgramPoint;
+LoopExit() {}
+static bool isKind(const ProgramPoint ) {
+  return Location.getKind() == LoopExitKind;
+}
+};
+
 /// This is a meta program point, which should be skipped by all the diagnostic
 /// reasoning etc.
 class EpsilonPoint : public ProgramPoint {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35670: [StaticAnalyzer] Handle LoopExit CFGElement in the analyzer

2017-08-07 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin added inline comments.



Comment at: include/clang/Analysis/ProgramPoint.h:658
 
+class LoopExit : public ProgramPoint {
+public:

Can you add a comment explaining what meaning of this program point is.



Comment at: lib/StaticAnalyzer/Core/CoreEngine.cpp:586
 
+  if ((*Block)[Idx].getKind() == CFGElement::LoopExit) {
+WList->enqueue(N, Block, Idx+1);

I'm surprised both this and the checks for N's location above are needed. How 
does this arise?


https://reviews.llvm.org/D35670



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35670: [StaticAnalyzer] Handle LoopExit CFGElement in the analyzer

2017-07-26 Thread Peter Szecsi via Phabricator via cfe-commits
szepet updated this revision to Diff 108312.
szepet added a subscriber: cfe-commits.
szepet added a comment.

Accidentally left debug print removed.


https://reviews.llvm.org/D35670

Files:
  include/clang/Analysis/ProgramPoint.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -364,8 +364,10 @@
 case CFGElement::TemporaryDtor:
   ProcessImplicitDtor(E.castAs(), Pred);
   return;
-case CFGElement::LifetimeEnds:
 case CFGElement::LoopExit:
+  ProcessLoopExit(E.castAs().getLoopStmt(), Pred);
+  return;
+case CFGElement::LifetimeEnds:
   return;
   }
 }
@@ -510,6 +512,21 @@
   Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
 }
 
+void ExprEngine::ProcessLoopExit(const Stmt* S, ExplodedNode *Pred) {
+  PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
+S->getLocStart(),
+"Error evaluating end of the loop");
+  ExplodedNodeSet Dst;
+  Dst.Add(Pred);
+  NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+
+  LoopExit PP(S, Pred->getLocationContext());
+  Bldr.generateNode(PP, Pred->getState(), Pred);
+
+  // Enqueue the new nodes onto the work list.
+  Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
+}
+
 void ExprEngine::ProcessInitializer(const CFGInitializer Init,
 ExplodedNode *Pred) {
   const CXXCtorInitializer *BMI = Init.getInitializer();
@@ -2689,6 +2706,12 @@
 Out << "Epsilon Point";
 break;
 
+  case ProgramPoint::LoopExitKind: {
+LoopExit LE = Loc.castAs();
+Out << "LoopExit: " << LE.getLoopStmt()->getStmtClassName();
+break;
+  }
+
   case ProgramPoint::PreImplicitCallKind: {
 ImplicitCallPoint PC = Loc.castAs();
 Out << "PreCall: ";
Index: lib/StaticAnalyzer/Core/CoreEngine.cpp
===
--- lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -274,7 +274,8 @@
   assert(Loc.getAs() ||
  Loc.getAs() ||
  Loc.getAs() ||
- Loc.getAs());
+ Loc.getAs() ||
+ Loc.getAs());
   HandlePostStmt(WU.getBlock(), WU.getIndex(), Pred);
   break;
   }
@@ -566,7 +567,8 @@
 
   // Do not create extra nodes. Move to the next CFG element.
   if (N->getLocation().getAs() ||
-  N->getLocation().getAs()) {
+  N->getLocation().getAs()||
+  N->getLocation().getAs()) {
 WList->enqueue(N, Block, Idx+1);
 return;
   }
@@ -581,6 +583,11 @@
 return;
   }
 
+  if ((*Block)[Idx].getKind() == CFGElement::LoopExit) {
+WList->enqueue(N, Block, Idx+1);
+return;
+  }
+
   // At this point, we know we're processing a normal statement.
   CFGStmt CS = (*Block)[Idx].castAs();
   PostStmt Loc(CS.getStmt(), N->getLocationContext());
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -196,6 +196,8 @@
 
   void ProcessStmt(const CFGStmt S, ExplodedNode *Pred);
 
+  void ProcessLoopExit(const Stmt* S, ExplodedNode *Pred);
+
   void ProcessInitializer(const CFGInitializer I, ExplodedNode *Pred);
 
   void ProcessImplicitDtor(const CFGImplicitDtor D, ExplodedNode *Pred);
Index: include/clang/Analysis/ProgramPoint.h
===
--- include/clang/Analysis/ProgramPoint.h
+++ include/clang/Analysis/ProgramPoint.h
@@ -83,6 +83,7 @@
   PostImplicitCallKind,
   MinImplicitCallKind = PreImplicitCallKind,
   MaxImplicitCallKind = PostImplicitCallKind,
+  LoopExitKind,
   EpsilonKind};
 
 private:
@@ -654,6 +655,23 @@
   }
 };
 
+class LoopExit : public ProgramPoint {
+public:
+LoopExit(const Stmt *LoopStmt, const LocationContext *LC)
+: ProgramPoint(LoopStmt, nullptr, LoopExitKind, LC) {}
+
+const Stmt *getLoopStmt() const {
+  return static_cast(getData1());
+}
+
+private:
+LoopExit() {}
+friend class ProgramPoint;
+static bool isKind(const ProgramPoint ) {
+  return Location.getKind() == LoopExitKind;
+}
+};
+
 /// This is a meta program point, which should be skipped by all the diagnostic
 /// reasoning etc.
 class EpsilonPoint : public ProgramPoint {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits