[analyzer] Handle zeroing CXXConstructExprs.

Certain expressions can cause a constructor invocation to zero-initialize
its object even if the constructor itself does no initialization. The
analyzer now handles that before evaluating the call to the constructor,
using the same "default binding" mechanism that calloc() uses, rather
than simply ignoring the zero-initialization flag.

As a bonus, trivial default constructors are now no longer inlined; they
are instead processed explicitly by ExprEngine. This has a (positive)
effect on the generated path edges: they no longer stop at a default
constructor call unless there's a user-provided implementation.

<rdar://problem/14212563>

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@184511 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index ed90dc589181effc7ed2433908d53d690cf97710..96ea9f53395392450d9c73a2b14412062c47a4e3 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -176,6 +176,7 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
       }
 
       // FIXME: This will eventually need to handle new-expressions as well.
+      // Don't forget to update the pre-constructor initialization code below.
     }
 
     // If we couldn't find an existing region to construct into, assume we're
@@ -215,22 +216,60 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
 
   ExplodedNodeSet DstPreVisit;
   getCheckerManager().runCheckersForPreStmt(DstPreVisit, Pred, CE, *this);
+
+  bool IsArray = isa<ElementRegion>(Target);
+  ExplodedNodeSet PreInitialized;
+  {
+    StmtNodeBuilder Bldr(DstPreVisit, PreInitialized, *currBldrCtx);
+    if (CE->requiresZeroInitialization()) {
+      // Type of the zero doesn't matter.
+      SVal ZeroVal = svalBuilder.makeZeroVal(getContext().CharTy);
+
+      for (ExplodedNodeSet::iterator I = DstPreVisit.begin(),
+                                     E = DstPreVisit.end();
+           I != E; ++I) {
+        ProgramStateRef State = (*I)->getState();
+        // FIXME: Once we properly handle constructors in new-expressions, we'll
+        // need to invalidate the region before setting a default value, to make
+        // sure there aren't any lingering bindings around. This probably needs
+        // to happen regardless of whether or not the object is zero-initialized
+        // to handle random fields of a placement-initialized object picking up
+        // old bindings. We might only want to do it when we need to, though.
+        // FIXME: This isn't actually correct for arrays -- we need to zero-
+        // initialize the entire array, not just the first element -- but our
+        // handling of arrays everywhere else is weak as well, so this shouldn't
+        // actually make things worse.
+        State = State->bindDefault(loc::MemRegionVal(Target), ZeroVal);
+        Bldr.generateNode(CE, *I, State, /*tag=*/0, ProgramPoint::PreStmtKind);
+      }
+    }
+  }
+
   ExplodedNodeSet DstPreCall;
-  getCheckerManager().runCheckersForPreCall(DstPreCall, DstPreVisit,
+  getCheckerManager().runCheckersForPreCall(DstPreCall, PreInitialized,
                                             *Call, *this);
 
   ExplodedNodeSet DstEvaluated;
   StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
 
-  bool IsArray = isa<ElementRegion>(Target);
-  if (CE->getConstructor()->isTrivial() &&
-      CE->getConstructor()->isCopyOrMoveConstructor() &&
-      !IsArray) {
-    // FIXME: Handle other kinds of trivial constructors as well.
-    for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
-         I != E; ++I)
-      performTrivialCopy(Bldr, *I, *Call);
-
+  if (CE->getConstructor()->isTrivial() && !IsArray) {
+    if (CE->getConstructor()->isCopyOrMoveConstructor()) {
+      for (ExplodedNodeSet::iterator I = DstPreCall.begin(),
+                                     E = DstPreCall.end();
+           I != E; ++I)
+        performTrivialCopy(Bldr, *I, *Call);
+    } else {
+      assert(CE->getConstructor()->isDefaultConstructor());
+
+      // We still have to bind the return value.
+      for (ExplodedNodeSet::iterator I = DstPreCall.begin(),
+                                     E = DstPreCall.end();
+           I != E; ++I) {
+        ProgramStateRef State = (*I)->getState();
+        State = bindReturnValue(*Call, LCtx, State);
+        Bldr.generateNode(CE, *I, State);
+      }
+    }
   } else {
     for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
          I != E; ++I)

diff --git a/test/Analysis/ctor-inlining.mm b/test/Analysis/ctor.mm
similarity index 84%
rename from test/Analysis/ctor-inlining.mm
rename to test/Analysis/ctor.mm
index 8cdb005968c3141bd7d89e46284b433727ba0290..37334fe896e75b9cc6d5760d30398ca253f1cadb 100644 (file)
--- a/test/Analysis/ctor-inlining.mm
+++ b/test/Analysis/ctor.mm
@@ -500,3 +500,78 @@ namespace ArrayMembers {
     clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{UNKNOWN}}
   }
 };
+
+namespace ZeroInitialization {
+  struct raw_pair {
+    int p1;
+    int p2;
+  };
+
+  void testVarDecl() {
+    raw_pair p{};
+    clang_analyzer_eval(p.p1 == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(p.p2 == 0); // expected-warning{{TRUE}}
+  }
+
+  void testTemporary() {
+    clang_analyzer_eval(raw_pair().p1 == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(raw_pair().p2 == 0); // expected-warning{{TRUE}}
+  }
+
+  void testArray() {
+    raw_pair p[2] = {};
+    clang_analyzer_eval(p[0].p1 == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(p[0].p2 == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(p[1].p1 == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(p[1].p2 == 0); // expected-warning{{TRUE}}
+  }
+
+  void testNew() {
+    // FIXME: Pending proper implementation of constructors for 'new'.
+    raw_pair *pp = new raw_pair();
+    clang_analyzer_eval(pp->p1 == 0); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(pp->p2 == 0); // expected-warning{{UNKNOWN}}
+  }
+
+  void testArrayNew() {
+    // FIXME: Pending proper implementation of constructors for 'new[]'.
+    raw_pair *p = new raw_pair[2]();
+    clang_analyzer_eval(p[0].p1 == 0); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(p[0].p2 == 0); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(p[1].p1 == 0); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(p[1].p2 == 0); // expected-warning{{UNKNOWN}}
+  }
+
+  struct initializing_pair {
+  public:
+    int x;
+    raw_pair y;
+    initializing_pair() : x(), y() {}
+  };
+  
+  void testFieldInitializers() {
+    initializing_pair p;
+    clang_analyzer_eval(p.x == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(p.y.p1 == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(p.y.p2 == 0); // expected-warning{{TRUE}}
+  }
+
+  struct subclass : public raw_pair {
+    subclass() = default;
+  };
+
+  void testSubclass() {
+    subclass p;
+    clang_analyzer_eval(p.p1 == 0); // expected-warning{{garbage}}
+  }
+
+  struct initializing_subclass : public raw_pair {
+    initializing_subclass() : raw_pair() {}
+  };
+
+  void testInitializingSubclass() {
+    initializing_subclass p;
+    clang_analyzer_eval(p.p1 == 0); // expected-warning{{TRUE}}
+    clang_analyzer_eval(p.p2 == 0); // expected-warning{{TRUE}}
+  }
+}

diff --git a/test/Analysis/inlining/path-notes.cpp b/test/Analysis/inlining/path-notes.cpp
index 810c150e4c313ef0f987f408418cd2d7afad2f05..29637f2c8bbb12d0936f868311958026b8df19a1 100644 (file)
--- a/test/Analysis/inlining/path-notes.cpp
+++ b/test/Analysis/inlining/path-notes.cpp
@@ -300,40 +300,6 @@ int callGenerateNoteOnDefaultArgument(int o) {
 // CHECK-NEXT:         <key>end</key>
 // CHECK-NEXT:          <array>
 // CHECK-NEXT:           <dict>
-// CHECK-NEXT:            <key>line</key><integer>31</integer>
-// CHECK-NEXT:            <key>col</key><integer>7</integer>
-// CHECK-NEXT:            <key>file</key><integer>0</integer>
-// CHECK-NEXT:           </dict>
-// CHECK-NEXT:           <dict>
-// CHECK-NEXT:            <key>line</key><integer>31</integer>
-// CHECK-NEXT:            <key>col</key><integer>7</integer>
-// CHECK-NEXT:            <key>file</key><integer>0</integer>
-// CHECK-NEXT:           </dict>
-// CHECK-NEXT:          </array>
-// CHECK-NEXT:        </dict>
-// CHECK-NEXT:       </array>
-// CHECK-NEXT:     </dict>
-// CHECK-NEXT:     <dict>
-// CHECK-NEXT:      <key>kind</key><string>control</string>
-// CHECK-NEXT:      <key>edges</key>
-// CHECK-NEXT:       <array>
-// CHECK-NEXT:        <dict>
-// CHECK-NEXT:         <key>start</key>
-// CHECK-NEXT:          <array>
-// CHECK-NEXT:           <dict>
-// CHECK-NEXT:            <key>line</key><integer>31</integer>
-// CHECK-NEXT:            <key>col</key><integer>7</integer>
-// CHECK-NEXT:            <key>file</key><integer>0</integer>
-// CHECK-NEXT:           </dict>
-// CHECK-NEXT:           <dict>
-// CHECK-NEXT:            <key>line</key><integer>31</integer>
-// CHECK-NEXT:            <key>col</key><integer>7</integer>
-// CHECK-NEXT:            <key>file</key><integer>0</integer>
-// CHECK-NEXT:           </dict>
-// CHECK-NEXT:          </array>
-// CHECK-NEXT:         <key>end</key>
-// CHECK-NEXT:          <array>
-// CHECK-NEXT:           <dict>
 // CHECK-NEXT:            <key>line</key><integer>32</integer>
 // CHECK-NEXT:            <key>col</key><integer>3</integer>
 // CHECK-NEXT:            <key>file</key><integer>0</integer>
@@ -887,40 +853,6 @@ int callGenerateNoteOnDefaultArgument(int o) {
 // CHECK-NEXT:         <key>end</key>
 // CHECK-NEXT:          <array>
 // CHECK-NEXT:           <dict>
-// CHECK-NEXT:            <key>line</key><integer>44</integer>
-// CHECK-NEXT:            <key>col</key><integer>5</integer>
-// CHECK-NEXT:            <key>file</key><integer>0</integer>
-// CHECK-NEXT:           </dict>
-// CHECK-NEXT:           <dict>
-// CHECK-NEXT:            <key>line</key><integer>44</integer>
-// CHECK-NEXT:            <key>col</key><integer>13</integer>
-// CHECK-NEXT:            <key>file</key><integer>0</integer>
-// CHECK-NEXT:           </dict>
-// CHECK-NEXT:          </array>
-// CHECK-NEXT:        </dict>
-// CHECK-NEXT:       </array>
-// CHECK-NEXT:     </dict>
-// CHECK-NEXT:     <dict>
-// CHECK-NEXT:      <key>kind</key><string>control</string>
-// CHECK-NEXT:      <key>edges</key>
-// CHECK-NEXT:       <array>
-// CHECK-NEXT:        <dict>
-// CHECK-NEXT:         <key>start</key>
-// CHECK-NEXT:          <array>
-// CHECK-NEXT:           <dict>
-// CHECK-NEXT:            <key>line</key><integer>44</integer>
-// CHECK-NEXT:            <key>col</key><integer>5</integer>
-// CHECK-NEXT:            <key>file</key><integer>0</integer>
-// CHECK-NEXT:           </dict>
-// CHECK-NEXT:           <dict>
-// CHECK-NEXT:            <key>line</key><integer>44</integer>
-// CHECK-NEXT:            <key>col</key><integer>13</integer>
-// CHECK-NEXT:            <key>file</key><integer>0</integer>
-// CHECK-NEXT:           </dict>
-// CHECK-NEXT:          </array>
-// CHECK-NEXT:         <key>end</key>
-// CHECK-NEXT:          <array>
-// CHECK-NEXT:           <dict>
 // CHECK-NEXT:            <key>line</key><integer>46</integer>
 // CHECK-NEXT:            <key>col</key><integer>3</integer>
 // CHECK-NEXT:            <key>file</key><integer>0</integer>