[analyzer] Fix a crash when destroying a non-region.

Add defensive check that prevents a crash when we try to evaluate a destructor
whose this-value is a concrete integer that isn't a null.

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

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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index eb20b7d6cc98193fda17b673b09c703b2e83acb0..eeb606ebfde5f8af56f6e57db9fd04390dd8a8de 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -530,7 +530,7 @@ public:
   void VisitCXXDestructor(QualType ObjectType, const MemRegion *Dest,
                           const Stmt *S, bool IsBaseDtor,
                           ExplodedNode *Pred, ExplodedNodeSet &Dst,
-                          const EvalCallOptions &Options);
+                          EvalCallOptions &Options);
 
   void VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
                                 ExplodedNode *Pred,

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 1fef5b3c1edd565610f7b14a6fbc4416659ea1f9..c5a1f9b857e09c93f6f5e11fdd96d3847fc2a3e6 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -977,8 +977,8 @@ void ExprEngine::ProcessAutomaticObjDtor(const CFGAutomaticObjDtor Dtor,
   Region = makeZeroElementRegion(state, loc::MemRegionVal(Region), varType,
                                  CallOpts.IsArrayCtorOrDtor).getAsRegion();
 
-  VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(), /*IsBase=*/ false,
-                     Pred, Dst, CallOpts);
+  VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(),
+                     /*IsBase=*/false, Pred, Dst, CallOpts);
 }
 
 void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor,
@@ -1036,8 +1036,9 @@ void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D,
   SVal BaseVal = getStoreManager().evalDerivedToBase(ThisVal, BaseTy,
                                                      Base->isVirtual());
 
-  VisitCXXDestructor(BaseTy, BaseVal.castAs<loc::MemRegionVal>().getRegion(),
-                     CurDtor->getBody(), /*IsBase=*/ true, Pred, Dst, {});
+  EvalCallOptions CallOpts;
+  VisitCXXDestructor(BaseTy, BaseVal.getAsRegion(), CurDtor->getBody(),
+                     /*IsBase=*/true, Pred, Dst, CallOpts);
 }
 
 void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
@@ -1048,10 +1049,10 @@ void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
   const LocationContext *LCtx = Pred->getLocationContext();
 
   const auto *CurDtor = cast<CXXDestructorDecl>(LCtx->getDecl());
-  Loc ThisVal = getSValBuilder().getCXXThis(CurDtor,
-                                            LCtx->getStackFrame());
-  SVal FieldVal =
-      State->getLValue(Member, State->getSVal(ThisVal).castAs<Loc>());
+  Loc ThisStorageLoc =
+      getSValBuilder().getCXXThis(CurDtor, LCtx->getStackFrame());
+  Loc ThisLoc = State->getSVal(ThisStorageLoc).castAs<Loc>();
+  SVal FieldVal = State->getLValue(Member, ThisLoc);
 
   // FIXME: We need to run the same destructor on every element of the array.
   // This workaround will just run the first destructor (which will still
@@ -1060,8 +1061,8 @@ void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
   FieldVal = makeZeroElementRegion(State, FieldVal, T,
                                    CallOpts.IsArrayCtorOrDtor);
 
-  VisitCXXDestructor(T, FieldVal.castAs<loc::MemRegionVal>().getRegion(),
-                     CurDtor->getBody(), /*IsBase=*/false, Pred, Dst, CallOpts);
+  VisitCXXDestructor(T, FieldVal.getAsRegion(), CurDtor->getBody(),
+                     /*IsBase=*/false, Pred, Dst, CallOpts);
 }
 
 void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
@@ -1109,8 +1110,6 @@ void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
   EvalCallOptions CallOpts;
   CallOpts.IsTemporaryCtorOrDtor = true;
   if (!MR) {
-    CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
-
     // If we have no MR, we still need to unwrap the array to avoid destroying
     // the whole array at once. Regardless, we'd eventually need to model array
     // destructors properly, element-by-element.

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 10c8cb1a9ee10aead6e1516dab89afdef38f2187..91afde603ca939533dd5c6b9712250609f0f61ae 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -604,7 +604,7 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType,
                                     bool IsBaseDtor,
                                     ExplodedNode *Pred,
                                     ExplodedNodeSet &Dst,
-                                    const EvalCallOptions &CallOpts) {
+                                    EvalCallOptions &CallOpts) {
   assert(S && "A destructor without a trigger!");
   const LocationContext *LCtx = Pred->getLocationContext();
   ProgramStateRef State = Pred->getState();
@@ -612,7 +612,6 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType,
   const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl();
   assert(RecordDecl && "Only CXXRecordDecls should have destructors");
   const CXXDestructorDecl *DtorDecl = RecordDecl->getDestructor();
-
   // FIXME: There should always be a Decl, otherwise the destructor call
   // shouldn't have been added to the CFG in the first place.
   if (!DtorDecl) {
@@ -626,9 +625,27 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType,
     return;
   }
 
+  if (!Dest) {
+    // We're trying to destroy something that is not a region. This may happen
+    // for a variety of reasons (unknown target region, concrete integer instead
+    // of target region, etc.). The current code makes an attempt to recover.
+    // FIXME: We probably don't really need to recover when we're dealing
+    // with concrete integers specifically.
+    CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
+    if (const Expr *E = dyn_cast_or_null<Expr>(S)) {
+      Dest = MRMgr.getCXXTempObjectRegion(E, Pred->getLocationContext());
+    } else {
+      static SimpleProgramPointTag T("ExprEngine", "SkipInvalidDestructor");
+      NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+      Bldr.generateSink(Pred->getLocation().withTag(&T),
+                        Pred->getState(), Pred);
+      return;
+    }
+  }
+
   CallEventManager &CEMgr = getStateManager().getCallEventManager();
   CallEventRef<CXXDestructorCall> Call =
-    CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx);
+      CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx);
 
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 Call->getSourceRange().getBegin(),

diff --git a/test/Analysis/dtor.cpp b/test/Analysis/dtor.cpp
index d843f03aada7eb079bf0b5b3d48a983c5c4f6292..1c6251781545caa3b86f581d71ac279ea9b8fa57 100644 (file)
--- a/test/Analysis/dtor.cpp
+++ b/test/Analysis/dtor.cpp
@@ -540,3 +540,33 @@ void f() {
   clang_analyzer_eval(__alignof(NonTrivial) > 0); // expected-warning{{TRUE}}
 }
 }
+
+namespace dtor_over_loc_concrete_int {
+struct A {
+  ~A() {}
+};
+
+struct B {
+  A a;
+  ~B() {}
+};
+
+struct C : A {
+  ~C() {}
+};
+
+void testB() {
+  B *b = (B *)-1;
+  b->~B(); // no-crash
+}
+
+void testC() {
+  C *c = (C *)-1;
+  c->~C(); // no-crash
+}
+
+void testAutoDtor() {
+  const A &a = *(A *)-1;
+  // no-crash
+}
+} // namespace dtor_over_loc_concrete_int