[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-08 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC324668: [CFG] Add extra context to C++ constructor statement 
elements. (authored by dergachev, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42672?vs=133139=133500#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42672

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.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/cfg-rich-constructors.cpp
  test/Analysis/cfg.cpp

Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -231,6 +231,9 @@
   /// \sa IncludeLoopExitInCFG
   Optional IncludeLoopExitInCFG;
 
+  /// \sa IncludeRichConstructorsInCFG
+  Optional IncludeRichConstructorsInCFG;
+
   /// \sa mayInlineCXXStandardLibrary
   Optional InlineCXXStandardLibrary;
   
@@ -444,6 +447,13 @@
   /// the values "true" and "false".
   bool includeLoopExitInCFG();
 
+  /// Returns whether or not construction site information should be included
+  /// in the CFG C++ constructor elements.
+  ///
+  /// This is controlled by the 'cfg-rich-constructors' config options,
+  /// which accepts the values "true" and "false".
+  bool includeRichConstructorsInCFG();
+
   /// Returns whether or not C++ standard library functions may be considered
   /// for inlining.
   ///
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -194,7 +194,7 @@
   void processCFGElement(const CFGElement E, ExplodedNode *Pred,
  unsigned StmtIdx, NodeBuilderContext *Ctx) override;
 
-  void ProcessStmt(const CFGStmt S, ExplodedNode *Pred);
+  void ProcessStmt(const Stmt *S, ExplodedNode *Pred);
 
   void ProcessLoopExit(const Stmt* S, ExplodedNode *Pred);
 
Index: include/clang/Analysis/AnalysisDeclContext.h
===
--- include/clang/Analysis/AnalysisDeclContext.h
+++ include/clang/Analysis/AnalysisDeclContext.h
@@ -439,6 +439,7 @@
  bool synthesizeBodies = false,
  bool addStaticInitBranches = false,
  bool addCXXNewAllocator = true,
+ bool addRichCXXConstructors = true,
  CodeInjector *injector = nullptr);
 
   AnalysisDeclContext *getContext(const Decl *D);
Index: include/clang/Analysis/CFG.h
===
--- include/clang/Analysis/CFG.h
+++ include/clang/Analysis/CFG.h
@@ -15,7 +15,7 @@
 #ifndef LLVM_CLANG_ANALYSIS_CFG_H
 #define LLVM_CLANG_ANALYSIS_CFG_H
 
-#include "clang/AST/Stmt.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/Analysis/Support/BumpVector.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/DenseMap.h"
@@ -55,11 +55,15 @@
 public:
   enum Kind {
 // main kind
-Statement,
 Initializer,
 NewAllocator,
 LifetimeEnds,
 LoopExit,
+// stmt kind
+Statement,
+Constructor,
+STMT_BEGIN = Statement,
+STMT_END = Constructor,
 // dtor kind
 AutomaticObjectDtor,
 DeleteDtor,
@@ -117,27 +121,85 @@
 
 class CFGStmt : public CFGElement {
 public:
-  CFGStmt(Stmt *S) : CFGElement(Statement, S) {}
+  explicit CFGStmt(Stmt *S, Kind K = Statement) : CFGElement(K, S) {
+assert(isKind(*this));
+  }
 
   const Stmt *getStmt() const {
 return static_cast(Data1.getPointer());
   }
 
 private:
   friend class CFGElement;
 
+  static bool isKind(const CFGElement ) {
+return E.getKind() >= STMT_BEGIN && E.getKind() <= STMT_END;
+  }
+
+protected:
   CFGStmt() = default;
+};
+
+// This is bulky data for CFGConstructor which would not fit into the
+// CFGElement's room (pair of pointers). Contains the information
+// necessary to express what memory is being initialized by
+// the construction.
+class ConstructionContext {
+  // The construction site - the statement that triggered the construction
+  // for one of its parts. For instance, stack variable declaration statement
+  // triggers construction of itself or its elements if it's an array,
+  // new-expression triggers construction of the newly allocated object(s).
+  Stmt *Trigger = nullptr;
+
+public:
+

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-08 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL324668: [CFG] Add extra context to C++ constructor statement 
elements. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42672?vs=133139=133499#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42672

Files:
  cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
  cfe/trunk/include/clang/Analysis/CFG.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
  cfe/trunk/lib/Analysis/CFG.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  cfe/trunk/test/Analysis/analyzer-config.c
  cfe/trunk/test/Analysis/analyzer-config.cpp
  cfe/trunk/test/Analysis/cfg-rich-constructors.cpp
  cfe/trunk/test/Analysis/cfg.cpp

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -231,6 +231,9 @@
   /// \sa IncludeLoopExitInCFG
   Optional IncludeLoopExitInCFG;
 
+  /// \sa IncludeRichConstructorsInCFG
+  Optional IncludeRichConstructorsInCFG;
+
   /// \sa mayInlineCXXStandardLibrary
   Optional InlineCXXStandardLibrary;
   
@@ -444,6 +447,13 @@
   /// the values "true" and "false".
   bool includeLoopExitInCFG();
 
+  /// Returns whether or not construction site information should be included
+  /// in the CFG C++ constructor elements.
+  ///
+  /// This is controlled by the 'cfg-rich-constructors' config options,
+  /// which accepts the values "true" and "false".
+  bool includeRichConstructorsInCFG();
+
   /// Returns whether or not C++ standard library functions may be considered
   /// for inlining.
   ///
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -194,7 +194,7 @@
   void processCFGElement(const CFGElement E, ExplodedNode *Pred,
  unsigned StmtIdx, NodeBuilderContext *Ctx) override;
 
-  void ProcessStmt(const CFGStmt S, ExplodedNode *Pred);
+  void ProcessStmt(const Stmt *S, ExplodedNode *Pred);
 
   void ProcessLoopExit(const Stmt* S, ExplodedNode *Pred);
 
Index: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
===
--- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
+++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h
@@ -439,6 +439,7 @@
  bool synthesizeBodies = false,
  bool addStaticInitBranches = false,
  bool addCXXNewAllocator = true,
+ bool addRichCXXConstructors = true,
  CodeInjector *injector = nullptr);
 
   AnalysisDeclContext *getContext(const Decl *D);
Index: cfe/trunk/include/clang/Analysis/CFG.h
===
--- cfe/trunk/include/clang/Analysis/CFG.h
+++ cfe/trunk/include/clang/Analysis/CFG.h
@@ -15,7 +15,7 @@
 #ifndef LLVM_CLANG_ANALYSIS_CFG_H
 #define LLVM_CLANG_ANALYSIS_CFG_H
 
-#include "clang/AST/Stmt.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/Analysis/Support/BumpVector.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/DenseMap.h"
@@ -55,11 +55,15 @@
 public:
   enum Kind {
 // main kind
-Statement,
 Initializer,
 NewAllocator,
 LifetimeEnds,
 LoopExit,
+// stmt kind
+Statement,
+Constructor,
+STMT_BEGIN = Statement,
+STMT_END = Constructor,
 // dtor kind
 AutomaticObjectDtor,
 DeleteDtor,
@@ -117,27 +121,85 @@
 
 class CFGStmt : public CFGElement {
 public:
-  CFGStmt(Stmt *S) : CFGElement(Statement, S) {}
+  explicit CFGStmt(Stmt *S, Kind K = Statement) : CFGElement(K, S) {
+assert(isKind(*this));
+  }
 
   const Stmt *getStmt() const {
 return static_cast(Data1.getPointer());
   }
 
 private:
   friend class CFGElement;
 
+  static bool isKind(const CFGElement ) {
+return E.getKind() >= STMT_BEGIN && E.getKind() <= STMT_END;
+  }
+
+protected:
   CFGStmt() = default;
+};
+
+// This is bulky data for CFGConstructor which would not fit into the
+// CFGElement's room (pair of pointers). Contains the information
+// necessary to express what memory is being initialized by
+// the construction.
+class ConstructionContext {
+  // The construction site - the statement that

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-08 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added a comment.

All right, so this change is indeed safe, i.e. no crashes or changes in 
analyzer behavior so far on my large codebase run. Will commit now.


https://reviews.llvm.org/D42672



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

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-06 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ updated this revision to Diff 133139.
NoQ added a comment.

Make CFGConstructor a sub-class of CFGStmt. I failed to notice any compile time 
regressions for now; if any, they must be super bottlenecked on CFG 
construction or analysis-based warnings. After poking @rsmith in the chat a 
little bit, we decided to go the easy way and make constructor a statement.


https://reviews.llvm.org/D42672

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.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/cfg-rich-constructors.cpp
  test/Analysis/cfg.cpp

Index: test/Analysis/cfg.cpp
===
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -1,5 +1,16 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
-// RUN: FileCheck --input-file=%t %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
+
+// This file tests how we construct two different flavors of the Clang CFG -
+// the CFG used by the Sema analysis-based warnings and the CFG used by the
+// static analyzer. The difference in the behavior is checked via FileCheck
+// prefixes (WARNINGS and ANALYZER respectively). When introducing new analyzer
+// flags, no new run lines should be added - just these flags would go to the
+// respective line depending on where is it turned on and where is it turned
+// off. Feel free to add tests that test only one of the CFG flavors if you're
+// not sure how the other flavor is supposed to work in your case.
 
 // CHECK-LABEL: void checkWrap(int i)
 // CHECK: ENTRY
@@ -116,7 +127,8 @@
 // CHECK-NEXT:   Succs (1): B1
 // CHECK: [B1]
 // CHECK-NEXT:   1:  CFGNewAllocator(A *)
-// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// WARNINGS-NEXT:   2:  (CXXConstructExpr, class A)
+// ANALYZER-NEXT:   2:  (CXXConstructExpr, [B1.3], class A)
 // CHECK-NEXT:   3: new A([B1.2])
 // CHECK-NEXT:   4: A *a = new A();
 // CHECK-NEXT:   5: a
@@ -138,7 +150,8 @@
 // CHECK: [B1]
 // CHECK-NEXT:   1: 5
 // CHECK-NEXT:   2: CFGNewAllocator(A *)
-// CHECK-NEXT:   3:  (CXXConstructExpr, class A [5])
+// WARNINGS-NEXT:   3:  (CXXConstructExpr, class A [5])
+// ANALYZER-NEXT:   3:  (CXXConstructExpr, [B1.4], class A [5])
 // CHECK-NEXT:   4: new A {{\[\[}}B1.1]]
 // CHECK-NEXT:   5: A *a = new A [5];
 // CHECK-NEXT:   6: a
@@ -331,7 +344,8 @@
 // CHECK-NEXT:  3: [B1.2] (ImplicitCastExpr, ArrayToPointerDecay, int *)
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// WARNINGS-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// ANALYZER-NEXT:  6:  (CXXConstructExpr, [B1.7], class MyClass)
 // CHECK-NEXT:  7: new ([B1.4]) MyClass([B1.6])
 // CHECK-NEXT:  8: MyClass *obj = new (buffer) MyClass();
 // CHECK-NEXT:  Preds (1): B2
@@ -363,7 +377,8 @@
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: 5
 // CHECK-NEXT:  6: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// WARNINGS-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// ANALYZER-NEXT:  7:  (CXXConstructExpr, [B1.8], class MyClass [5])
 // CHECK-NEXT:  8: new ([B1.4]) MyClass {{\[\[}}B1.5]]
 // CHECK-NEXT:  9: MyClass *obj = new (buffer) MyClass [5];
 // CHECK-NEXT:  Preds (1): B2
Index: test/Analysis/cfg-rich-constructors.cpp
===
--- /dev/null
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class C {
+public:
+  C();
+  C(C *);
+};
+
+typedef __typeof(sizeof(int)) size_t;
+void *operator new(size_t size, void *placement);
+
+namespace operator_new {
+
+// CHECK: void operatorNewWithConstructor()
+// CHECK:  1: CFGNewAllocator(C *)
+// CHECK-NEXT: 2:

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-05 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added inline comments.



Comment at: include/clang/Analysis/CFG.h:153
+
+  ConstructionContext() = default;
+  ConstructionContext(CXXConstructExpr *Constructor, Stmt *Trigger)

xazax.hun wrote:
> Maybe I am getting this wrong, but I think in this case the members will be 
> default initialized and will get indeterminate values.
> See: http://en.cppreference.com/w/cpp/language/default_initialization
> > Default initialization is performed in three situations:
> > ..
> > 3) when a base class or a non-static data member is not mentioned in a 
> > constructor initializer list and that constructor is called.
> > 
> > 
> > The effects of default initialization are:
> > 
> > if T is a non-POD (until C++11) class type ...
> > 
> > if T is an array type, every element of the array is default-initialized;
> > 
> > otherwise, nothing is done: the objects with automatic storage duration 
> > (and their subobjects) are initialized to indeterminate values. 
Ew, right. I should be more careful (no idea how it works).



Comment at: lib/Analysis/CFG.cpp:4402
+  stmt = SE->getStmt();
+else if (auto CE = BI->getAs())
+  stmt = CE->getConstructor();

xazax.hun wrote:
> So this is one of the places where subclassing would help? Could you measure 
> the compile time regression after making `CFGStmt`'s `isKind` more complex?
Yeah, any user of the new mode would be surprised that constructors are not 
statements, which is indeed annoying. But i wanted to play super safe.

I'm not seeing any compile time regressions yet after making `isKind` more 
complex - i.e. ran "`clang sqlite3.c`" release-without-asserts 100 times before 
and 100 times after, mean compilation time difference around 0.04% (in the bad 
direction, i.e. after-time is 0.04% worse than before-time), p-value (Welch's 
t-test) around 0.7. I guess i could probably run llvm compilation before and 
after for more coverage, but that wouldn't be statistically reliable.

Do we have any more reliable benchmarks?


https://reviews.llvm.org/D42672



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

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-05 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ updated this revision to Diff 132908.
NoQ marked an inline comment as done.
NoQ added a comment.

Fix uninitialized variables.


https://reviews.llvm.org/D42672

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/Analysis/LiveVariables.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/cfg-rich-constructors.cpp
  test/Analysis/cfg.cpp

Index: test/Analysis/cfg.cpp
===
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -1,5 +1,16 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
-// RUN: FileCheck --input-file=%t %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
+
+// This file tests how we construct two different flavors of the Clang CFG -
+// the CFG used by the Sema analysis-based warnings and the CFG used by the
+// static analyzer. The difference in the behavior is checked via FileCheck
+// prefixes (WARNINGS and ANALYZER respectively). When introducing new analyzer
+// flags, no new run lines should be added - just these flags would go to the
+// respective line depending on where is it turned on and where is it turned
+// off. Feel free to add tests that test only one of the CFG flavors if you're
+// not sure how the other flavor is supposed to work in your case.
 
 // CHECK-LABEL: void checkWrap(int i)
 // CHECK: ENTRY
@@ -116,7 +127,8 @@
 // CHECK-NEXT:   Succs (1): B1
 // CHECK: [B1]
 // CHECK-NEXT:   1:  CFGNewAllocator(A *)
-// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// WARNINGS-NEXT:   2:  (CXXConstructExpr, class A)
+// ANALYZER-NEXT:   2:  (CXXConstructExpr, [B1.3], class A)
 // CHECK-NEXT:   3: new A([B1.2])
 // CHECK-NEXT:   4: A *a = new A();
 // CHECK-NEXT:   5: a
@@ -138,7 +150,8 @@
 // CHECK: [B1]
 // CHECK-NEXT:   1: 5
 // CHECK-NEXT:   2: CFGNewAllocator(A *)
-// CHECK-NEXT:   3:  (CXXConstructExpr, class A [5])
+// WARNINGS-NEXT:   3:  (CXXConstructExpr, class A [5])
+// ANALYZER-NEXT:   3:  (CXXConstructExpr, [B1.4], class A [5])
 // CHECK-NEXT:   4: new A {{\[\[}}B1.1]]
 // CHECK-NEXT:   5: A *a = new A [5];
 // CHECK-NEXT:   6: a
@@ -331,7 +344,8 @@
 // CHECK-NEXT:  3: [B1.2] (ImplicitCastExpr, ArrayToPointerDecay, int *)
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// WARNINGS-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// ANALYZER-NEXT:  6:  (CXXConstructExpr, [B1.7], class MyClass)
 // CHECK-NEXT:  7: new ([B1.4]) MyClass([B1.6])
 // CHECK-NEXT:  8: MyClass *obj = new (buffer) MyClass();
 // CHECK-NEXT:  Preds (1): B2
@@ -363,7 +377,8 @@
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: 5
 // CHECK-NEXT:  6: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// WARNINGS-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// ANALYZER-NEXT:  7:  (CXXConstructExpr, [B1.8], class MyClass [5])
 // CHECK-NEXT:  8: new ([B1.4]) MyClass {{\[\[}}B1.5]]
 // CHECK-NEXT:  9: MyClass *obj = new (buffer) MyClass [5];
 // CHECK-NEXT:  Preds (1): B2
Index: test/Analysis/cfg-rich-constructors.cpp
===
--- /dev/null
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class C {
+public:
+  C();
+  C(C *);
+};
+
+typedef __typeof(sizeof(int)) size_t;
+void *operator new(size_t size, void *placement);
+
+namespace operator_new {
+
+// CHECK: void operatorNewWithConstructor()
+// CHECK:  1: CFGNewAllocator(C *)
+// CHECK-NEXT: 2:  (CXXConstructExpr, [B1.3], class C)
+// CHECK-NEXT: 3: new C([B1.2])
+void operatorNewWithConstructor() {
+  new C();
+}
+
+//

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-04 Thread Gábor Horváth via Phabricator via cfe-commits 
xazax.hun added inline comments.



Comment at: include/clang/Analysis/CFG.h:153
+
+  ConstructionContext() = default;
+  ConstructionContext(CXXConstructExpr *Constructor, Stmt *Trigger)

Maybe I am getting this wrong, but I think in this case the members will be 
default initialized and will get indeterminate values.
See: http://en.cppreference.com/w/cpp/language/default_initialization
> Default initialization is performed in three situations:
> ..
> 3) when a base class or a non-static data member is not mentioned in a 
> constructor initializer list and that constructor is called.
> 
> 
> The effects of default initialization are:
> 
> if T is a non-POD (until C++11) class type ...
> 
> if T is an array type, every element of the array is default-initialized;
> 
> otherwise, nothing is done: the objects with automatic storage duration (and 
> their subobjects) are initialized to indeterminate values. 



Comment at: lib/Analysis/CFG.cpp:4402
+  stmt = SE->getStmt();
+else if (auto CE = BI->getAs())
+  stmt = CE->getConstructor();

So this is one of the places where subclassing would help? Could you measure 
the compile time regression after making `CFGStmt`'s `isKind` more complex?


https://reviews.llvm.org/D42672



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

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-02 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ updated this revision to Diff 132685.
NoQ added a comment.

Rename the method, add comments.


https://reviews.llvm.org/D42672

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/Analysis/LiveVariables.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/cfg-rich-constructors.cpp
  test/Analysis/cfg.cpp

Index: test/Analysis/cfg.cpp
===
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -1,5 +1,16 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
-// RUN: FileCheck --input-file=%t %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
+
+// This file tests how we construct two different flavors of the Clang CFG -
+// the CFG used by the Sema analysis-based warnings and the CFG used by the
+// static analyzer. The difference in the behavior is checked via FileCheck
+// prefixes (WARNINGS and ANALYZER respectively). When introducing new analyzer
+// flags, no new run lines should be added - just these flags would go to the
+// respective line depending on where is it turned on and where is it turned
+// off. Feel free to add tests that test only one of the CFG flavors if you're
+// not sure how the other flavor is supposed to work in your case.
 
 // CHECK-LABEL: void checkWrap(int i)
 // CHECK: ENTRY
@@ -116,7 +127,8 @@
 // CHECK-NEXT:   Succs (1): B1
 // CHECK: [B1]
 // CHECK-NEXT:   1:  CFGNewAllocator(A *)
-// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// WARNINGS-NEXT:   2:  (CXXConstructExpr, class A)
+// ANALYZER-NEXT:   2:  (CXXConstructExpr, [B1.3], class A)
 // CHECK-NEXT:   3: new A([B1.2])
 // CHECK-NEXT:   4: A *a = new A();
 // CHECK-NEXT:   5: a
@@ -138,7 +150,8 @@
 // CHECK: [B1]
 // CHECK-NEXT:   1: 5
 // CHECK-NEXT:   2: CFGNewAllocator(A *)
-// CHECK-NEXT:   3:  (CXXConstructExpr, class A [5])
+// WARNINGS-NEXT:   3:  (CXXConstructExpr, class A [5])
+// ANALYZER-NEXT:   3:  (CXXConstructExpr, [B1.4], class A [5])
 // CHECK-NEXT:   4: new A {{\[\[}}B1.1]]
 // CHECK-NEXT:   5: A *a = new A [5];
 // CHECK-NEXT:   6: a
@@ -331,7 +344,8 @@
 // CHECK-NEXT:  3: [B1.2] (ImplicitCastExpr, ArrayToPointerDecay, int *)
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// WARNINGS-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// ANALYZER-NEXT:  6:  (CXXConstructExpr, [B1.7], class MyClass)
 // CHECK-NEXT:  7: new ([B1.4]) MyClass([B1.6])
 // CHECK-NEXT:  8: MyClass *obj = new (buffer) MyClass();
 // CHECK-NEXT:  Preds (1): B2
@@ -363,7 +377,8 @@
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: 5
 // CHECK-NEXT:  6: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// WARNINGS-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// ANALYZER-NEXT:  7:  (CXXConstructExpr, [B1.8], class MyClass [5])
 // CHECK-NEXT:  8: new ([B1.4]) MyClass {{\[\[}}B1.5]]
 // CHECK-NEXT:  9: MyClass *obj = new (buffer) MyClass [5];
 // CHECK-NEXT:  Preds (1): B2
Index: test/Analysis/cfg-rich-constructors.cpp
===
--- /dev/null
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class C {
+public:
+  C();
+  C(C *);
+};
+
+typedef __typeof(sizeof(int)) size_t;
+void *operator new(size_t size, void *placement);
+
+namespace operator_new {
+
+// CHECK: void operatorNewWithConstructor()
+// CHECK:  1: CFGNewAllocator(C *)
+// CHECK-NEXT: 2:  (CXXConstructExpr, [B1.3], class C)
+// CHECK-NEXT: 3: new C([B1.2])
+void operatorNewWithConstructor() {
+  new C();
+}
+
+// CHECK: void

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-02 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added inline comments.



Comment at: lib/Analysis/CFG.cpp:653
+  // triggered by the given trigger.
+  void VisitForConstructionContext(Stmt *Trigger, Stmt *Child);
+

dcoughlin wrote:
> I find the name of this method very confusing. It sounds like you are 
> visiting an AST node called a 'ForConstructionContext', but that is not what 
> this function does. It is also not clear from either the name or the comment 
> what the method is supposed to do. Can you be more precise?
> 
> You can fix this in a follow-up commit if you like.
Yeah i guess i just followed the `VisitForTemporaryDtors` pattern^^ Will fix.


https://reviews.llvm.org/D42672



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

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

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

This looks good to me, although as I noted inline I think both the name and the 
comment for VisitForConstructionContext() are confusing. If you can be more 
precise I think it would help future maintainers.




Comment at: lib/Analysis/CFG.cpp:653
+  // triggered by the given trigger.
+  void VisitForConstructionContext(Stmt *Trigger, Stmt *Child);
+

I find the name of this method very confusing. It sounds like you are visiting 
an AST node called a 'ForConstructionContext', but that is not what this 
function does. It is also not clear from either the name or the comment what 
the method is supposed to do. Can you be more precise?

You can fix this in a follow-up commit if you like.


https://reviews.llvm.org/D42672



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

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-02 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added inline comments.



Comment at: lib/Analysis/CFG.cpp:3899
+  if (auto *CE = const_cast(NE->getConstructExpr()))
+CurrentConstructionContext = {/*Constructor=*/CE, /*Trigger=*/NE};
+

NoQ wrote:
> dcoughlin wrote:
> > Is it possible that there is already a CurrentConstructionContext? Will 
> > this overwrite it?
> > 
> > Can you write an assertion to make sure that this doesn't happen?
> > 
> > Generally I would expect a context to have a well-defined start and end. I 
> > think we should be explicit about where we expect it to start and end so we 
> > can find violations of our expectations.
> Yes, for now it'd be overwritten every time we meet a new constructor (this 
> wasn't the case in the first version of this revision). However, the 
> additional check (which should be eventually replaced with an assertion, but 
> is for now needed) that we're picking the right context in `CXXConstructExpr` 
> visit, together with only visiting every constructor once during CFG 
> construction during normal visits (because weird stuff like 
> `CXXDefaultArgExpr` isn't yet supported), guarantees that we're not doing 
> anything wrong.
> 
> I should have cleaned this up, but i don't want to invest attention into that 
> because subsequent patches will clearly make things way more complex than 
> that, whenever we start dealing with a multitude of construction contexts or 
> with partially-constructed contexts.
> 
> So for now it's a trivial kinda-safe solution.
> 
> I should document that though, for sure.
Fixed now - added the `VisitForConstructionContext()` wrapper which contains 
the assertion that checks that we're not overwriting any existing context.

The context is being cleaned up after it's consumed in `appendConstructor()`.


https://reviews.llvm.org/D42672



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

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-02 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ updated this revision to Diff 132637.
NoQ added a comment.

Whoops! Actually use the new function with the assertion.

Also disable creating construction contexts when the flag is off.


https://reviews.llvm.org/D42672

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/Analysis/LiveVariables.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/cfg-rich-constructors.cpp
  test/Analysis/cfg.cpp

Index: test/Analysis/cfg.cpp
===
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -1,5 +1,16 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
-// RUN: FileCheck --input-file=%t %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
+
+// This file tests how we construct two different flavors of the Clang CFG -
+// the CFG used by the Sema analysis-based warnings and the CFG used by the
+// static analyzer. The difference in the behavior is checked via FileCheck
+// prefixes (WARNINGS and ANALYZER respectively). When introducing new analyzer
+// flags, no new run lines should be added - just these flags would go to the
+// respective line depending on where is it turned on and where is it turned
+// off. Feel free to add tests that test only one of the CFG flavors if you're
+// not sure how the other flavor is supposed to work in your case.
 
 // CHECK-LABEL: void checkWrap(int i)
 // CHECK: ENTRY
@@ -116,7 +127,8 @@
 // CHECK-NEXT:   Succs (1): B1
 // CHECK: [B1]
 // CHECK-NEXT:   1:  CFGNewAllocator(A *)
-// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// WARNINGS-NEXT:   2:  (CXXConstructExpr, class A)
+// ANALYZER-NEXT:   2:  (CXXConstructExpr, [B1.3], class A)
 // CHECK-NEXT:   3: new A([B1.2])
 // CHECK-NEXT:   4: A *a = new A();
 // CHECK-NEXT:   5: a
@@ -138,7 +150,8 @@
 // CHECK: [B1]
 // CHECK-NEXT:   1: 5
 // CHECK-NEXT:   2: CFGNewAllocator(A *)
-// CHECK-NEXT:   3:  (CXXConstructExpr, class A [5])
+// WARNINGS-NEXT:   3:  (CXXConstructExpr, class A [5])
+// ANALYZER-NEXT:   3:  (CXXConstructExpr, [B1.4], class A [5])
 // CHECK-NEXT:   4: new A {{\[\[}}B1.1]]
 // CHECK-NEXT:   5: A *a = new A [5];
 // CHECK-NEXT:   6: a
@@ -331,7 +344,8 @@
 // CHECK-NEXT:  3: [B1.2] (ImplicitCastExpr, ArrayToPointerDecay, int *)
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// WARNINGS-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// ANALYZER-NEXT:  6:  (CXXConstructExpr, [B1.7], class MyClass)
 // CHECK-NEXT:  7: new ([B1.4]) MyClass([B1.6])
 // CHECK-NEXT:  8: MyClass *obj = new (buffer) MyClass();
 // CHECK-NEXT:  Preds (1): B2
@@ -363,7 +377,8 @@
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: 5
 // CHECK-NEXT:  6: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// WARNINGS-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// ANALYZER-NEXT:  7:  (CXXConstructExpr, [B1.8], class MyClass [5])
 // CHECK-NEXT:  8: new ([B1.4]) MyClass {{\[\[}}B1.5]]
 // CHECK-NEXT:  9: MyClass *obj = new (buffer) MyClass [5];
 // CHECK-NEXT:  Preds (1): B2
Index: test/Analysis/cfg-rich-constructors.cpp
===
--- /dev/null
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class C {
+public:
+  C();
+  C(C *);
+};
+
+typedef __typeof(sizeof(int)) size_t;
+void *operator new(size_t size, void *placement);
+
+namespace operator_new {
+
+// CHECK: void operatorNewWithConstructor()
+// CHECK:  1: CFGNewAllocator(C *)
+// CHECK-NEXT: 2:  (CXXConstructExpr, [B1.3], class C)
+// CHECK-NEXT: 3: new C([B1.2])

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-01 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ updated this revision to Diff 132532.
NoQ marked an inline comment as done.
NoQ added a comment.

Cleanup construction contexts after they have been consumed. Add assertions to 
verify that our understanding of the context's lifespan is reasonable.

I will run the new mode on a large codebase before committing, to see if these 
assertions ever fail or if i'm breaking anything else.

Prettify ConstructionContext into a class with getters, write explicit 
constructors as pointed out in other patches.


https://reviews.llvm.org/D42672

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/Analysis/LiveVariables.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/cfg-rich-constructors.cpp
  test/Analysis/cfg.cpp

Index: test/Analysis/cfg.cpp
===
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -1,5 +1,16 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
-// RUN: FileCheck --input-file=%t %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
+
+// This file tests how we construct two different flavors of the Clang CFG -
+// the CFG used by the Sema analysis-based warnings and the CFG used by the
+// static analyzer. The difference in the behavior is checked via FileCheck
+// prefixes (WARNINGS and ANALYZER respectively). When introducing new analyzer
+// flags, no new run lines should be added - just these flags would go to the
+// respective line depending on where is it turned on and where is it turned
+// off. Feel free to add tests that test only one of the CFG flavors if you're
+// not sure how the other flavor is supposed to work in your case.
 
 // CHECK-LABEL: void checkWrap(int i)
 // CHECK: ENTRY
@@ -116,7 +127,8 @@
 // CHECK-NEXT:   Succs (1): B1
 // CHECK: [B1]
 // CHECK-NEXT:   1:  CFGNewAllocator(A *)
-// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// WARNINGS-NEXT:   2:  (CXXConstructExpr, class A)
+// ANALYZER-NEXT:   2:  (CXXConstructExpr, [B1.3], class A)
 // CHECK-NEXT:   3: new A([B1.2])
 // CHECK-NEXT:   4: A *a = new A();
 // CHECK-NEXT:   5: a
@@ -138,7 +150,8 @@
 // CHECK: [B1]
 // CHECK-NEXT:   1: 5
 // CHECK-NEXT:   2: CFGNewAllocator(A *)
-// CHECK-NEXT:   3:  (CXXConstructExpr, class A [5])
+// WARNINGS-NEXT:   3:  (CXXConstructExpr, class A [5])
+// ANALYZER-NEXT:   3:  (CXXConstructExpr, [B1.4], class A [5])
 // CHECK-NEXT:   4: new A {{\[\[}}B1.1]]
 // CHECK-NEXT:   5: A *a = new A [5];
 // CHECK-NEXT:   6: a
@@ -331,7 +344,8 @@
 // CHECK-NEXT:  3: [B1.2] (ImplicitCastExpr, ArrayToPointerDecay, int *)
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// WARNINGS-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// ANALYZER-NEXT:  6:  (CXXConstructExpr, [B1.7], class MyClass)
 // CHECK-NEXT:  7: new ([B1.4]) MyClass([B1.6])
 // CHECK-NEXT:  8: MyClass *obj = new (buffer) MyClass();
 // CHECK-NEXT:  Preds (1): B2
@@ -363,7 +377,8 @@
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: 5
 // CHECK-NEXT:  6: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// WARNINGS-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// ANALYZER-NEXT:  7:  (CXXConstructExpr, [B1.8], class MyClass [5])
 // CHECK-NEXT:  8: new ([B1.4]) MyClass {{\[\[}}B1.5]]
 // CHECK-NEXT:  9: MyClass *obj = new (buffer) MyClass [5];
 // CHECK-NEXT:  Preds (1): B2
Index: test/Analysis/cfg-rich-constructors.cpp
===
--- /dev/null
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class C {
+public:
+  C();
+

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-01 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ updated this revision to Diff 132520.
NoQ added a comment.

Add more targeted tests.


https://reviews.llvm.org/D42672

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/Analysis/LiveVariables.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/cfg-rich-constructors.cpp
  test/Analysis/cfg.cpp

Index: test/Analysis/cfg.cpp
===
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -1,5 +1,16 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
-// RUN: FileCheck --input-file=%t %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
+
+// This file tests how we construct two different flavors of the Clang CFG -
+// the CFG used by the Sema analysis-based warnings and the CFG used by the
+// static analyzer. The difference in the behavior is checked via FileCheck
+// prefixes (WARNINGS and ANALYZER respectively). When introducing new analyzer
+// flags, no new run lines should be added - just these flags would go to the
+// respective line depending on where is it turned on and where is it turned
+// off. Feel free to add tests that test only one of the CFG flavors if you're
+// not sure how the other flavor is supposed to work in your case.
 
 // CHECK-LABEL: void checkWrap(int i)
 // CHECK: ENTRY
@@ -116,7 +127,8 @@
 // CHECK-NEXT:   Succs (1): B1
 // CHECK: [B1]
 // CHECK-NEXT:   1:  CFGNewAllocator(A *)
-// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// WARNINGS-NEXT:   2:  (CXXConstructExpr, class A)
+// ANALYZER-NEXT:   2:  (CXXConstructExpr, [B1.3], class A)
 // CHECK-NEXT:   3: new A([B1.2])
 // CHECK-NEXT:   4: A *a = new A();
 // CHECK-NEXT:   5: a
@@ -138,7 +150,8 @@
 // CHECK: [B1]
 // CHECK-NEXT:   1: 5
 // CHECK-NEXT:   2: CFGNewAllocator(A *)
-// CHECK-NEXT:   3:  (CXXConstructExpr, class A [5])
+// WARNINGS-NEXT:   3:  (CXXConstructExpr, class A [5])
+// ANALYZER-NEXT:   3:  (CXXConstructExpr, [B1.4], class A [5])
 // CHECK-NEXT:   4: new A {{\[\[}}B1.1]]
 // CHECK-NEXT:   5: A *a = new A [5];
 // CHECK-NEXT:   6: a
@@ -331,7 +344,8 @@
 // CHECK-NEXT:  3: [B1.2] (ImplicitCastExpr, ArrayToPointerDecay, int *)
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// WARNINGS-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// ANALYZER-NEXT:  6:  (CXXConstructExpr, [B1.7], class MyClass)
 // CHECK-NEXT:  7: new ([B1.4]) MyClass([B1.6])
 // CHECK-NEXT:  8: MyClass *obj = new (buffer) MyClass();
 // CHECK-NEXT:  Preds (1): B2
@@ -363,7 +377,8 @@
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: 5
 // CHECK-NEXT:  6: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// WARNINGS-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// ANALYZER-NEXT:  7:  (CXXConstructExpr, [B1.8], class MyClass [5])
 // CHECK-NEXT:  8: new ([B1.4]) MyClass {{\[\[}}B1.5]]
 // CHECK-NEXT:  9: MyClass *obj = new (buffer) MyClass [5];
 // CHECK-NEXT:  Preds (1): B2
Index: test/Analysis/cfg-rich-constructors.cpp
===
--- /dev/null
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
+// RUN: FileCheck --input-file=%t %s
+
+class C {
+public:
+  C();
+  C(C *);
+};
+
+typedef __typeof(sizeof(int)) size_t;
+void *operator new(size_t size, void *placement);
+
+namespace operator_new {
+
+// CHECK: void operatorNewWithConstructor()
+// CHECK:  1: CFGNewAllocator(C *)
+// CHECK-NEXT: 2:  (CXXConstructExpr, [B1.3], class C)
+// CHECK-NEXT: 3: new C([B1.2])
+void operatorNewWithConstructor() {
+  new C();
+}
+
+// CHECK: void

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-01 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added inline comments.



Comment at: lib/Analysis/CFG.cpp:3899
+  if (auto *CE = const_cast(NE->getConstructExpr()))
+CurrentConstructionContext = {/*Constructor=*/CE, /*Trigger=*/NE};
+

dcoughlin wrote:
> Is it possible that there is already a CurrentConstructionContext? Will this 
> overwrite it?
> 
> Can you write an assertion to make sure that this doesn't happen?
> 
> Generally I would expect a context to have a well-defined start and end. I 
> think we should be explicit about where we expect it to start and end so we 
> can find violations of our expectations.
Yes, for now it'd be overwritten every time we meet a new constructor (this 
wasn't the case in the first version of this revision). However, the additional 
check (which should be eventually replaced with an assertion, but is for now 
needed) that we're picking the right context in `CXXConstructExpr` visit, 
together with only visiting every constructor once during CFG construction 
during normal visits (because weird stuff like `CXXDefaultArgExpr` isn't yet 
supported), guarantees that we're not doing anything wrong.

I should have cleaned this up, but i don't want to invest attention into that 
because subsequent patches will clearly make things way more complex than that, 
whenever we start dealing with a multitude of construction contexts or with 
partially-constructed contexts.

So for now it's a trivial kinda-safe solution.

I should document that though, for sure.



Comment at: lib/Analysis/CFG.cpp:4388
+case Stmt::WhileStmtClass: {
+  const VarDecl *var = cast(stmt)->getConditionVariable();
+  if (var)

dcoughlin wrote:
> Please, let's try to avoid changes that are unrelated to the functionality 
> being added.
I decreased indent of this whole huge for() loop, so the blame is already 
screwed (but phabricator carefully hides that), so i think this is a valid part 
of the patch.



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:632
+
+void ExprEngine::ProcessConstructor(const CFGConstructor C,
+ExplodedNode *Pred) {

dcoughlin wrote:
> You don't seem to be using the constructor context for anything other than 
> getting the CXXConstructExpr. Will a later patch require having a 
> CFGConstructor here?
> 
> This seems like unnecessarily duplicated code. Can we just change ProcessStmt 
> to take a `Stmt *` rather than a CFGStmt and use it for all statements 
> including constructors?
> 
Hmm, well, yeah, i guess, there isn't much value in passing the `CFGElement` 
around as long as we can find the current `CFGElement` any time we want in 
`ExprEngine->currBldrCtx`.

But if not for that, it was the whole point to have the construction context 
there. Will fix.


https://reviews.llvm.org/D42672



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

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-02-01 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ updated this revision to Diff 132476.
NoQ marked 5 inline comments as done.
NoQ added a comment.

Address comments. Most importantly, separate out sema and analyzer CFG tests.


https://reviews.llvm.org/D42672

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

Index: test/Analysis/cfg.cpp
===
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -1,5 +1,12 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
-// RUN: FileCheck --input-file=%t %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=false %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 -analyzer-config cfg-rich-constructors=true %s > %t 2>&1
+// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
+
+// This file tests how we construct two different flavors of the Clang CFG -
+// the CFG used by the Sema analysis-based warnings and the CFG used by the
+// static analyzer. The difference in the behavior is checked via FileCheck
+// prefixes (WARNINGS and ANALYZER respectively).
 
 // CHECK-LABEL: void checkWrap(int i)
 // CHECK: ENTRY
@@ -116,7 +123,8 @@
 // CHECK-NEXT:   Succs (1): B1
 // CHECK: [B1]
 // CHECK-NEXT:   1:  CFGNewAllocator(A *)
-// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// WARNINGS-NEXT:   2:  (CXXConstructExpr, class A)
+// ANALYZER-NEXT:   2:  (CXXConstructExpr, [B1.3], class A)
 // CHECK-NEXT:   3: new A([B1.2])
 // CHECK-NEXT:   4: A *a = new A();
 // CHECK-NEXT:   5: a
@@ -138,7 +146,8 @@
 // CHECK: [B1]
 // CHECK-NEXT:   1: 5
 // CHECK-NEXT:   2: CFGNewAllocator(A *)
-// CHECK-NEXT:   3:  (CXXConstructExpr, class A [5])
+// WARNINGS-NEXT:   3:  (CXXConstructExpr, class A [5])
+// ANALYZER-NEXT:   3:  (CXXConstructExpr, [B1.4], class A [5])
 // CHECK-NEXT:   4: new A {{\[\[}}B1.1]]
 // CHECK-NEXT:   5: A *a = new A [5];
 // CHECK-NEXT:   6: a
@@ -331,7 +340,8 @@
 // CHECK-NEXT:  3: [B1.2] (ImplicitCastExpr, ArrayToPointerDecay, int *)
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// WARNINGS-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// ANALYZER-NEXT:  6:  (CXXConstructExpr, [B1.7], class MyClass)
 // CHECK-NEXT:  7: new ([B1.4]) MyClass([B1.6])
 // CHECK-NEXT:  8: MyClass *obj = new (buffer) MyClass();
 // CHECK-NEXT:  Preds (1): B2
@@ -363,7 +373,8 @@
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: 5
 // CHECK-NEXT:  6: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// WARNINGS-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// ANALYZER-NEXT:  7:  (CXXConstructExpr, [B1.8], class MyClass [5])
 // CHECK-NEXT:  8: new ([B1.4]) MyClass {{\[\[}}B1.5]]
 // CHECK-NEXT:  9: MyClass *obj = new (buffer) MyClass [5];
 // CHECK-NEXT:  Preds (1): B2
Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -26,6 +26,7 @@
 // CHECK-NEXT: cfg-implicit-dtors = true
 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
+// CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-temporary-dtors = false
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
@@ -42,4 +43,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 24
+// CHECK-NEXT: num-entries = 25
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -15,6 +15,7 @@
 // CHECK-NEXT: cfg-implicit-dtors = true
 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
+// CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-temporary-dtors = false
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT:

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-01-31 Thread Devin Coughlin via Phabricator via cfe-commits 
dcoughlin added a comment.

I think it will be great to have a more explicit representation of construction 
in the CFG! Comments in line.




Comment at: include/clang/Analysis/CFG.h:143
+  CXXConstructExpr *Constructor;
+  Stmt *Trigger;
+

I think it would be good to add a comment saying what Trigger is.



Comment at: include/clang/Analysis/CFG.h:155
+/// constructor expression.
+class CFGConstructor : public CFGElement {
+public:

Can you add a comment saying that this is only used by the analyzer's CFG?



Comment at: lib/Analysis/CFG.cpp:475
 
+  ConstructionContext CurrentConstructionContext = {nullptr, nullptr};
+

A comment here would be helpful to future maintainers. For example, what does 
it mean for a construction context to be "Current"?



Comment at: lib/Analysis/CFG.cpp:3899
+  if (auto *CE = const_cast(NE->getConstructExpr()))
+CurrentConstructionContext = {/*Constructor=*/CE, /*Trigger=*/NE};
+

Is it possible that there is already a CurrentConstructionContext? Will this 
overwrite it?

Can you write an assertion to make sure that this doesn't happen?

Generally I would expect a context to have a well-defined start and end. I 
think we should be explicit about where we expect it to start and end so we can 
find violations of our expectations.



Comment at: lib/Analysis/CFG.cpp:4388
+case Stmt::WhileStmtClass: {
+  const VarDecl *var = cast(stmt)->getConditionVariable();
+  if (var)

Please, let's try to avoid changes that are unrelated to the functionality 
being added.



Comment at: lib/StaticAnalyzer/Core/AnalysisManager.cpp:32
 /*addCXXNewAllocator=*/true,
+/*addRichCXXConstructors=*/true,
 injector),

I think this really needs to be a flag in AnalyzerOptions so you can keep the 
existing CFG tests for the behavior that other clients of the CFG rely on and 
add new tests with this flag explicitly set.



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:632
+
+void ExprEngine::ProcessConstructor(const CFGConstructor C,
+ExplodedNode *Pred) {

You don't seem to be using the constructor context for anything other than 
getting the CXXConstructExpr. Will a later patch require having a 
CFGConstructor here?

This seems like unnecessarily duplicated code. Can we just change ProcessStmt 
to take a `Stmt *` rather than a CFGStmt and use it for all statements 
including constructors?



https://reviews.llvm.org/D42672



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

[PATCH] D42672: [CFG] [analyzer] Heavier CFGConstructor elements.

2018-01-30 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ updated this revision to Diff 132053.
NoQ added a comment.

Remove the stack of contexts for now. We're not using it anywhere yet, and i'm 
not sure if it'd be necessary.


https://reviews.llvm.org/D42672

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/Analysis/LiveVariables.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  test/Analysis/cfg.cpp

Index: test/Analysis/cfg.cpp
===
--- test/Analysis/cfg.cpp
+++ test/Analysis/cfg.cpp
@@ -116,7 +116,7 @@
 // CHECK-NEXT:   Succs (1): B1
 // CHECK: [B1]
 // CHECK-NEXT:   1:  CFGNewAllocator(A *)
-// CHECK-NEXT:   2:  (CXXConstructExpr, class A)
+// CHECK-NEXT:   2:  (CXXConstructExpr, [B1.3], class A)
 // CHECK-NEXT:   3: new A([B1.2])
 // CHECK-NEXT:   4: A *a = new A();
 // CHECK-NEXT:   5: a
@@ -138,7 +138,7 @@
 // CHECK: [B1]
 // CHECK-NEXT:   1: 5
 // CHECK-NEXT:   2: CFGNewAllocator(A *)
-// CHECK-NEXT:   3:  (CXXConstructExpr, class A [5])
+// CHECK-NEXT:   3:  (CXXConstructExpr, [B1.4], class A [5])
 // CHECK-NEXT:   4: new A {{\[\[}}B1.1]]
 // CHECK-NEXT:   5: A *a = new A [5];
 // CHECK-NEXT:   6: a
@@ -331,7 +331,7 @@
 // CHECK-NEXT:  3: [B1.2] (ImplicitCastExpr, ArrayToPointerDecay, int *)
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  6:  (CXXConstructExpr, class MyClass)
+// CHECK-NEXT:  6:  (CXXConstructExpr, [B1.7], class MyClass)
 // CHECK-NEXT:  7: new ([B1.4]) MyClass([B1.6])
 // CHECK-NEXT:  8: MyClass *obj = new (buffer) MyClass();
 // CHECK-NEXT:  Preds (1): B2
@@ -363,7 +363,7 @@
 // CHECK-NEXT:  4: [B1.3] (ImplicitCastExpr, BitCast, void *)
 // CHECK-NEXT:  5: 5
 // CHECK-NEXT:  6: CFGNewAllocator(MyClass *)
-// CHECK-NEXT:  7:  (CXXConstructExpr, class MyClass [5])
+// CHECK-NEXT:  7:  (CXXConstructExpr, [B1.8], class MyClass [5])
 // CHECK-NEXT:  8: new ([B1.4]) MyClass {{\[\[}}B1.5]]
 // CHECK-NEXT:  9: MyClass *obj = new (buffer) MyClass [5];
 // CHECK-NEXT:  Preds (1): B2
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -553,6 +553,9 @@
   case CFGElement::Statement:
 return PathDiagnosticLocation(Source.castAs().getStmt(),
   SM, CallerCtx);
+  case CFGElement::Constructor:
+return PathDiagnosticLocation(
+Source.castAs().getConstructor(), SM, CallerCtx);
   case CFGElement::Initializer: {
 const CFGInitializer  = Source.castAs();
 return PathDiagnosticLocation(Init.getInitializer()->getInit(),
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -209,7 +209,10 @@
   // See if we're constructing an existing region by looking at the next
   // element in the CFG.
   const CFGBlock *B = CurrBldrCtx.getBlock();
-  assert(isa(((*B)[currStmtIdx]).castAs().getStmt()));
+  // TODO: Retrieve construction target from CFGConstructor directly.
+  assert(
+  (*B)[currStmtIdx].getAs() ||
+  isa(((*B)[currStmtIdx]).castAs().getStmt()));
   unsigned int NextStmtIdx = currStmtIdx + 1;
   if (NextStmtIdx >= B->size())
 return None;
@@ -250,10 +253,14 @@
 Previous = (*B)[PreviousStmtIdx];
   }
 
-  if (Optional PrevStmtElem = Previous.getAs()) {
+  if (auto PrevStmtElem = Previous.getAs()) {
 if (auto *CtorExpr = dyn_cast(PrevStmtElem->getStmt())) {
   return CtorExpr;
 }
+  } else if (auto PrevCtorElem = Previous.getAs()) {
+if (auto *CtorExpr = PrevCtorElem->getConstructor()) {
+  return CtorExpr;
+}
   }
 
   return nullptr;
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -454,10 +454,13 @@
 
   switch (E.getKind()) {
 case CFGElement::Statement:
-  ProcessStmt(const_cast(E.castAs().getStmt()), Pred);
+  ProcessStmt(E.castAs(), Pred);
+  return;
+case CFGElement::Constructor:
+  ProcessConstructor(E.castAs(), Pred);
   return;
 case CFGElement::Initializer:
-  ProcessInitializer(E.castAs().getInitializer(), Pred);
+  ProcessInitializer(E.castAs(), Pred);
   return;
 case CFGElement::NewAllocator:
   ProcessNewAllocator(E.castAs().getAllocatorExpr(),
@@ -479,7 +482,7 @@
 }
 
 static bool shouldRemoveDeadBindings(AnalysisManager ,
-