[cfe-commits] r163348 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp lib/StaticAnalyzer/Core/CallEvent.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp test/Analysis/dtor.cpp

Jordan Rose jordan_rose at apple.com
Thu Sep 6 13:37:09 PDT 2012


Author: jrose
Date: Thu Sep  6 15:37:08 2012
New Revision: 163348

URL: http://llvm.org/viewvc/llvm-project?rev=163348&view=rev
Log:
[analyzer] Don't attempt to devirtualize calls to base class destructors.

CXXDestructorCall now has a flag for when it is a base destructor call.
Other kinds of destructor calls (locals, fields, temporaries, and 'delete')
all behave as "whole-object" destructors and do not behave differently
from one another (specifically, in these cases we /should/ try to
devirtualize a call to a virtual destructor).

This was causing crashes in both our internal buildbot, the crash still
being tracked in PR13765, and some of the crashes being tracked in PR13763,
due to a assertion failure. (The behavior under -Asserts happened to be
correct anyway.)

Adding this knowledge also allows our DynamicTypePropagation checker to do
a bit less work; the special rules about virtual method calls during a
destructor only require extra handling during base destructors.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    cfe/trunk/test/Analysis/dtor.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h?rev=163348&r1=163347&r2=163348&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h Thu Sep  6 15:37:08 2012
@@ -604,6 +604,8 @@
   friend class CallEventManager;
 
 protected:
+  typedef llvm::PointerIntPair<const MemRegion *, 1, bool> DtorDataTy;
+
   /// Creates an implicit destructor.
   ///
   /// \param DD The destructor that will be called.
@@ -612,10 +614,10 @@
   /// \param St The path-sensitive state at this point in the program.
   /// \param LCtx The location context at this point in the program.
   CXXDestructorCall(const CXXDestructorDecl *DD, const Stmt *Trigger,
-                    const MemRegion *Target, ProgramStateRef St,
-                    const LocationContext *LCtx)
+                    const MemRegion *Target, bool IsBaseDestructor,
+                    ProgramStateRef St, const LocationContext *LCtx)
     : CXXInstanceCall(DD, St, LCtx) {
-    Data = Target;
+    Data = DtorDataTy(Target, IsBaseDestructor).getOpaqueValue();
     Location = Trigger->getLocEnd();
   }
 
@@ -626,9 +628,16 @@
   virtual SourceRange getSourceRange() const { return Location; }
   virtual unsigned getNumArgs() const { return 0; }
 
+  virtual RuntimeDefinition getRuntimeDefinition() const;
+
   /// \brief Returns the value of the implicit 'this' object.
   virtual SVal getCXXThisVal() const;
 
+  /// Returns true if this is a call to a base class destructor.
+  bool isBaseDestructor() const {
+    return DtorDataTy::getFromOpaqueValue(Data).getInt();
+  }
+
   virtual Kind getKind() const { return CE_CXXDestructor; }
 
   static bool classof(const CallEvent *CA) {
@@ -882,6 +891,13 @@
     return new (allocate()) T(A1, A2, A3, St, LCtx);
   }
 
+  template <typename T, typename Arg1, typename Arg2, typename Arg3,
+            typename Arg4>
+  T *create(Arg1 A1, Arg2 A2, Arg3 A3, Arg4 A4, ProgramStateRef St,
+            const LocationContext *LCtx) {
+    return new (allocate()) T(A1, A2, A3, A4, St, LCtx);
+  }
+
 public:
   CallEventManager(llvm::BumpPtrAllocator &alloc) : Alloc(alloc) {}
 
@@ -908,9 +924,9 @@
 
   CallEventRef<CXXDestructorCall>
   getCXXDestructorCall(const CXXDestructorDecl *DD, const Stmt *Trigger,
-                       const MemRegion *Target, ProgramStateRef State,
-                       const LocationContext *LCtx) {
-    return create<CXXDestructorCall>(DD, Trigger, Target, State, LCtx);
+                       const MemRegion *Target, bool IsBase,
+                       ProgramStateRef State, const LocationContext *LCtx) {
+    return create<CXXDestructorCall>(DD, Trigger, Target, IsBase, State, LCtx);
   }
 
   CallEventRef<CXXAllocatorCall>

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=163348&r1=163347&r2=163348&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Thu Sep  6 15:37:08 2012
@@ -385,8 +385,8 @@
   void VisitCXXConstructExpr(const CXXConstructExpr *E, ExplodedNode *Pred,
                              ExplodedNodeSet &Dst);
 
-  void VisitCXXDestructor(QualType ObjectType,
-                          const MemRegion *Dest, const Stmt *S,
+  void VisitCXXDestructor(QualType ObjectType, const MemRegion *Dest,
+                          const Stmt *S, bool IsBaseDtor,
                           ExplodedNode *Pred, ExplodedNodeSet &Dst);
 
   void VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp?rev=163348&r1=163347&r2=163348&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp Thu Sep  6 15:37:08 2012
@@ -83,14 +83,14 @@
 
   if (const CXXDestructorCall *Dtor = dyn_cast<CXXDestructorCall>(&Call)) {
     // C++11 [class.cdtor]p4 (see above)
+    if (!Dtor->isBaseDestructor())
+      return;
 
     const MemRegion *Target = Dtor->getCXXThisVal().getAsRegion();
     if (!Target)
       return;
 
-    // FIXME: getRuntimeDefinition() can be expensive. It would be better to do
-    // this when we are entering the stack frame for the destructor.
-    const Decl *D = Dtor->getRuntimeDefinition().getDecl();
+    const Decl *D = Dtor->getDecl();
     if (!D)
       return;
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=163348&r1=163347&r2=163348&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Thu Sep  6 15:37:08 2012
@@ -559,10 +559,19 @@
 
 SVal CXXDestructorCall::getCXXThisVal() const {
   if (Data)
-    return loc::MemRegionVal(static_cast<const MemRegion *>(Data));
+    return loc::MemRegionVal(DtorDataTy::getFromOpaqueValue(Data).getPointer());
   return UnknownVal();
 }
 
+RuntimeDefinition CXXDestructorCall::getRuntimeDefinition() const {
+  // Base destructors are always called non-virtually.
+  // Skip CXXInstanceCall's devirtualization logic in this case.
+  if (isBaseDestructor())
+    return AnyFunctionCall::getRuntimeDefinition();
+
+  return CXXInstanceCall::getRuntimeDefinition();
+}
+
 
 CallEvent::param_iterator ObjCMethodCall::param_begin() const {
   const ObjCMethodDecl *D = getDecl();
@@ -892,5 +901,5 @@
     Trigger = Dtor->getBody();
 
   return getCXXDestructorCall(Dtor, Trigger, ThisVal.getAsRegion(),
-                              State, CallerCtx);
+                              isa<CFGBaseDtor>(E), State, CallerCtx);
 }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=163348&r1=163347&r2=163348&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Thu Sep  6 15:37:08 2012
@@ -446,7 +446,7 @@
   Loc dest = state->getLValue(varDecl, Pred->getLocationContext());
 
   VisitCXXDestructor(varType, cast<loc::MemRegionVal>(dest).getRegion(),
-                     Dtor.getTriggerStmt(), Pred, Dst);
+                     Dtor.getTriggerStmt(), /*IsBase=*/false, Pred, Dst);
 }
 
 void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D,
@@ -464,7 +464,7 @@
   SVal BaseVal = getStoreManager().evalDerivedToBase(ThisVal, BaseTy);
 
   VisitCXXDestructor(BaseTy, cast<loc::MemRegionVal>(BaseVal).getRegion(),
-                     CurDtor->getBody(), Pred, Dst);
+                     CurDtor->getBody(), /*IsBase=*/true, Pred, Dst);
 }
 
 void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
@@ -480,7 +480,7 @@
 
   VisitCXXDestructor(Member->getType(),
                      cast<loc::MemRegionVal>(FieldVal).getRegion(),
-                     CurDtor->getBody(), Pred, Dst);
+                     CurDtor->getBody(), /*IsBase=*/false, Pred, Dst);
 }
 
 void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=163348&r1=163347&r2=163348&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Thu Sep  6 15:37:08 2012
@@ -163,6 +163,7 @@
 void ExprEngine::VisitCXXDestructor(QualType ObjectType,
                                     const MemRegion *Dest,
                                     const Stmt *S,
+                                    bool IsBaseDtor,
                                     ExplodedNode *Pred, 
                                     ExplodedNodeSet &Dst) {
   const LocationContext *LCtx = Pred->getLocationContext();
@@ -183,7 +184,7 @@
 
   CallEventManager &CEMgr = getStateManager().getCallEventManager();
   CallEventRef<CXXDestructorCall> Call =
-    CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, State, LCtx);
+    CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx);
 
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 Call->getSourceRange().getBegin(),

Modified: cfe/trunk/test/Analysis/dtor.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=163348&r1=163347&r2=163348&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dtor.cpp (original)
+++ cfe/trunk/test/Analysis/dtor.cpp Thu Sep  6 15:37:08 2012
@@ -251,3 +251,33 @@
     free(p); // no-warning
   }
 }
+
+namespace MultipleInheritanceVirtualDtors {
+  class VirtualDtor {
+  protected:
+    virtual ~VirtualDtor() {
+      clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
+    }
+  };
+
+  class NonVirtualDtor {
+  protected:
+    ~NonVirtualDtor() {
+      clang_analyzer_checkInlined(true); // expected-warning{{TRUE}}
+    }
+  };
+
+  class SubclassA : public VirtualDtor, public NonVirtualDtor {
+  public:
+    virtual ~SubclassA() {}
+  };
+  class SubclassB : public NonVirtualDtor, public VirtualDtor {
+  public:
+    virtual ~SubclassB() {}
+  };
+
+  void test() {
+    SubclassA a;
+    SubclassB b;
+  }
+}





More information about the cfe-commits mailing list