[analyzer] Handle C++ member initializers and destructors.

This uses CFG to tell if a constructor call is for a member, and uses
the member's region appropriately.

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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
index fd5f4373cd26ea4739791a11c4c25917cde23cf3..b49e8e3bf8f00dfec9f1d861c5c5b4aa859d45a1 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -238,6 +238,9 @@ public:
   /// Get the lvalue for a field reference.
   SVal getLValue(const FieldDecl *decl, SVal Base) const;
 
+  /// Get the lvalue for an indirect field reference.
+  SVal getLValue(const IndirectFieldDecl *decl, SVal Base) const;
+
   /// Get the lvalue for an array index.
   SVal getLValue(QualType ElementType, SVal Idx, SVal Base) const;
 
@@ -649,6 +652,18 @@ inline SVal ProgramState::getLValue(const FieldDecl *D, SVal Base) const {
   return getStateManager().StoreMgr->getLValueField(D, Base);
 }
 
+inline SVal ProgramState::getLValue(const IndirectFieldDecl *D,
+                                    SVal Base) const {
+  StoreManager &SM = *getStateManager().StoreMgr;
+  for (IndirectFieldDecl::chain_iterator I = D->chain_begin(),
+                                         E = D->chain_end();
+       I != E; ++I) {
+    Base = SM.getLValueField(cast<FieldDecl>(*I), Base);
+  }
+
+  return Base;
+}
+
 inline SVal ProgramState::getLValue(QualType ElementType, SVal Idx, SVal Base) const{
   if (NonLoc *N = dyn_cast<NonLoc>(&Idx))
     return getStateManager().StoreMgr->getLValueElement(ElementType, *N, Base);

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 09cd02701ed931ebcf9b212cdc05e37870736a3a..cc244a56a352eaaabae82edffd876eda14193aa1 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -358,43 +358,42 @@ void ExprEngine::ProcessInitializer(const CFGInitializer Init,
   ExplodedNodeSet Dst;
   NodeBuilder Bldr(Pred, Dst, *currentBuilderContext);
 
+  ProgramStateRef State = Pred->getState();
+
   // We don't set EntryNode and currentStmt. And we don't clean up state.
   const CXXCtorInitializer *BMI = Init.getInitializer();
   const StackFrameContext *stackFrame =
                            cast<StackFrameContext>(Pred->getLocationContext());
   const CXXConstructorDecl *decl =
                            cast<CXXConstructorDecl>(stackFrame->getDecl());
-  SVal thisVal = Pred->getState()->getSVal(svalBuilder.getCXXThis(decl,
-                                                                  stackFrame));
+  SVal thisVal = State->getSVal(svalBuilder.getCXXThis(decl, stackFrame));
 
+  // Evaluate the initializer, if necessary
   if (BMI->isAnyMemberInitializer()) {
-    // Evaluate the initializer.
-    ProgramStateRef state = Pred->getState();
-
-    const FieldDecl *FD = BMI->getAnyMember();
-
-    // FIXME: This does not work for initializers that call constructors.
-    SVal FieldLoc = state->getLValue(FD, thisVal);
-    SVal InitVal = state->getSVal(BMI->getInit(), Pred->getLocationContext());
-    state = state->bindLoc(FieldLoc, InitVal);
+    // Constructors build the object directly in the field,
+    // but non-objects must be copied in from the initializer.
+    if (!isa<CXXConstructExpr>(BMI->getInit())) {
+      SVal FieldLoc;
+      if (BMI->isIndirectMemberInitializer())
+        FieldLoc = State->getLValue(BMI->getIndirectMember(), thisVal);
+      else
+        FieldLoc = State->getLValue(BMI->getMember(), thisVal);
 
-    // Use a custom node building process.
-    PostInitializer PP(BMI, stackFrame);
-    // Builder automatically add the generated node to the deferred set,
-    // which are processed in the builder's dtor.
-    Bldr.generateNode(PP, state, Pred);
+      SVal InitVal = State->getSVal(BMI->getInit(), stackFrame);
+      State = State->bindLoc(FieldLoc, InitVal);
+    }
   } else {
     assert(BMI->isBaseInitializer());
-
     // We already did all the work when visiting the CXXConstructExpr.
-    // Just construct a PostInitializer node so that the diagnostics don't get
-    // confused.
-    PostInitializer PP(BMI, stackFrame);
-    // Builder automatically add the generated node to the deferred set,
-    // which are processed in the builder's dtor.
-    Bldr.generateNode(PP, Pred->getState(), Pred);
   }
 
+  // Construct a PostInitializer node whether the state changed or not,
+  // so that the diagnostics don't get confused.
+  PostInitializer PP(BMI, stackFrame);
+  // Builder automatically add the generated node to the deferred set,
+  // which are processed in the builder's dtor.
+  Bldr.generateNode(PP, State, Pred);
+
   // Enqueue the new nodes onto the work list.
   Engine.enqueue(Dst, currentBuilderContext->getBlock(), currentStmtIdx);
 }
@@ -459,7 +458,20 @@ void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D,
 }
 
 void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
-                                   ExplodedNode *Pred, ExplodedNodeSet &Dst) {}
+                                   ExplodedNode *Pred, ExplodedNodeSet &Dst) {
+  const FieldDecl *Member = D.getFieldDecl();
+  ProgramStateRef State = Pred->getState();
+  const LocationContext *LCtx = Pred->getLocationContext();
+
+  const CXXDestructorDecl *CurDtor = cast<CXXDestructorDecl>(LCtx->getDecl());
+  Loc ThisVal = getSValBuilder().getCXXThis(CurDtor,
+                                            LCtx->getCurrentStackFrame());
+  SVal FieldVal = State->getLValue(Member, cast<Loc>(State->getSVal(ThisVal)));
+
+  VisitCXXDestructor(Member->getType(),
+                     cast<loc::MemRegionVal>(FieldVal).getRegion(),
+                     CurDtor->getBody(), Pred, Dst);
+}
 
 void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
                                       ExplodedNode *Pred,

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index b21928866c9d1d5d3bd9d471c021f0acb109e0e1..88fba2919e81d231d9be17e50a62f21aa12203ce 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -63,6 +63,25 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
             if (Var->getInit() == CE)
               Target = State->getLValue(Var, LCtx).getAsRegion();
 
+      // Is this a constructor for a member?
+      if (const CFGInitializer *InitElem = dyn_cast<CFGInitializer>(&Next)) {
+        const CXXCtorInitializer *Init = InitElem->getInitializer();
+        assert(Init->isAnyMemberInitializer());
+
+        const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl());
+        Loc ThisPtr = getSValBuilder().getCXXThis(CurCtor,
+                                                  LCtx->getCurrentStackFrame());
+        SVal ThisVal = State->getSVal(ThisPtr);
+
+        if (Init->isIndirectMemberInitializer()) {
+          SVal Field = State->getLValue(Init->getIndirectMember(), ThisVal);
+          Target = cast<loc::MemRegionVal>(Field).getRegion();
+        } else {
+          SVal Field = State->getLValue(Init->getMember(), ThisVal);
+          Target = cast<loc::MemRegionVal>(Field).getRegion();
+        }
+      }
+
       // FIXME: This will eventually need to handle new-expressions as well.
     }
 

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 29481c6512691c6c498587997940d906df50c002..bc6aeaa0368da1cf86e812580cb1b111d0480f87 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -305,13 +305,15 @@ bool ExprEngine::inlineCall(const CallEvent &Call,
     if (!ADC->getCFGBuildOptions().AddImplicitDtors ||
         !ADC->getCFGBuildOptions().AddInitializers)
       return false;
-    // FIXME: This is a hack. We don't handle member or temporary constructors
+
+    // FIXME: This is a hack. We don't handle temporary destructors
     // right now, so we shouldn't inline their constructors.
     if (const CXXConstructorCall *Ctor = dyn_cast<CXXConstructorCall>(&Call)) {
       const CXXConstructExpr *CtorExpr = Ctor->getOriginExpr();
       if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete)
-        if (!isa<VarRegion>(Ctor->getCXXThisVal().getAsRegion()))
-          return false;
+        if (const MemRegion *Target = Ctor->getCXXThisVal().getAsRegion())
+          if (!isa<DeclRegion>(Target))
+            return false;
     }
     break;
   }

diff --git a/test/Analysis/dtor.cpp b/test/Analysis/dtor.cpp
index 8d67f78a4a787918856952a15d8c0c089e283d69..f5837539cb7b41fd5aabec16e4ef4491037b6251 100644 (file)
--- a/test/Analysis/dtor.cpp
+++ b/test/Analysis/dtor.cpp
@@ -103,3 +103,21 @@ void testMultipleInheritance3() {
     // expected-warning@25 {{Attempt to free released memory}}
   }
 }
+
+
+class SmartPointerMember {
+  SmartPointer P;
+public:
+  SmartPointerMember(void *x) : P(x) {}
+};
+
+void testSmartPointerMember() {
+  char *mem = (char*)malloc(4);
+  {
+    SmartPointerMember Deleter(mem);
+    // Remove dead bindings...
+    doSomething();
+    // destructor called here
+  }
+  *mem = 0; // expected-warning{{Use of memory after it is freed}}
+}

diff --git a/test/Analysis/initializer.cpp b/test/Analysis/initializer.cpp
index 6640e1fc4955e63cc56a9f149eac8d362947ade5..0580503a44bf54202e62463c4e05c91356175e14 100644 (file)
--- a/test/Analysis/initializer.cpp
+++ b/test/Analysis/initializer.cpp
@@ -1,4 +1,7 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store region -cfg-add-initializers -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store region -cfg-add-initializers -cfg-add-implicit-dtors -verify %s
+
+// We don't inline constructors unless we have both initializers and
+// implicit destructors turned on.
 
 void clang_analyzer_eval(bool);
 
@@ -11,3 +14,33 @@ public:
 A::A() : x(0) {
   clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
 }
+
+
+class DirectMember {
+  int x;
+public:
+  DirectMember(int value) : x(value) {}
+
+  int getX() { return x; }
+};
+
+void testDirectMember() {
+  DirectMember obj(3);
+  clang_analyzer_eval(obj.getX() == 3); // expected-warning{{TRUE}}
+}
+
+
+class IndirectMember {
+  struct {
+    int x;
+  };
+public:
+  IndirectMember(int value) : x(value) {}
+
+  int getX() { return x; }
+};
+
+void testIndirectMember() {
+  IndirectMember obj(3);
+  clang_analyzer_eval(obj.getX() == 3); // expected-warning{{TRUE}}
+}