[Analysis] Bug fix for exploded graph branching in evalCall for constructor

Summary:
Make exactly single NodeBuilder exists at any given time

Reviewers: NoQ, Szelethus, vsavchenko, xazax.hun

Reviewed By: NoQ

Subscribers: martong, cfe-commits
Tags: #clang

Differential Revision: https://reviews.llvm.org/D85796

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 38a680e..802bc93 100644 (file)
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -602,11 +602,11 @@ void ExprEngine::handleConstructor(const Expr *E,
                                             *Call, *this);
 
   ExplodedNodeSet DstEvaluated;
-  StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
 
   if (CE && CE->getConstructor()->isTrivial() &&
       CE->getConstructor()->isCopyOrMoveConstructor() &&
       !CallOpts.IsArrayCtorOrDtor) {
+    StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
     // FIXME: Handle other kinds of trivial constructors as well.
     for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
          I != E; ++I)
@@ -626,6 +626,8 @@ void ExprEngine::handleConstructor(const Expr *E,
   // in the CFG, would be called at the end of the full expression or
   // later (for life-time extended temporaries) -- but avoids infeasible
   // paths when no-return temporary destructors are used for assertions.
+  ExplodedNodeSet DstEvaluatedPostProcessed;
+  StmtNodeBuilder Bldr(DstEvaluated, DstEvaluatedPostProcessed, *currBldrCtx);
   const AnalysisDeclContext *ADC = LCtx->getAnalysisDeclContext();
   if (!ADC->getCFGBuildOptions().AddTemporaryDtors) {
     if (llvm::isa_and_nonnull<CXXTempObjectRegion>(TargetRegion) &&
@@ -655,7 +657,7 @@ void ExprEngine::handleConstructor(const Expr *E,
   }
 
   ExplodedNodeSet DstPostArgumentCleanup;
-  for (ExplodedNode *I : DstEvaluated)
+  for (ExplodedNode *I : DstEvaluatedPostProcessed)
     finishArgumentConstruction(DstPostArgumentCleanup, I, *Call);
 
   // If there were other constructors called for object-type arguments

diff --git a/clang/test/Analysis/smart-ptr-text-output.cpp b/clang/test/Analysis/smart-ptr-text-output.cpp
index 5280d00..1132a37 100644 (file)
--- a/clang/test/Analysis/smart-ptr-text-output.cpp
+++ b/clang/test/Analysis/smart-ptr-text-output.cpp
@@ -36,14 +36,15 @@ void derefAfterCtrWithNullVariable() {
 }
 
 void derefAfterRelease() {
-  std::unique_ptr<A> P(new A());
+  std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
+  // FIXME: should mark region as uninteresting after release, so above note will not be there
   P.release(); // expected-note {{Smart pointer 'P' is released and set to null}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
 
 void derefAfterReset() {
-  std::unique_ptr<A> P(new A());
+  std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
   P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
@@ -51,7 +52,7 @@ void derefAfterReset() {
 
 void derefAfterResetWithNull() {
   A *NullInnerPtr = nullptr; // expected-note {{'NullInnerPtr' initialized to a null pointer value}}
-  std::unique_ptr<A> P(new A());
+  std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
   P.reset(NullInnerPtr); // expected-note {{Smart pointer 'P' reset using a null value}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
@@ -67,7 +68,7 @@ void derefOnReleasedNullRawPtr() {
 }
 
 void derefOnSwappedNullPtr() {
-  std::unique_ptr<A> P(new A());
+  std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
   std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
   P.swap(PNull); // expected-note {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
   PNull->foo(); // No warning.
@@ -77,13 +78,11 @@ void derefOnSwappedNullPtr() {
 
 // FIXME: Fix this test when "std::swap" is modeled seperately.
 void derefOnStdSwappedNullPtr() {
-  std::unique_ptr<A> P;
+  std::unique_ptr<A> P; // expected-note {{Default constructed smart pointer 'P' is null}}
   std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
   std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:978 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
   // expected-note@-1 {{Calling 'swap<A>'}}
   // expected-note@-2 {{Returning from 'swap<A>'}}
-  PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
-  // expected-note@-1{{Dereference of null smart pointer 'PNull'}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }

diff --git a/clang/test/Analysis/smart-ptr.cpp b/clang/test/Analysis/smart-ptr.cpp
index f72a918..bcf1e56 100644 (file)
--- a/clang/test/Analysis/smart-ptr.cpp
+++ b/clang/test/Analysis/smart-ptr.cpp
@@ -41,6 +41,7 @@ A *return_null() {
 
 void derefAfterValidCtr() {
   std::unique_ptr<A> P(new A());
+  clang_analyzer_numTimesReached(); // expected-warning {{1}}
   P->foo(); // No warning.
 }
 
@@ -50,17 +51,20 @@ void derefOfUnknown(std::unique_ptr<A> P) {
 
 void derefAfterDefaultCtr() {
   std::unique_ptr<A> P;
+  clang_analyzer_numTimesReached(); // expected-warning {{1}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterCtrWithNull() {
   std::unique_ptr<A> P(nullptr);
+  clang_analyzer_numTimesReached(); // expected-warning {{1}}
   *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterCtrWithNullVariable() {
   A *InnerPtr = nullptr;
   std::unique_ptr<A> P(InnerPtr);
+  clang_analyzer_numTimesReached(); // expected-warning {{1}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
@@ -87,6 +91,7 @@ void derefAfterResetWithNull() {
 void derefAfterResetWithNonNull() {
   std::unique_ptr<A> P;
   P.reset(new A());
+  clang_analyzer_numTimesReached(); // expected-warning {{1}}
   P->foo(); // No warning.
 }
 
@@ -116,37 +121,40 @@ void pass_smart_ptr_by_const_rvalue_ref(const std::unique_ptr<A> &&a);
 void pass_smart_ptr_by_ptr(std::unique_ptr<A> *a);
 void pass_smart_ptr_by_const_ptr(const std::unique_ptr<A> *a);
 
-void regioninvalidationTest() {
-  {
-    std::unique_ptr<A> P;
-    pass_smart_ptr_by_ref(P);
-    P->foo(); // no-warning
-  }
-  {
-    std::unique_ptr<A> P;
-    pass_smart_ptr_by_const_ref(P);
-    P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
-  }
-  {
-    std::unique_ptr<A> P;
-    pass_smart_ptr_by_rvalue_ref(std::move(P));
-    P->foo(); // no-warning
-  }
-  {
-    std::unique_ptr<A> P;
-    pass_smart_ptr_by_const_rvalue_ref(std::move(P));
-    P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
-  }
-  {
-    std::unique_ptr<A> P;
-    pass_smart_ptr_by_ptr(&P);
-    P->foo();
-  }
-  {
-    std::unique_ptr<A> P;
-    pass_smart_ptr_by_const_ptr(&P);
-    P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
-  }
+void regioninvalidationWithPassByRef() {
+  std::unique_ptr<A> P;
+  pass_smart_ptr_by_ref(P);
+  P->foo(); // no-warning
+}
+
+void regioninvalidationWithPassByCostRef() {
+  std::unique_ptr<A> P;
+  pass_smart_ptr_by_const_ref(P);
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationWithPassByRValueRef() {
+  std::unique_ptr<A> P;
+  pass_smart_ptr_by_rvalue_ref(std::move(P));
+  P->foo(); // no-warning
+}
+
+void regioninvalidationWithPassByConstRValueRef() {
+  std::unique_ptr<A> P;
+  pass_smart_ptr_by_const_rvalue_ref(std::move(P));
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationWithPassByPtr() {
+  std::unique_ptr<A> P;
+  pass_smart_ptr_by_ptr(&P);
+  P->foo();
+}
+
+void regioninvalidationWithPassByConstPtr() {
+  std::unique_ptr<A> P;
+  pass_smart_ptr_by_const_ptr(&P);
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 struct StructWithSmartPtr {
@@ -160,37 +168,40 @@ void pass_struct_with_smart_ptr_by_const_rvalue_ref(const StructWithSmartPtr &&a
 void pass_struct_with_smart_ptr_by_ptr(StructWithSmartPtr *a);
 void pass_struct_with_smart_ptr_by_const_ptr(const StructWithSmartPtr *a);
 
-void regioninvalidationTestWithinStruct() {
-  {
-    StructWithSmartPtr S;
-    pass_struct_with_smart_ptr_by_ref(S);
-    S.P->foo(); // no-warning
-  }
-  {
-    StructWithSmartPtr S;
-    pass_struct_with_smart_ptr_by_const_ref(S);
-    S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
-  }
-  {
-    StructWithSmartPtr S;
-    pass_struct_with_smart_ptr_by_rvalue_ref(std::move(S));
-    S.P->foo(); // no-warning
-  }
-  {
-    StructWithSmartPtr S;
-    pass_struct_with_smart_ptr_by_const_rvalue_ref(std::move(S));
-    S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
-  }
-  {
-    StructWithSmartPtr S;
-    pass_struct_with_smart_ptr_by_ptr(&S);
-    S.P->foo();
-  }
-  {
-    StructWithSmartPtr S;
-    pass_struct_with_smart_ptr_by_const_ptr(&S);
-    S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
-  }
+void regioninvalidationWithinStructPassByRef() {
+  StructWithSmartPtr S;
+  pass_struct_with_smart_ptr_by_ref(S);
+  S.P->foo(); // no-warning
+}
+
+void regioninvalidationWithinStructPassByConstRef() {
+  StructWithSmartPtr S;
+  pass_struct_with_smart_ptr_by_const_ref(S);
+  S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationWithinStructPassByRValueRef() {
+  StructWithSmartPtr S;
+  pass_struct_with_smart_ptr_by_rvalue_ref(std::move(S));
+  S.P->foo(); // no-warning
+}
+
+void regioninvalidationWithinStructPassByConstRValueRef() {
+  StructWithSmartPtr S;
+  pass_struct_with_smart_ptr_by_const_rvalue_ref(std::move(S));
+  S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationWithinStructPassByPtr() {
+  StructWithSmartPtr S;
+  pass_struct_with_smart_ptr_by_ptr(&S);
+  S.P->foo(); // no-warning
+}
+
+void regioninvalidationWithinStructPassByConstPtr() {
+  StructWithSmartPtr S;
+  pass_struct_with_smart_ptr_by_const_ptr(&S);
+  S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterAssignment() {
@@ -217,14 +228,20 @@ void derefOnSwappedNullPtr() {
   (*P).foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
-void derefOnStdSwappedNullPtr() {
+void derefOnFirstStdSwappedNullPtr() {
   std::unique_ptr<A> P;
   std::unique_ptr<A> PNull;
   std::swap(P, PNull);
-  PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
+void derefOnSecondStdSwappedNullPtr() {
+  std::unique_ptr<A> P;
+  std::unique_ptr<A> PNull;
+  std::swap(P, PNull);
+  PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
 void derefOnSwappedValidPtr() {
   std::unique_ptr<A> P(new A());
   std::unique_ptr<A> PValid(new A());