r178697 - [analyzer] Correctly handle destructors for lifetime-extended temporaries.

Jordan Rose jordan_rose at apple.com
Wed Apr 3 14:16:58 PDT 2013


Author: jrose
Date: Wed Apr  3 16:16:58 2013
New Revision: 178697

URL: http://llvm.org/viewvc/llvm-project?rev=178697&view=rev
Log:
[analyzer] Correctly handle destructors for lifetime-extended temporaries.

The lifetime of a temporary can be extended when it is immediately bound
to a local reference:

  const Value &MyVal = Value("temporary");

In this case, the temporary object's lifetime is extended for the entire
scope of the reference; at the end of the scope it is destroyed.

The analyzer was modeling this improperly in two ways:
- Since we don't model temporary constructors just yet, we create a fake
  temporary region when it comes time to "materialize" a temporary into
  a real object (lvalue). This wasn't taking base casts into account when
  the bindings being materialized was Unknown; now it always respects base
  casts except when the temporary region is itself a pointer.
- When actually destroying the region, the analyzer did not actually load
  from the reference variable -- it was basically destroying the reference
  instead of its referent. Now it does do the load.

This will be more useful whenever we finally start modeling temporaries,
or at least those that get bound to local reference variables.

<rdar://problem/13552274>

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/trunk/test/Analysis/dtor.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=178697&r1=178696&r2=178697&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed Apr  3 16:16:58 2013
@@ -171,18 +171,28 @@ ExprEngine::createTemporaryRegionIfNeede
                                           const Expr *Ex,
                                           const Expr *Result) {
   SVal V = State->getSVal(Ex, LC);
-  if (!Result && !V.getAs<NonLoc>())
-    return State;
+  if (!Result) {
+    // If we don't have an explicit result expression, we're in "if needed"
+    // mode. Only create a region if the current value is a NonLoc.
+    if (!V.getAs<NonLoc>())
+      return State;
+    Result = Ex;
+  } else {
+    // We need to create a region no matter what. For sanity, make sure we don't
+    // try to stuff a Loc into a non-pointer temporary region.
+    assert(!V.getAs<Loc>() || Loc::isLocType(Result->getType()));
+  }
 
   ProgramStateManager &StateMgr = State->getStateManager();
   MemRegionManager &MRMgr = StateMgr.getRegionManager();
   StoreManager &StoreMgr = StateMgr.getStoreManager();
 
   // We need to be careful about treating a derived type's value as
-  // bindings for a base type. Start by stripping and recording base casts.
+  // bindings for a base type. Unless we're creating a temporary pointer region,
+  // start by stripping and recording base casts.
   SmallVector<const CastExpr *, 4> Casts;
   const Expr *Inner = Ex->IgnoreParens();
-  if (V.getAs<NonLoc>()) {
+  if (!Loc::isLocType(Result->getType())) {
     while (const CastExpr *CE = dyn_cast<CastExpr>(Inner)) {
       if (CE->getCastKind() == CK_DerivedToBase ||
           CE->getCastKind() == CK_UncheckedDerivedToBase)
@@ -195,8 +205,13 @@ ExprEngine::createTemporaryRegionIfNeede
   }
 
   // Create a temporary object region for the inner expression (which may have
-  // a more derived type) and bind the NonLoc value into it.
-  SVal Reg = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(Inner, LC));
+  // a more derived type) and bind the value into it.
+  const TypedValueRegion *TR = MRMgr.getCXXTempObjectRegion(Inner, LC);
+  SVal Reg = loc::MemRegionVal(TR);
+
+  if (V.isUnknown())
+    V = getSValBuilder().conjureSymbolVal(Result, LC, TR->getValueType(),
+                                          currBldrCtx->blockCount());
   State = State->bindLoc(Reg, V);
 
   // Re-apply the casts (from innermost to outermost) for type sanity.
@@ -206,7 +221,7 @@ ExprEngine::createTemporaryRegionIfNeede
     Reg = StoreMgr.evalDerivedToBase(Reg, *I);
   }
 
-  State = State->BindExpr(Result ? Result : Ex, LC, Reg);
+  State = State->BindExpr(Result, LC, Reg);
   return State;
 }
 
@@ -515,18 +530,20 @@ void ExprEngine::ProcessImplicitDtor(con
 void ExprEngine::ProcessAutomaticObjDtor(const CFGAutomaticObjDtor Dtor,
                                          ExplodedNode *Pred,
                                          ExplodedNodeSet &Dst) {
-  ProgramStateRef state = Pred->getState();
   const VarDecl *varDecl = Dtor.getVarDecl();
-
   QualType varType = varDecl->getType();
 
-  if (const ReferenceType *refType = varType->getAs<ReferenceType>())
-    varType = refType->getPointeeType();
+  ProgramStateRef state = Pred->getState();
+  SVal dest = state->getLValue(varDecl, Pred->getLocationContext());
+  const MemRegion *Region = dest.castAs<loc::MemRegionVal>().getRegion();
 
-  Loc dest = state->getLValue(varDecl, Pred->getLocationContext());
+  if (const ReferenceType *refType = varType->getAs<ReferenceType>()) {
+    varType = refType->getPointeeType();
+    Region = state->getSVal(Region).getAsRegion();
+  }
 
-  VisitCXXDestructor(varType, dest.castAs<loc::MemRegionVal>().getRegion(),
-                     Dtor.getTriggerStmt(), /*IsBase=*/ false, Pred, Dst);
+  VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(), /*IsBase=*/ false,
+                     Pred, Dst);
 }
 
 void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D,

Modified: cfe/trunk/test/Analysis/dtor.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=178697&r1=178696&r2=178697&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dtor.cpp (original)
+++ cfe/trunk/test/Analysis/dtor.cpp Wed Apr  3 16:16:58 2013
@@ -328,3 +328,76 @@ namespace MultidimensionalArrays {
     clang_analyzer_eval(j == 42); // expected-warning{{UNKNOWN}}
   }
 }
+
+namespace LifetimeExtension {
+  struct IntWrapper {
+	int x;
+	IntWrapper(int y) : x(y) {}
+	IntWrapper() {
+      extern void use(int);
+      use(x); // no-warning
+	}
+  };
+
+  struct DerivedWrapper : public IntWrapper {
+	DerivedWrapper(int y) : IntWrapper(y) {}
+  };
+
+  DerivedWrapper get() {
+	return DerivedWrapper(1);
+  }
+
+  void test() {
+	const DerivedWrapper &d = get(); // lifetime extended here
+  }
+
+
+  class SaveOnDestruct {
+  public:
+    static int lastOutput;
+    int value;
+
+    SaveOnDestruct();
+    ~SaveOnDestruct() {
+      lastOutput = value;
+    }
+  };
+
+  void testSimple() {
+    {
+      const SaveOnDestruct &obj = SaveOnDestruct();
+      if (obj.value != 42)
+        return;
+      // destructor called here
+    }
+
+    clang_analyzer_eval(SaveOnDestruct::lastOutput == 42); // expected-warning{{TRUE}}
+  }
+
+  class VirtualDtorBase {
+  public:
+    int value;
+    virtual ~VirtualDtorBase() {}
+  };
+
+  class SaveOnVirtualDestruct : public VirtualDtorBase {
+  public:
+    static int lastOutput;
+
+    SaveOnVirtualDestruct();
+    virtual ~SaveOnVirtualDestruct() {
+      lastOutput = value;
+    }
+  };
+
+  void testVirtual() {
+    {
+      const VirtualDtorBase &obj = SaveOnVirtualDestruct();
+      if (obj.value != 42)
+        return;
+      // destructor called here
+    }
+
+    clang_analyzer_eval(SaveOnVirtualDestruct::lastOutput == 42); // expected-warning{{TRUE}}
+  }
+}





More information about the cfe-commits mailing list