[PATCH] D16403: Add scope information to CFG

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC327258: [analyzer] Add scope information to CFG (authored by 
chefmax, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -632,6 +632,8 @@
   ProcessLoopExit(E.castAs().getLoopStmt(), Pred);
   return;
 case CFGElement::LifetimeEnds:
+case CFGElement::ScopeBegin:
+case CFGElement::ScopeEnd:
   return;
   }
 }
Index: lib/StaticAnalyzer/Core/AnalysisManager.cpp
===
--- lib/StaticAnalyzer/Core/AnalysisManager.cpp
+++ lib/StaticAnalyzer/Core/AnalysisManager.cpp
@@ -26,6 +26,7 @@
 // Adding LoopExit elements to the CFG is a requirement for loop
 // unrolling.
 Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(),
+Options.includeScopesInCFG(),
 Options.shouldSynthesizeBodies(),
 Options.shouldConditionalizeStaticInitializers(),
 /*addCXXNewAllocator=*/true,
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -614,6 +614,9 @@
 return PathDiagnosticLocation::createEnd(Dtor.getBindTemporaryExpr(), SM,
  CallerCtx);
   }
+  case CFGElement::ScopeBegin:
+  case CFGElement::ScopeEnd:
+llvm_unreachable("not yet implemented!");
   case CFGElement::LifetimeEnds:
   case CFGElement::LoopExit:
 llvm_unreachable("CFGElement kind should not be on callsite!");
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -224,6 +224,12 @@
   /* Default = */ true);
 }
 
+bool AnalyzerOptions::includeScopesInCFG() {
+  return getBooleanOption(IncludeScopesInCFG,
+  "cfg-scopes",
+  /* Default = */ false);
+}
+
 bool AnalyzerOptions::mayInlineCXXStandardLibrary() {
   return getBooleanOption(InlineCXXStandardLibrary,
   "c++-stdlib-inlining",
Index: lib/Analysis/CFG.cpp
===
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -234,6 +234,13 @@
   assert(VarIter != 0 && "Iterator has invalid value of VarIter member");
   return >Vars[VarIter - 1];
 }
+
+const VarDecl *getFirstVarInScope() const {
+  assert(Scope && "Dereferencing invalid iterator is not allowed");
+  assert(VarIter != 0 && "Iterator has invalid value of VarIter member");
+  return Scope->Vars[0];
+}
+
 VarDecl *operator*() const {
   return *this->operator->();
 }
@@ -267,6 +274,7 @@
 
 int distance(const_iterator L);
 const_iterator shared_parent(const_iterator L);
+bool pointsToFirstDeclaredVar() { return VarIter == 1; }
   };
 
 private:
@@ -479,6 +487,9 @@
   llvm::DenseMap
   ConstructionContextMap;
 
+  using DeclsWithEndedScopeSetTy = llvm::SmallSetVector;
+  DeclsWithEndedScopeSetTy DeclsWithEndedScope;
+
   bool badCFG = false;
   const CFG::BuildOptions 
   
@@ -576,6 +587,12 @@
   CFGBlock *VisitChildren(Stmt *S);
   CFGBlock *VisitNoRecurse(Expr *E, AddStmtChoice asc);
 
+  void maybeAddScopeBeginForVarDecl(CFGBlock *B, const VarDecl *VD,
+const Stmt *S) {
+if (ScopePos && (VD == ScopePos.getFirstVarInScope()))
+  appendScopeBegin(B, VD, S);
+  }
+
   /// When creating the CFG for temporary destructors, we want to mirror the
   /// branch structure of the corresponding constructor calls.
   /// Thus, while visiting a statement for temporary destructors, we keep a
@@ -688,6 +705,11 @@
   void addAutomaticObjHandling(LocalScope::const_iterator B,
LocalScope::const_iterator E, Stmt *S);
   void addImplicitDtorsForDestructor(const CXXDestructorDecl *DD);
+  void addScopesEnd(LocalScope::const_iterator B, LocalScope::const_iterator E,
+Stmt *S);
+
+  void 

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D16403#1031567, @thakis wrote:

> Since this affects analysis-based warnings, have you checked if this patch 
> has any effect on compile times?


Just in case - this is under an analyzer-only flag, so the actual warnings are 
not affected. I guess it might be useful to evaluate compilation time impact 
caused by checking the flag and bailing out, but i doubt it'd be noticeable.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Since this affects analysis-based warnings, have you checked if this patch has 
any effect on compile times?


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/Analysis/CFG.cpp:1700
 /// way return valid LocalScope object.
 LocalScope* CFGBuilder::createOrReuseLocalScope(LocalScope* Scope) {
   if (Scope)

NoQ wrote:
> It seems that something  has moved asterisks to a weird spot, might be a 
> rebase artifact or accidental tooling.
Uhm, it was always this way, just weird history when i looked at the diff 
between diffs, nvm, sorry.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

I think we should land this and celebrate.

@szepet: Ouch, i was sure i already answered this, sorry, dunno where this went.

> So, LoopExit and ScopeExit would be the same but the underlying TriggerStmt 
> would decide which marks a loop and which marks a variable?

I was thinking of a single `CFGElement` to mark both. Which would probably be 
called "`ScopeExit`", but you could use it for detecting the end of the loop 
and, if necessary, add whatever information you need into it.




Comment at: lib/Analysis/CFG.cpp:1700
 /// way return valid LocalScope object.
 LocalScope* CFGBuilder::createOrReuseLocalScope(LocalScope* Scope) {
   if (Scope)

It seems that something  has moved asterisks to a weird spot, might be a rebase 
artifact or accidental tooling.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

Now, the cfg for this example:

  void test_for_implicit_scope() {
for (A a; A b = a;)
  A c;
  }

looks like this:
F5874197: CFG-implicit-for.dot 


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 137150.
m.ostapenko added a comment.

Rebased and updated. Fix the issue with ordering between ScopeEnds and implicit 
destructors.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1171 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: 1
+// CHECK-NEXT:   2: return [B1.1];
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, [B1.3], class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, [B1.5], class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   7: CFGScopeEnd(a)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, [B1.3], class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(c)
+// CHECK-NEXT:   5:  (CXXConstructExpr, [B1.6], class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, [B1.8], class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  10: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  11: CFGScopeEnd(c)
+// CHECK-NEXT:  12:  (CXXConstructExpr, [B1.13], class A)
+// CHECK-NEXT:  13: A b;
+// CHECK-NEXT:  14: [B1.13].~A() (Implicit destructor)
+// CHECK-NEXT:  15: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  16: CFGScopeEnd(a)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, [B1.2], class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   6: CFGScopeEnd(a)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: return;
+// CHECK-NEXT:   2: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   3: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   4: CFGScopeEnd(a)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B3]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, [B3.3], class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4:  (CXXConstructExpr, [B3.5], class A)
+// CHECK-NEXT:   5: A b;
+// CHECK-NEXT:   6: UV
+// CHECK-NEXT:   7: [B3.6] (ImplicitCastExpr, LValueToRValue, _Bool)
+// CHECK-NEXT:   T: if [B3.7]
+// CHECK-NEXT:   Preds (1): B4
+// CHECK-NEXT:   Succs (2): B2 B1
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (2): B1 B2
+void test_return() {
+  A a;
+  A b;
+  if (UV) return;
+  A c;
+}
+
+// CHECK:  [B5 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B4
+// CHECK:  [B1]
+// CHECK-NEXT:   1: [B4.8].~A() (Implicit destructor)
+// CHECK-NEXT:   2: CFGScopeEnd(b)
+// CHECK-NEXT:   3: [B4.3].~A() (Implicit destructor)
+// CHECK-NEXT:   4: CFGScopeEnd(a)
+// CHECK-NEXT:   Preds (2): B2 B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(c)
+// CHECK-NEXT:   2:  (CXXConstructExpr, [B2.3], class A)
+// CHECK-NEXT:   3: A c;
+// CHECK-NEXT:   4: 

[PATCH] D16403: Add scope information to CFG

[Peter Szecsi via Phabricator via cfe-commits]

szepet added a comment.

In https://reviews.llvm.org/D16403#1011218, @NoQ wrote:

> Yeah, i mean, like, if we change the scope markers to also appear even when 
> no variables are present in the scope, then it would be possible to replace 
> loop markers with some of the scope markers, right?


OK, probably I am a bit too slow for this, but I dont get it. Yes, it would be 
possible to replace them IF these markers appears even in case of no variables. 
However, this patch is based on LocalScopes which practically means that 
VarDecls are needed.  Aaand here we are, it would require a different approach 
to consistently mark things like LoopExit.
 Another thing that what should be the TriggerStmt? I guess the Stmt of the 
loop. So, LoopExit and ScopeExit would be the same but the underlying 
TriggerStmt would decide which marks a loop and which marks a variable? Then I 
can just rewrite LoopExit into ScopeEnd. Or if you would like to see that, a 
subclass of ScopeEnd. However, the main functionality should stay the same (I 
guess).
So, could you help me out what are those things I do not see/understand?


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D16403#1013346, @m.ostapenko wrote:

> In https://reviews.llvm.org/D16403#1011218, @NoQ wrote:
>
> > Yeah, i mean, like, if we change the scope markers to also appear even when 
> > no variables are present in the scope, then it would be possible to replace 
> > loop markers with some of the scope markers, right?
>
>
> Hm, so does this mean that I need to cover the case when no variables are 
> present in loop scope here in this patch?


Definitely not :) Feel free to address it some day (or let someone else address 
it) - the culture of keeping our patches small is something that we currently 
badly lack :)


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

In https://reviews.llvm.org/D16403#1001466, @NoQ wrote:

> I poked Devin offline and we agreed that the overall approach is good to go. 
> Maxim, thank you for picking it up!
>
> We still don't have scopes for segments of code that don't have any variables 
> in them, so i guess it's not yet in the shape where it is super useful for 
> loops, but it's already useful for finding use of stale stack variables, 
> which was the whole point originally, so i think this should definitely land 
> soon.
>
> In https://reviews.llvm.org/D16403#993512, @m.ostapenko wrote:
>
> > In https://reviews.llvm.org/D16403#992452, @NoQ wrote:
> >
> > > Am i understanding correctly that while destroying `a` you can still use 
> > > the storage of `b` safely? Or should `a` go out of scope before `b` gets 
> > > destroyed?
> >
> >
> > AFAIU, when we destroying `a` we can still use the storage of `b` because 
> > nothing can be allocated into b's storage between calling destructors for 
> > `b` and `a`. So, imho the sequence should look like:
> >
> >   [B4.5].~A() (Implicit destructor)
> >   [B5.3].~A() (Implicit destructor)
> >   CFGScopeEnd(b)
> >   CFGScopeEnd(a)
> >  
> >
>
>
> Thought about it a bit more and this still doesn't look correct to me. Like, 
> `a.~A()` is a function call. It can do a lot of unexpected stuff to registers 
> and stack space before jumping into the function. And given that `a` and `b` 
> are in different scopes (`a` is in loop scope, `b` is in single iteration 
> scope), storage of `b` is not protected from such stuff during call to the 
> destructor of `a`. So there's definitely something fishy here. I guess scope 
> ends and destructors would have to be properly interleaved in all cases of 
> exiting multiple scopes.


Sounds reasonable, I'll fix this.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

In https://reviews.llvm.org/D16403#1011218, @NoQ wrote:

> In https://reviews.llvm.org/D16403#1010096, @szepet wrote:
>
> > In https://reviews.llvm.org/D16403#992454, @NoQ wrote:
> >
> > > @szepet: so i see that `LoopExit` goes in the beginning of the cleanup 
> > > block after the loop, while various `ScopeEnd`s go after the `LoopExit`. 
> > > Would loop unrolling be significantly broken if you simply subscribe to 
> > > `ScopeEnd` instead of `LoopExit` and avoid cleaning up the loop state 
> > > until destructors are processed? I might not be remembering correctly - 
> > > is `LoopExit` only used for cleanup, or do we have more work to be done 
> > > here?
> >
> >
> > I guess your following comment just answers this:
> >
> > In https://reviews.llvm.org/D16403#1001466, @NoQ wrote:
> >
> > > We still don't have scopes for segments of code that don't have any 
> > > variables in them, so i guess it's not yet in the shape where it is super 
> > > useful for loops, but it's already useful for finding use of stale stack 
> > > variables, which was the whole point originally, so i think this should 
> > > definitely land soon.
> >
> >
> > It could be, however, we would lose cases like:
> >
> >   int i = 0;
> >   int a[32];
> >   for(i = 0;i<32;++i) {a[i] = i;}
> >
> >
> > Since there is no variable which has the scope of the loop, ScopeEnd would 
> > be not enough. Sure, we could remove this case, however, the aim is to 
> > extend the loop-patterns for completely unrolling. Another thing that there 
> > are the patches which would enhance the covered cases by LoopExit 
> > (https://reviews.llvm.org/D39398) and add the LoopEntrance to the 
> > CFG(https://reviews.llvm.org/D41150) as well. So, at this point, I feel 
> > like it would be a huge step back not to use these elements. (Sorry Maxim 
> > that we are discussing this here^^)
>
>
> Yeah, i mean, like, if we change the scope markers to also appear even when 
> no variables are present in the scope, then it would be possible to replace 
> loop markers with some of the scope markers, right?


Hm, so does this mean that I need to cover the case when no variables are 
present in loop scope here in this patch?


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D16403#1010096, @szepet wrote:

> In https://reviews.llvm.org/D16403#992454, @NoQ wrote:
>
> > @szepet: so i see that `LoopExit` goes in the beginning of the cleanup 
> > block after the loop, while various `ScopeEnd`s go after the `LoopExit`. 
> > Would loop unrolling be significantly broken if you simply subscribe to 
> > `ScopeEnd` instead of `LoopExit` and avoid cleaning up the loop state until 
> > destructors are processed? I might not be remembering correctly - is 
> > `LoopExit` only used for cleanup, or do we have more work to be done here?
>
>
> I guess your following comment just answers this:
>
> In https://reviews.llvm.org/D16403#1001466, @NoQ wrote:
>
> > We still don't have scopes for segments of code that don't have any 
> > variables in them, so i guess it's not yet in the shape where it is super 
> > useful for loops, but it's already useful for finding use of stale stack 
> > variables, which was the whole point originally, so i think this should 
> > definitely land soon.
>
>
> It could be, however, we would lose cases like:
>
>   int i = 0;
>   int a[32];
>   for(i = 0;i<32;++i) {a[i] = i;}
>
>
> Since there is no variable which has the scope of the loop, ScopeEnd would be 
> not enough. Sure, we could remove this case, however, the aim is to extend 
> the loop-patterns for completely unrolling. Another thing that there are the 
> patches which would enhance the covered cases by LoopExit 
> (https://reviews.llvm.org/D39398) and add the LoopEntrance to the 
> CFG(https://reviews.llvm.org/D41150) as well. So, at this point, I feel like 
> it would be a huge step back not to use these elements. (Sorry Maxim that we 
> are discussing this here^^)


Yeah, i mean, like, if we change the scope markers to also appear even when no 
variables are present in the scope, then it would be possible to replace loop 
markers with some of the scope markers, right?


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Peter Szecsi via Phabricator via cfe-commits]

szepet added a comment.

In https://reviews.llvm.org/D16403#992452, @NoQ wrote:

> Thank you, this explanation looks very reasonable.
>
> All right, so right after the termination of the loop we have
>
>   [B1]
>   1: ForStmt (LoopExit)
>   2: [B4.5].~A() (Implicit destructor)
>   3: [B5.3].~A() (Implicit destructor)
>   4: CFGScopeEnd(a)
>   5: CFGScopeEnd(b)
>
>
> ... where `[B4.5]` is `A b = a;` and `[B5.3]` is `A a;`. Am i understanding 
> correctly that while destroying `a` you can still use the storage of `b` 
> safely? Or should `a` go out of scope before `b` gets destroyed? Also, is the 
> order of scope ends actually correct here - shouldn't `b` go out of scope 
> earlier? Given that they are in very different lifetime scopes (`a` is one 
> for the whole loop, `b` is per-loop-iteration). I guess the order would 
> matter for the analyzer.


I guess CFGScopeEnd should happen (should be encountered) before the LoopExit 
and CFGScopeBegin should be after LoopEntrance (still not sure if it is going 
to be part of the CFG ever.) Just like parenthesis {(())}  that is what would 
make the most sense for me. However, I do not want to block this patch or 
something so I can do these changes as well by modifying LoopExit (and probably 
should since ImplicitDestructor calls should precede the LoopExit as well).

In https://reviews.llvm.org/D16403#992454, @NoQ wrote:

> @szepet: so i see that `LoopExit` goes in the beginning of the cleanup block 
> after the loop, while various `ScopeEnd`s go after the `LoopExit`. Would loop 
> unrolling be significantly broken if you simply subscribe to `ScopeEnd` 
> instead of `LoopExit` and avoid cleaning up the loop state until destructors 
> are processed? I might not be remembering correctly - is `LoopExit` only used 
> for cleanup, or do we have more work to be done here?


I guess your following comment just answers this:

In https://reviews.llvm.org/D16403#1001466, @NoQ wrote:

> We still don't have scopes for segments of code that don't have any variables 
> in them, so i guess it's not yet in the shape where it is super useful for 
> loops, but it's already useful for finding use of stale stack variables, 
> which was the whole point originally, so i think this should definitely land 
> soon.


It could be, however, we would lose cases like:

  int i = 0;
  int a[32];
  for(i = 0;i<32;++i) {a[i] = i;}

Since there is no variable which has the scope of the loop, ScopeEnd would be 
not enough. Sure, we could remove this case, however, the aim is to extend the 
loop-patterns for completely unrolling. Another thing that there are the 
patches which would enhance the covered cases by LoopExit 
(https://reviews.llvm.org/D39398) and add the LoopEntrance to the 
CFG(https://reviews.llvm.org/D41150) as well. So, at this point, I feel like it 
would be a huge step back not to use these elements. (Sorry Maxim that we are 
discussing this here^^)


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a reviewer: rsmith.
NoQ added a subscriber: rsmith.
NoQ added a comment.

Added @rsmith because we're trying to give him a heads up every time large CFG 
changes are coming, because if we mess up it would affect not only the analyzer 
but the whole compiler through analysis-based compiler warnings, just in case.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I poked Devin offline and we agreed that the overall approach is good to go. 
Maxim, thank you for picking it up!

We still don't have scopes for segments of code that don't have any variables 
in them, so i guess it's not yet in the shape where it is super useful for 
loops, but it's already useful for finding use of stale stack variables, which 
was the whole point originally, so i think this should definitely land soon.

In https://reviews.llvm.org/D16403#993512, @m.ostapenko wrote:

> In https://reviews.llvm.org/D16403#992452, @NoQ wrote:
>
> > Am i understanding correctly that while destroying `a` you can still use 
> > the storage of `b` safely? Or should `a` go out of scope before `b` gets 
> > destroyed?
>
>
> AFAIU, when we destroying `a` we can still use the storage of `b` because 
> nothing can be allocated into b's storage between calling destructors for `b` 
> and `a`. So, imho the sequence should look like:
>
>   [B4.5].~A() (Implicit destructor)
>   [B5.3].~A() (Implicit destructor)
>   CFGScopeEnd(b)
>   CFGScopeEnd(a)
>  
>


Thought about it a bit more and this still doesn't look correct to me. Like, 
`a.~A()` is a function call. It can do a lot of unexpected stuff to registers 
and stack space before jumping into the function. And given that `a` and `b` 
are in different scopes (`a` is in loop scope, `b` is in single iteration 
scope), storage of `b` is not protected from such stuff during call to the 
destructor of `a`. So there's definitely something fishy here. I guess scope 
ends and destructors would have to be properly interleaved in all cases of 
exiting multiple scopes.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 132393.
m.ostapenko added a comment.

Fix scope ends order (as discussed above) and adjust a testcase.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1168 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: 1
+// CHECK-NEXT:   2: return [B1.1];
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   7: CFGScopeEnd(a)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(c)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  10: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  11: CFGScopeEnd(c)
+// CHECK-NEXT:  12:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  13: A b;
+// CHECK-NEXT:  14: [B1.13].~A() (Implicit destructor)
+// CHECK-NEXT:  15: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  16: CFGScopeEnd(a)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   6: CFGScopeEnd(a)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: return;
+// CHECK-NEXT:   2: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   3: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   4: CFGScopeEnd(a)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B3]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   5: A b;
+// CHECK-NEXT:   6: UV
+// CHECK-NEXT:   7: [B3.6] (ImplicitCastExpr, LValueToRValue, _Bool)
+// CHECK-NEXT:   T: if [B3.7]
+// CHECK-NEXT:   Preds (1): B4
+// CHECK-NEXT:   Succs (2): B2 B1
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (2): B1 B2
+void test_return() {
+  A a;
+  A b;
+  if (UV) return;
+  A c;
+}
+
+// CHECK:  [B5 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B4
+// CHECK:  [B1]
+// CHECK-NEXT:   1: [B4.8].~A() (Implicit destructor)
+// CHECK-NEXT:   2: CFGScopeEnd(b)
+// CHECK-NEXT:   3: [B4.3].~A() (Implicit destructor)
+// CHECK-NEXT:   4: CFGScopeEnd(a)
+// CHECK-NEXT:   Preds (2): B2 B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(c)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A c;
+// CHECK-NEXT:   4: [B2.3].~A() (Implicit destructor)
+// CHECK-NEXT:   5: CFGScopeEnd(c)
+// CHECK-NEXT:   Preds (1): B4
+// 

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

In https://reviews.llvm.org/D16403#992452, @NoQ wrote:

> Thank you, this explanation looks very reasonable.
>
> All right, so right after the termination of the loop we have
>
>   [B1]
>   1: ForStmt (LoopExit)
>   2: [B4.5].~A() (Implicit destructor)
>   3: [B5.3].~A() (Implicit destructor)
>   4: CFGScopeEnd(a)
>   5: CFGScopeEnd(b)
>
>
> ... where `[B4.5]` is `A b = a;` and `[B5.3]` is `A a;`. Am i understanding 
> correctly that while destroying `a` you can still use the storage of `b` 
> safely? Or should `a` go out of scope before `b` gets destroyed?


AFAIU, when we destroying `a` we can still use the storage of `b` because 
nothing can be allocated into b's storage between calling destructors for `b` 
and `a`. So, imho the sequence should look like:

  [B4.5].~A() (Implicit destructor)
  [B5.3].~A() (Implicit destructor)
  CFGScopeEnd(b)
  CFGScopeEnd(a)



> Also, is the order of scope ends actually correct here - shouldn't `b` go out 
> of scope earlier? Given that they are in very different lifetime scopes (`a` 
> is one for the whole loop, `b` is per-loop-iteration). I guess the order 
> would matter for the analyzer.

Yeah, this is a bug in a current implementation. Will fix this in the next 
patch version.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

@szepet: so i see that `LoopExit` goes in the beginning of the cleanup block 
after the loop, while various `ScopeEnd`s go after the `LoopExit`. Would loop 
unrolling be significantly broken if you simply subscribe to `ScopeEnd` instead 
of `LoopExit` and avoid cleaning up the loop state until destructors are 
processed? I might not be remembering correctly - is `LoopExit` only used for 
cleanup, or do we have more work to be done here?


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thank you, this explanation looks very reasonable.

All right, so right after the termination of the loop we have

  [B1]
  1: ForStmt (LoopExit)
  2: [B4.5].~A() (Implicit destructor)
  3: [B5.3].~A() (Implicit destructor)
  4: CFGScopeEnd(a)
  5: CFGScopeEnd(b)

... where `[B4.5]` is `A b = a;` and `[B5.3]` is `A a;`. Am i understanding 
correctly that while destroying `a` you can still use the storage of `b` 
safely? Or should `a` go out of scope before `b` gets destroyed? Also, is the 
order of scope ends actually correct here - shouldn't `b` go out of scope 
earlier? Given that they are in very different lifetime scopes (`a` is one for 
the whole loop, `b` is per-loop-iteration). I guess the order would matter for 
the analyzer.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

Actually upload the files.
F5792949: CFG-scopes-destructors-loopexit.dot 


F5792948: CFG-scopes-loopexit-lifetime.dot 


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

Hi Artem,

In https://reviews.llvm.org/D16403#989451, @NoQ wrote:

> Hmm. @m.ostapenko @szepet @mgehre - I think it might be a good time to figure 
> out if `ScopeBegin`/`ScopeEnd`, `LoopEntrance`/`LoopExit`, `LifetimeEnds`, 
> `AutomaticObjectDtor` elements work nicely together. How should they be 
> ordered with respect to each other?


Here are several observations about `ScopeBegin`/`ScopeEnd`, 
`LoopEntrance`/`LoopExit`, `LifetimeEnds`, `AutomaticObjectDtor` interaction:

1. `LifetimeEnds` and  `AutomaticObjectDtor` don't work together (I'm unable to 
tell why).
2. The difference between `ScopeBegin`/`ScopeEnd` and  `LifetimeEnds` was 
described by Devin several comments above, in short:
  - `LifetimeEnds` emits markers for when the lifetime of a C++ object in an 
automatic variable ends. For C++ objects with non-trivial destructors, this 
point is when the destructor is called. At this point the storage for the 
variable still exists, but what you can do with that storage is very restricted 
by the language because its contents have been destroyed.
  - `ScopeBegin`/`ScopeEnd` add markers for when the storage duration for the 
variable begins and ends. Hence I can conclude that `ScopeEnd` should reside 
after implicit destructors and `LifetimeEnds` markers in CFG.
3. `LoopEntrance`/`LoopExit` improve modelling of unrolled loops and I'm not 
sure whether scopes across iterations are the only thing that's modeled here.

> Is any of these scope representation a superset of another scope 
> representation, or maybe fully covered by other two or three other scope 
> representations?

Given my observations above, I'm not sure whether some representation can be 
fully covered by others -- all of them serve different purposes. Or perhaps I'm 
missing something?

> Would anybody be willing to produce some pictures (`-analyzer-checker 
> debug.ViewCFG` and attach here) with current and/or intended behavior?

I'm attaching two CFGs for the following example:

  void test_for_implicit_scope() {
for (A a; A b = a; )
  A c;
  }

  $ ./bin/clang -cc1 -fcxx-exceptions -fexceptions -analyze 
-analyzer-checker=debug.ViewCFG -analyzer-config cfg-scopes=true 
-analyzer-config cfg-loopexit=true -analyzer-config unroll-loops=true 
/tmp/scopes-cfg-output.cpp
  -> CFG-scopes-destructors-loopexit.dot
  
  $ ./bin/clang -cc1 -fcxx-exceptions -fexceptions -analyze 
-analyzer-checker=debug.ViewCFG -analyzer-config cfg-scopes=true 
-analyzer-config cfg-loopexit=true -analyzer-config cfg-lifetime=true 
-analyzer-config cfg-implicit-dtors=false /tmp/scopes-cfg-output.cpp
  -> CFG-scopes-loopexit-lifetime.dot{F5792940}
  
  {F5792939}



> Not sure, i guess `LifetimeEnds` is mostly used in `clang-tidy` so it does 
> not necessarily need to work together with analyzer-specific elements (or 
> maybe it's so great that we should switch to using it), but it would still be 
> great if we had a single scope representation which would be rich enough to 
> satisfy all needs.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> Do we need to have one-to-one mapping between ScopeBegins and corresponding 
> ScopeEnds or is it OK to assume that ScopeEnd can terminate several nested 
> scopes?

It's fine if `ScopeEnds` terminates multiple scopes - as long as it is easy to 
find out what scopes are being terminated by looking at it. Because in the 
analyzer we need to put the scope on the stack when we enter it and pop it from 
the stack when we leave it, and those must match no matter what. So imagine 
that we look at the current `ScopeEnd` and at the stack of scopes we currently 
have. Once we have that, we should be able to figure out what scopes are 
ending, without using `ParentMap` or `CFGStmtMap` or re-visiting a large chunk 
of the AST recursively - ideally by a direct lookup.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hmm. @m.ostapenko @szepet @mgehre - I think it might be a good time to figure 
out if `ScopeBegin`/`ScopeEnd`, `LoopEntrance`/`LoopExit`, `LifetimeEnds`, 
`AutomaticObjectDtor` elements work nicely together. How should they be ordered 
with respect to each other? Is any of these scope representation a superset of 
another scope representation, or maybe fully covered by other two or three 
other scope representations? Would anybody be willing to produce some pictures 
(`-analyzer-checker debug.ViewCFG` and attach here) with current and/or 
intended behavior? Not sure, i guess `LifetimeEnds` is mostly used in 
`clang-tidy` so it does not necessarily need to work together with 
analyzer-specific elements (or maybe it's so great that we should switch to 
using it), but it would still be great if we had a single scope representation 
which would be rich enough to satisfy all needs.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 131091.
m.ostapenko added a comment.
Herald added a subscriber: llvm-commits.

Rebased and ping.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1171 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: 1
+// CHECK-NEXT:   2: return [B1.1];
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: CFGScopeEnd(a)
+// CHECK-NEXT:   7: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(c)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: CFGScopeEnd(c)
+// CHECK-NEXT:  10: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  11: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  12:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  13: A b;
+// CHECK-NEXT:  14: CFGScopeEnd(a)
+// CHECK-NEXT:  15: [B1.13].~A() (Implicit destructor)
+// CHECK-NEXT:  16: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: CFGScopeEnd(a)
+// CHECK-NEXT:   4: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   6: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: return;
+// CHECK-NEXT:   2: CFGScopeEnd(a)
+// CHECK-NEXT:   3: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B3]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   5: A b;
+// CHECK-NEXT:   6: UV
+// CHECK-NEXT:   7: [B3.6] (ImplicitCastExpr, LValueToRValue, _Bool)
+// CHECK-NEXT:   T: if [B3.7]
+// CHECK-NEXT:   Preds (1): B4
+// CHECK-NEXT:   Succs (2): B2 B1
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (2): B1 B2
+void test_return() {
+  A a;
+  A b;
+  if (UV) return;
+  A c;
+}
+
+// CHECK:  [B5 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B4
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeEnd(b)
+// CHECK-NEXT:   2: [B4.8].~A() (Implicit destructor)
+// CHECK-NEXT:   3: CFGScopeEnd(a)
+// CHECK-NEXT:   4: [B4.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (2): B2 B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(c)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A c;
+// CHECK-NEXT:   4: CFGScopeEnd(c)
+// CHECK-NEXT:   5: [B2.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B4
+// CHECK-NEXT: 

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 130183.
m.ostapenko retitled this revision from "Add scope information to CFG for 
If/While/For/Do/Compound/CXXRangeFor statements" to "Add scope information to 
CFG".
m.ostapenko added a comment.

Some code cleanup + updated test case.


https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1171 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: 1
+// CHECK-NEXT:   2: return [B1.1];
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: CFGScopeEnd(a)
+// CHECK-NEXT:   7: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(c)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: CFGScopeEnd(c)
+// CHECK-NEXT:  10: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  11: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  12:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  13: A b;
+// CHECK-NEXT:  14: CFGScopeEnd(a)
+// CHECK-NEXT:  15: [B1.13].~A() (Implicit destructor)
+// CHECK-NEXT:  16: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: CFGScopeEnd(a)
+// CHECK-NEXT:   4: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   6: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: return;
+// CHECK-NEXT:   2: CFGScopeEnd(a)
+// CHECK-NEXT:   3: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B3]
+// CHECK-NEXT:   1: CFGScopeBegin(a)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   5: A b;
+// CHECK-NEXT:   6: UV
+// CHECK-NEXT:   7: [B3.6] (ImplicitCastExpr, LValueToRValue, _Bool)
+// CHECK-NEXT:   T: if [B3.7]
+// CHECK-NEXT:   Preds (1): B4
+// CHECK-NEXT:   Succs (2): B2 B1
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (2): B1 B2
+void test_return() {
+  A a;
+  A b;
+  if (UV) return;
+  A c;
+}
+
+// CHECK:  [B5 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B4
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeEnd(b)
+// CHECK-NEXT:   2: [B4.8].~A() (Implicit destructor)
+// CHECK-NEXT:   3: CFGScopeEnd(a)
+// CHECK-NEXT:   4: [B4.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (2): B2 B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(c)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A c;
+// CHECK-NEXT:   4: CFGScopeEnd(c)
+// CHECK-NEXT:   5: [B2.3].~A() (Implicit destructor)
+// CHECK-NEXT:  

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 129985.
m.ostapenko added a comment.

Hi Devin,

now I'm very sorry for a such long delay. Now I have a bunch of time to proceed 
development of this patch (if scope contexts are still needed, of course).
Regarding to approach you suggested (reuse LocalScope infrastructure and use 
first VarDecl for ScopeBegin): it seems very reasonable for me, but perhaps I 
need more advisory here.
I've implemented a draft patch to make sure I've understood you correctly (this 
is not a final version, the test case is completely inadequate for now). While 
testing, I've discovered several questions that I would like to discuss:

1. Do we need to have one-to-one mapping between ScopeBegins and corresponding 
ScopeEnds or is it OK to assume that ScopeEnd can terminate several nested 
scopes?
2. In the following example:

  class A {
  public:
A() {}
~A() {}
operator int() const { return 1; }
  };
  
  bool UV;
  void test_for_implicit_scope() {
for (A a; A b = a; )
if (UV) continue;
A c;
}

  it seems that lifetime of **b** ends when we branch out from the loop body 
(if entered, of course), but it seems that in current implementation we don't 
generate an implicit destructor for **b** just before **continue**. Is this a 
bug, or perhaps I'm missing something?
  # The approach with first VarDecl works not so well with the following test 
case:

  void test_goto() {
A f;
  l0:
A d;
{ 
  A a;
  if (UV) goto l0;
  if (UV) goto l1;
  A b;
}
  l1:
A c;
  }

in this approach we'll don't emit a ScopeBegin for **d** because the first 
VarDecl is **f**. However IMHO we still need to add ScopeBegin for **d** in 
order to handle d's scope end occurring when we jumping to //l0// via **if (UV) 
goto l0;**. I think this can be solved by adding ScopeBegin for **d** when 
backpatching blocks late in **buildCFG** routine (when we know all targets of 
all gotos). Does this approach look reasonable for you?

Thanks.


https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,909 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B3 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2: 1
+// CHECK-NEXT:   3: return [B1.2];
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B2
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B1
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   7: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   10: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:   11: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:   12:  (CXXConstructExpr, class A)
+// CHECK:   13: A b;
+// CHECK:   14: CFGScopeEnd(CompoundStmt)
+// CHECK:   15: [B1.13].~A() (Implicit destructor)
+// CHECK:   16: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

Maxim, thanks for commandeering the patch. I apologize for the long delay 
reviewing!

I really like the approach of retrying without scopes enabled when we hit a 
construct we can't handle yet. This will make is possible to turn the feature 
on by default (and get it running on large amounts of code) while still 
developing it incrementally!

I'd like to suggest a change in the scope representation that will make it much 
easier to support all the corner cases and also re-use the existing logic for 
handling variable scopes. Right now the CFGScopeBegin and CFGScopeEnd elements 
use a statement (a loop or a CompoundStatement) to uniquely identify a scope. 
However, this is a problem because a `for` loop with an initializer may have 
two scopes that we really want to distinguish:

  for (int i = 0; i < 10; i++)
int j = i;

Here `i` and `j` really have different scopes, but with the current approach we 
won't be able tell them apart.

My suggestion is to instead use the first VarDecl declared in a scope to 
uniquely identify it. This means, for example, that CFGScopeBegin's constructor 
will take a VarDecl instead of a statement.

What's nice about this VarDecl approach is that the CFGBuilder already has a 
bunch of infrastructure to correctly track local scopes for variables (see the 
`LocalScope` class in CFG.cpp and the `addLocalScopeForStmt()` function.) 
Further, there are two functions, addLocalScopeAndDtors() and 
addAutomaticObjHandling() that are already called in all the places where a 
scope should end. This means you should only need to add logic to add an end of 
scope element in those two functions. What's more, future work to extend CFG 
construction to handle new C++ language features will also use these two 
functions -- so we will get support for those features for free. For an example 
of how to change those two functions, take a look at Matthias Gehre's commit in 
https://reviews.llvm.org/D15031. What you would need to do for CFGScopeEnd 
would be very similar to that patch. Using this approach should also make it 
possible to re-use the logic in that patch to eventually handle gotos.

To handle CFGScopeBegin, you would only need to change 
CFGBuilder::VisitDeclSubExpr() to detect when the variable being visited is the 
first variable declared in its LocalScope. If so, you would append the 
CFGScopeBegin element. What's nice about this is that you won't have to handle 
all the different kinds of loops individually.

What do you think? I'd be happy to have a Skype/Google hangouts talk about this 
if you want to have real-time discussion about it.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 112547.
m.ostapenko added a comment.

Ping^4


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1098 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2: 1
+// CHECK-NEXT:   3: return [B1.2];
+// CHECK-NEXT:   4: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: a
+// CHECK-NEXT:   5: [B1.4] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:   6: const A  = a;
+// CHECK-NEXT:   7: A() (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: [B1.7] (BindTemporary)
+// CHECK-NEXT:   9: [B1.8] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:  10: [B1.9]
+// CHECK-NEXT:  11: const A  = A();
+// CHECK-NEXT:  12: [B1.11].~A() (Implicit destructor)
+// CHECK-NEXT:  13: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  14: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_const_ref() {
+  A a;
+  const A& b = a;
+  const A& c = A();
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   7: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  10: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  11: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:  12:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  13: A b;
+// CHECK-NEXT:  14: [B1.13].~A() (Implicit destructor)
+// CHECK-NEXT:  15: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  16: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   6: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(IfStmt)
+// CHECK-NEXT:   2: return;
+// CHECK-NEXT:   3: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   5: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B3]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// 

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 111322.
m.ostapenko added a comment.

Ping^3


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1098 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2: 1
+// CHECK-NEXT:   3: return [B1.2];
+// CHECK-NEXT:   4: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: a
+// CHECK-NEXT:   5: [B1.4] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:   6: const A  = a;
+// CHECK-NEXT:   7: A() (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: [B1.7] (BindTemporary)
+// CHECK-NEXT:   9: [B1.8] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:  10: [B1.9]
+// CHECK-NEXT:  11: const A  = A();
+// CHECK-NEXT:  12: [B1.11].~A() (Implicit destructor)
+// CHECK-NEXT:  13: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  14: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_const_ref() {
+  A a;
+  const A& b = a;
+  const A& c = A();
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   7: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  10: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  11: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:  12:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  13: A b;
+// CHECK-NEXT:  14: [B1.13].~A() (Implicit destructor)
+// CHECK-NEXT:  15: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  16: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   6: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(IfStmt)
+// CHECK-NEXT:   2: return;
+// CHECK-NEXT:   3: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   5: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B3]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// 

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 110329.
m.ostapenko added a comment.

Rebased and ping.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1098 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2: 1
+// CHECK-NEXT:   3: return [B1.2];
+// CHECK-NEXT:   4: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: a
+// CHECK-NEXT:   5: [B1.4] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:   6: const A  = a;
+// CHECK-NEXT:   7: A() (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: [B1.7] (BindTemporary)
+// CHECK-NEXT:   9: [B1.8] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:  10: [B1.9]
+// CHECK-NEXT:  11: const A  = A();
+// CHECK-NEXT:  12: [B1.11].~A() (Implicit destructor)
+// CHECK-NEXT:  13: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  14: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_const_ref() {
+  A a;
+  const A& b = a;
+  const A& c = A();
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   7: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  10: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  11: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:  12:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  13: A b;
+// CHECK-NEXT:  14: [B1.13].~A() (Implicit destructor)
+// CHECK-NEXT:  15: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  16: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   6: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(IfStmt)
+// CHECK-NEXT:   2: return;
+// CHECK-NEXT:   3: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   5: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B3]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 108644.
m.ostapenko added a comment.

Updated some comments. Could someone take a look please?


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1099 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2: 1
+// CHECK-NEXT:   3: return [B1.2];
+// CHECK-NEXT:   4: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: a
+// CHECK-NEXT:   5: [B1.4] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:   6: const A  = a;
+// CHECK-NEXT:   7: A() (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: [B1.7] (BindTemporary)
+// CHECK-NEXT:   9: [B1.8] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:  10: [B1.9]
+// CHECK-NEXT:  11: const A  = A();
+// CHECK-NEXT:  12: [B1.11].~A() (Implicit destructor)
+// CHECK-NEXT:  13: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  14: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_const_ref() {
+  A a;
+  const A& b = a;
+  const A& c = A();
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   7: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  10: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  11:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  12: A b;
+// CHECK-NEXT:  13: [B1.12].~A() (Implicit destructor)
+// CHECK-NEXT:  14: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  15: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:  16: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   6: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(IfStmt)
+// CHECK-NEXT:   2: return;
+// CHECK-NEXT:   3: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   5: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B3]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 107894.
m.ostapenko added a comment.

Rebased and removed a bunch of stale changes. Also added a check for goto's: if 
we see GotoStmt and have cfg-scopes == true, make badCFG = true and retry 
without scopes enabled. This check will be removed once GotoStmt will become 
supported.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1099 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2: 1
+// CHECK-NEXT:   3: return [B1.2];
+// CHECK-NEXT:   4: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: a
+// CHECK-NEXT:   5: [B1.4] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:   6: const A  = a;
+// CHECK-NEXT:   7: A() (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: [B1.7] (BindTemporary)
+// CHECK-NEXT:   9: [B1.8] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:  10: [B1.9]
+// CHECK-NEXT:  11: const A  = A();
+// CHECK-NEXT:  12: [B1.11].~A() (Implicit destructor)
+// CHECK-NEXT:  13: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  14: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_const_ref() {
+  A a;
+  const A& b = a;
+  const A& c = A();
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   7: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  10: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  11:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  12: A b;
+// CHECK-NEXT:  13: [B1.12].~A() (Implicit destructor)
+// CHECK-NEXT:  14: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  15: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:  16: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   6: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(IfStmt)
+// CHECK-NEXT:   2: return;
+// CHECK-NEXT:   3: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   5: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds 

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

In https://reviews.llvm.org/D16403#808104, @NoQ wrote:

> I think the remaining switch-related code seems to be about C++17 switch 
> condition variables, i.e. `switch (int x = ...)`(?)


Yeah, exactly. I can remove it from this patch if it looks confusing.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I think the remaining switch-related code seems to be about C++17 switch 
condition variables, i.e. `switch (int x = ...)`(?)


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Peter Szecsi via Phabricator via cfe-commits]

szepet added a comment.

Hello Maxim!
Thanks for working on this!

> Ugh, yeah, SwitchStmt is tricky (thank you for pointing to Duff's device 
> example!). I've tried to resolve several issues with switch revealed by this 
> testcase, but didn't succeed for now :(. So, it was decided to remove 
> SwitchStmt support in this patch.

I think this is a great decision! It can build up incremental and the patch can 
be more easily reviewed. When do you plan to upload a patch wihtout casestmt 
support? (As far as I see there is switchstmt related code but maybe Im missing 
something.)


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 106408.
m.ostapenko retitled this revision from "Add scope information to CFG" to "Add 
scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements".
m.ostapenko added a comment.

Updating the diff. I've dropped SwitchStmt support, let's implement in 
separately as well as GotoStmt.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1030 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2: 1
+// CHECK-NEXT:   3: return [B1.2];
+// CHECK-NEXT:   4: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: a
+// CHECK-NEXT:   5: [B1.4] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:   6: const A  = a;
+// CHECK-NEXT:   7: A() (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: [B1.7] (BindTemporary)
+// CHECK-NEXT:   9: [B1.8] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:  10: [B1.9]
+// CHECK-NEXT:  11: const A  = A();
+// CHECK-NEXT:  12: [B1.11].~A() (Implicit destructor)
+// CHECK-NEXT:  13: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  14: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_const_ref() {
+  A a;
+  const A& b = a;
+  const A& c = A();
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   7: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:  10: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:  11:  (CXXConstructExpr, class A)
+// CHECK-NEXT:  12: A b;
+// CHECK-NEXT:  13: [B1.12].~A() (Implicit destructor)
+// CHECK-NEXT:  14: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:  15: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:  16: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B4 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B3
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   6: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeBegin(IfStmt)
+// CHECK-NEXT:   2: return;
+// CHECK-NEXT:   3: [B3.5].~A() (Implicit destructor)
+// CHECK-NEXT:   4: [B3.3].~A() (Implicit destructor)
+// CHECK-NEXT:   5: CFGScopeEnd(ReturnStmt)
+// 

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D16403#803518, @m.ostapenko wrote:

> There is one more thing I'd like to clarify: since e.g. BreakStmt can 
> terminate multiple scopes, do I need to create ScopeEnd marks for all of them 
> to make analyzer's work easier? Because right now only one "cumulative" block 
> is generated and I'm not sure that it's acceptable for analyzer.


The analyzer intends to maintain a stack of scopes. When the scope is entered 
during symbolic execution, it gets put on top of the stack, and when it is 
exited it is taken off the top of the stack. It's probably not important if we 
have one mark or multiple mark, but it's important to know what scopes, in what 
order, are getting entered or exited.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko added a comment.

Hi Artem, I'm sorry for a long delay (nasty corporate issues).

In https://reviews.llvm.org/D16403#789957, @NoQ wrote:

> Maxim, totally thanks for picking this up!
>
> Could you explain the idea behind `shouldDeferScopeEnd`, maybe in a code 
> comment before the function?
>
> In https://reviews.llvm.org/D16403#788926, @m.ostapenko wrote:
>
> > Current patch should support basic {If, While, For, Compound, Switch}Stmts 
> > as well as their interactions with {Break, Continue, Return}Stmts.
> >  GotoStmt and CXXForRangeStmt are not supported at this moment.
>
>
> `SwitchStmt` isn't much easier than `GotoStmt`; it doesn't jump backwards, 
> but it can still jump into multiple different scopes. Does your code handle 
> Duff's device (1)  (2) 
>  correctly? We should probably 
> add it as a test, or split out switch support into a separate patch together 
> with goto, if such test isn't yet supported.


Ugh, yeah, SwitchStmt is tricky (thank you for pointing to Duff's device 
example!). I've tried to resolve several issues with switch revealed by this 
testcase, but didn't succeed for now :(. So, it was decided to remove 
SwitchStmt support in this patch.
There is one more thing I'd like to clarify: since e.g. BreakStmt can terminate 
multiple scopes, do I need to create ScopeEnd marks for all of them to make 
analyzer's work easier? Because right now only one "cumulative" block is 
generated and I'm not sure that it's acceptable for analyzer.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Oh, I see now. I was wrongly thinking that these patches do the same thing and 
we're duplicating the work. Thank you very much for your explanation, Devin.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin added a comment.

> @dcoughlin As a reviewer of both patches - could you tell us what's the 
> difference between them? And how are we going to resolve this issue?

These two patches are emitting markers in the CFG for different things.

Here is how I would characterize the difference between the two patches.

- Despite its name, https://reviews.llvm.org/D15031, is really emitting markers 
for when the lifetime of a C++ object in an automatic variable ends.  For C++ 
objects with non-trivial destructors, this point is when the destructor is 
called. At this point the storage for the variable still exists, but what you 
can do with that storage is very restricted by the language because its 
contents have been destroyed. Note that even with the contents of the object 
have been destroyed, it is still meaningful to, say, take the address of the 
variable and construct a new object into it with a placement new. In other 
words, the same variable can have different objects, with different lifetimes, 
in it at different program points.

- In contrast, the purpose of this patch (https://reviews.llvm.org/D16403) is 
to add markers for when the storage duration for the variable begins and ends 
(this is, when the storage exists). Once the storage duration has ended, you 
can't placement new into the variables address, because another variable may 
already be at that address.

I don't think there is an "issue" to resolve here.  We should make sure the two 
patches play nicely with each other, though. In particular, we should make sure 
that the markers for when lifetime ends and when storage duration ends are 
ordered correctly.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added a comment.

> @dcoughlin As a reviewer of both patches - could you tell us what's the 
> difference between them? And how are we going to resolve this issue?

Unfortunately, @dcoughlin is on vacation this week; should be back next week.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Maxim, totally thanks for picking this up!

Could you explain the idea behind `shouldDeferScopeEnd`, maybe in a code 
comment before the function?

In https://reviews.llvm.org/D16403#788926, @m.ostapenko wrote:

> Current patch should support basic {If, While, For, Compound, Switch}Stmts as 
> well as their interactions with {Break, Continue, Return}Stmts.
>  GotoStmt and CXXForRangeStmt are not supported at this moment.


`SwitchStmt` isn't much easier than `GotoStmt`; it doesn't jump backwards, but 
it can still jump into multiple different scopes. Does your code handle Duff's 
device (1)  (2) 
 correctly? We should probably add 
it as a test, or split out switch support into a separate patch together with 
goto, if such test isn't yet supported.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hi Matthias,
I have posted a comment about review duplication (more than a year ago!) in 
your review but you haven't answered. So, all this time we were thinking that 
you do separate non-related work.
@dcoughlin As a reviewer of both patches - could you tell us what's the 
difference between them? And how are we going to resolve this issue?


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Please consider also https://reviews.llvm.org/D15031
It already handles all possible control flows. Instead of ScopeBegin and 
ScopeEnd,
it introduces LifetimeEnds elements. It's a more specific in that is also
correctly models the order of lifetime expiry of variables and the order
or destructor calls / lifetime ending.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403



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

[PATCH] D16403: Add scope information to CFG

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 103719.
m.ostapenko set the repository for this revision to rL LLVM.
m.ostapenko added a project: clang.
m.ostapenko added a comment.

So, updating the diff. This is still a very experimental version and any 
feedback would be greatly appreciated.
Current patch should support basic {If, While, For, Compound, Switch}Stmts as 
well as their interactions with {Break, Continue, Return}Stmts.
GotoStmt and CXXForRangeStmt are not supported at this moment.


Repository:
  rL LLVM

https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

Index: test/Analysis/scopes-cfg-output.cpp
===
--- /dev/null
+++ test/Analysis/scopes-cfg-output.cpp
@@ -0,0 +1,1468 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-scopes=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class A {
+public:
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  A() {}
+
+// CHECK:  [B1 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+  ~A() {}
+
+// CHECK:  [B3 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2: 1
+// CHECK-NEXT:   3: return [B1.2];
+// CHECK-NEXT:   Preds (1): B3
+// CHECK-NEXT:   Succs (1): B2
+// CHECK:  [B2]
+// CHECK-NEXT:   1: CFGScopeEnd(ReturnStmt)
+// CHECK-NEXT:   Preds (1): B1
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+  operator int() const { return 1; }
+};
+
+int getX();
+extern const bool UV;
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: a
+// CHECK-NEXT:   5: [B1.4] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:   6: const A  = a;
+// CHECK-NEXT:   7: A() (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: [B1.7] (BindTemporary)
+// CHECK-NEXT:   9: [B1.8] (ImplicitCastExpr, NoOp, const class A)
+// CHECK-NEXT:  10: [B1.9]
+// CHECK-NEXT:  11: const A  = A();
+// CHECK-NEXT:  12: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:  13: [B1.11].~A() (Implicit destructor)
+// CHECK-NEXT:  14: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+void test_const_ref() {
+  A a;
+  const A& b = a;
+  const A& c = A();
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A [2])
+// CHECK-NEXT:   3: A a[2];
+// CHECK-NEXT:   4:  (CXXConstructExpr, class A [0])
+// CHECK-NEXT:   5: A b[0];
+// CHECK-NEXT:   6: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   7: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_array() {
+  A a[2];
+  A b[0];
+}
+
+// CHECK:  [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:  [B1]
+// CHECK-NEXT:   1: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   3: A a;
+// CHECK-NEXT:   4: CFGScopeBegin(CompoundStmt)
+// CHECK-NEXT:   5:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   6: A c;
+// CHECK-NEXT:   7:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   8: A d;
+// CHECK-NEXT:   9: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   10: [B1.8].~A() (Implicit destructor)
+// CHECK-NEXT:   11: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:   12:  (CXXConstructExpr, class A)
+// CHECK:   13: A b;
+// CHECK:   14: CFGScopeEnd(CompoundStmt)
+// CHECK:   15: [B1.13].~A() (Implicit destructor)
+// CHECK:   16: [B1.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:  [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_scope() {
+  A a;
+  { A c;
+A d;
+  }
+  A b;
+}
+
+// CHECK:  [B5 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B4
+// CHECK:  [B1]
+// CHECK-NEXT:   1:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2: A c;
+// CHECK-NEXT:   3: CFGScopeEnd(CompoundStmt)
+// CHECK-NEXT:   4: [B1.2].~A() (Implicit destructor)
+// CHECK-NEXT:   5: [B4.5].~A() (Implicit destructor)
+// CHECK-NEXT:   6: [B4.3].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B4
+// CHECK-NEXT:   Succs (1): B0
+// CHECK: