[PATCH] [analyzer] Fix FP warnings when binding a temporary to a local static variable

[Pavel Labath]

Hi jordan_rose,

When binding a temporary object to a static local variable, the analyzer would
complain about a dangling reference even though the temporary's lifetime should
be extended past the end of the function. This commit tries to detect these
cases and construct them in a global memory region instead of a local one.

http://llvm-reviews.chandlerc.com/D1133

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  lib/StaticAnalyzer/Core/MemRegion.cpp
  test/Analysis/stack-addr-ps.cpp

Index: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -1272,6 +1272,11 @@
   const BlockDataRegion *getBlockDataRegion(const BlockTextRegion *bc,
                                             const LocationContext *lc = NULL);
 
+  /// Create a CXXTempObjectRegion for temporaries which are lifetime-extended
+  /// by static references. This differs from getCXXTempObjectRegion in the
+  /// super-region used.
+  const CXXTempObjectRegion *getCXXExtendedTempObjectRegion(Expr const *Ex);
+
 private:
   template <typename RegionTy, typename A1>
   RegionTy* getRegion(const A1 a1);
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -208,7 +208,15 @@
 
   // Create a temporary object region for the inner expression (which may have
   // a more derived type) and bind the value into it.
-  const TypedValueRegion *TR = MRMgr.getCXXTempObjectRegion(Inner, LC);
+  const TypedValueRegion *TR = NULL;
+  if (const MaterializeTemporaryExpr *MT =
+          dyn_cast<MaterializeTemporaryExpr>(Result)) {
+    if (MT->getStorageDuration() == SD_Static)
+        TR = MRMgr.getCXXExtendedTempObjectRegion(Inner);
+  }
+  if (!TR)
+    TR = MRMgr.getCXXTempObjectRegion(Inner, LC);
+
   SVal Reg = loc::MemRegionVal(TR);
 
   if (V.isUnknown())
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -136,8 +136,8 @@
     if (currStmtIdx + 1 < B->size()) {
       CFGElement Next = (*B)[currStmtIdx+1];
 
-      // Is this a constructor for a local variable?
       if (Optional<CFGStmt> StmtElem = Next.getAs<CFGStmt>()) {
+        // Is this a constructor for a local variable?
         if (const DeclStmt *DS = dyn_cast<DeclStmt>(StmtElem->getStmt())) {
           if (const VarDecl *Var = dyn_cast<VarDecl>(DS->getSingleDecl())) {
             if (Var->getInit()->IgnoreImplicit() == CE) {
@@ -147,6 +147,20 @@
               Target = LValue.getAsRegion();
             }
           }
+        // Or a constructor for a temporary variable bound to a static
+        // reference?
+        } else if (isa<ImplicitCastExpr>(StmtElem->getStmt()) &&
+                   currStmtIdx + 2 < B->size()) {
+          CFGElement NNext = (*B)[currStmtIdx+2];
+          if (Optional<CFGStmt> NStmtElem = NNext.getAs<CFGStmt>()) {
+            if (const MaterializeTemporaryExpr *MT =
+                    dyn_cast<MaterializeTemporaryExpr>(NStmtElem->getStmt())) {
+              if (MT->getStorageDuration() == SD_Static) {
+                MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
+                Target = MRMgr.getCXXExtendedTempObjectRegion(CE);
+              }
+            }
+          }
         }
       }
       
Index: lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- lib/StaticAnalyzer/Core/MemRegion.cpp
+++ lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -864,6 +864,12 @@
   return getSubRegion<BlockDataRegion>(BC, LC, sReg);
 }
 
+const CXXTempObjectRegion *
+MemRegionManager::getCXXExtendedTempObjectRegion(Expr const *Ex) {
+  return getSubRegion<CXXTempObjectRegion>(
+      Ex, getGlobalsRegion(MemRegion::GlobalInternalSpaceRegionKind, NULL));
+}
+
 const CompoundLiteralRegion*
 MemRegionManager::getCompoundLiteralRegion(const CompoundLiteralExpr *CL,
                                            const LocationContext *LC) {
Index: test/Analysis/stack-addr-ps.cpp
===================================================================
--- test/Analysis/stack-addr-ps.cpp
+++ test/Analysis/stack-addr-ps.cpp
@@ -20,6 +20,10 @@
   return s3; // expected-warning{{Address of stack memory associated with local variable 's1' returned}} expected-warning {{reference to stack memory associated with local variable 's1' returned}}
 }
 
+void g4() {
+  static const int &x = 3; // no warning
+}
+
 int get_value();
 
 const int &get_reference1() { return get_value(); } // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1133.1.patch
Type: text/x-patch
Size: 4532 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130712/1ac8a8d1/attachment.bin>