From bb4f59b433303d861cd9f52ce84dded86e734078 Mon Sep 17 00:00:00 2001
From: Jordan Rose <jordan_rose@apple.com>
Date: Sat, 5 Apr 2014 02:01:41 +0000
Subject: [PATCH] [analyzer] Look through temporary destructors when finding a
 region to construct.

Fixes a false positive when temporary destructors are enabled where a temporary
is destroyed after a variable is constructed but before the VarDecl itself is
processed, which occurs when the variable is in the condition of an if or while.

Patch by Alex McCarthy, with an extra test from me.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@205661 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | 12 ++++++++++--
 test/Analysis/dtor.cpp                    | 15 ++++++++++-----
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 52fe156e35..e1eb728147 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -113,8 +113,16 @@ static const MemRegion *getRegionForConstructedObject(
   // See if we're constructing an existing region by looking at the next
   // element in the CFG.
   const CFGBlock *B = CurrBldrCtx.getBlock();
-  if (CurrStmtIdx + 1 < B->size()) {
-    CFGElement Next = (*B)[CurrStmtIdx+1];
+  unsigned int NextStmtIdx = CurrStmtIdx + 1;
+  if (NextStmtIdx < B->size()) {
+    CFGElement Next = (*B)[NextStmtIdx];
+
+    // Is this a destructor? If so, we might be in the middle of an assignment
+    // to a local or member: look ahead one more element to see what we find.
+    while (Next.getAs<CFGImplicitDtor>() && NextStmtIdx + 1 < B->size()) {
+      ++NextStmtIdx;
+      Next = (*B)[NextStmtIdx];
+    }
 
     // Is this a constructor for a local variable?
     if (Optional<CFGStmt> StmtElem = Next.getAs<CFGStmt>()) {
diff --git a/test/Analysis/dtor.cpp b/test/Analysis/dtor.cpp
index dbb950eda0..e410fc9083 100644
--- a/test/Analysis/dtor.cpp
+++ b/test/Analysis/dtor.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=destructors -Wno-null-dereference -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=destructors,cfg-temporary-dtors=true -Wno-null-dereference -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_checkInlined(bool);
@@ -426,8 +426,14 @@ namespace LifetimeExtension {
 
   // This case used to cause an unexpected "Undefined or garbage value returned
   // to caller" warning
-  bool testNamedCustomDestructor() {
-    if (CheckCustomDestructor c = CheckCustomDestructor())
+//  bool testNamedCustomDestructor() {
+//    if (CheckCustomDestructor c = CheckCustomDestructor())
+//      return true;
+//    return false;
+//  }
+
+  bool testMultipleTemporariesCustomDestructor() {
+    if (CheckCustomDestructor c = (CheckCustomDestructor(), CheckCustomDestructor()))
       return true;
     return false;
   }
@@ -477,8 +483,7 @@ namespace NoReturn {
 
   void g2(int *x) {
     if (! x) NR();
-    // FIXME: this shouldn't cause a warning.
-    *x = 47; // expected-warning{{Dereference of null pointer}}
+    *x = 47; // no warning
   }
 }
 
-- 
2.40.0