From e69b043088388837a744a2463902b39b320a4d2f Mon Sep 17 00:00:00 2001
From: Devin Coughlin <dcoughlin@apple.com>
Date: Sun, 15 Nov 2015 03:07:17 +0000
Subject: [PATCH] [analyzer] Refer to capture field to determine if capture is
 reference.

The analyzer incorrectly treats captures as references if either the original
captured variable is a reference or the variable is captured by reference.
This causes the analyzer to crash when capturing a reference type by copy
(PR24914). Fix this by refering solely to the capture field to determine when a
DeclRefExpr for a lambda capture should be treated as a reference type.

https://llvm.org/bugs/show_bug.cgi?id=24914
rdar://problem/23524412

llvm-svn: 253157
---
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 10 +--
 clang/test/Analysis/lambdas.cpp              | 72 ++++++++++++++++++++
 2 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index b6aed88c6005..37e13801bdad 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1867,7 +1867,7 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
     const auto *MD = D ? dyn_cast<CXXMethodDecl>(D) : nullptr;
     const auto *DeclRefEx = dyn_cast<DeclRefExpr>(Ex);
     SVal V;
-    bool CaptureByReference = false;
+    bool IsReference;
     if (AMgr.options.shouldInlineLambdas() && DeclRefEx &&
         DeclRefEx->refersToEnclosingVariableOrCapture() && MD &&
         MD->getParent()->isLambda()) {
@@ -1882,22 +1882,22 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
         // created in the lambda object.
         assert(VD->getType().isConstQualified());
         V = state->getLValue(VD, LocCtxt);
+        IsReference = false;
       } else {
         Loc CXXThis =
             svalBuilder.getCXXThis(MD, LocCtxt->getCurrentStackFrame());
         SVal CXXThisVal = state->getSVal(CXXThis);
         V = state->getLValue(FD, CXXThisVal);
-        if (FD->getType()->isReferenceType() &&
-            !VD->getType()->isReferenceType())
-          CaptureByReference = true;
+        IsReference = FD->getType()->isReferenceType();
       }
     } else {
       V = state->getLValue(VD, LocCtxt);
+      IsReference = VD->getType()->isReferenceType();
     }
 
     // For references, the 'lvalue' is the pointer address stored in the
     // reference region.
-    if (VD->getType()->isReferenceType() || CaptureByReference) {
+    if (IsReference) {
       if (const MemRegion *R = V.getAsRegion())
         V = state->getSVal(R);
       else
diff --git a/clang/test/Analysis/lambdas.cpp b/clang/test/Analysis/lambdas.cpp
index 10f6d5595820..a906cedb3a94 100644
--- a/clang/test/Analysis/lambdas.cpp
+++ b/clang/test/Analysis/lambdas.cpp
@@ -90,6 +90,17 @@ void testReturnValue() {
   clang_analyzer_eval(b == 8); // expected-warning{{TRUE}}
 }
 
+void testAliasingBetweenParameterAndCapture() {
+  int i = 5;
+
+  auto l = [&i](int &p) {
+    i++;
+    p++;
+  };
+  l(i);
+  clang_analyzer_eval(i == 7); // expected-warning{{TRUE}}
+}
+
 // Nested lambdas.
 
 void testNestedLambdas() {
@@ -210,6 +221,67 @@ void captureConstants() {
   }();
 }
 
+void captureReferenceByCopy(int &p) {
+  int v = 7;
+  p = 8;
+
+  // p is a reference captured by copy
+  [&v,p]() mutable {
+    v = p;
+    p = 22;
+  }();
+
+  clang_analyzer_eval(v == 8); // expected-warning{{TRUE}}
+  clang_analyzer_eval(p == 8); // expected-warning{{TRUE}}
+}
+
+void captureReferenceByReference(int &p) {
+  int v = 7;
+  p = 8;
+
+  // p is a reference captured by reference
+  [&v,&p]() {
+    v = p;
+    p = 22;
+  }();
+
+  clang_analyzer_eval(v == 8); // expected-warning{{TRUE}}
+  clang_analyzer_eval(p == 22); // expected-warning{{TRUE}}
+}
+
+void callMutableLambdaMultipleTimes(int &p) {
+  int v = 0;
+  p = 8;
+
+  auto l = [&v, p]() mutable {
+    v = p;
+    p++;
+  };
+
+  l();
+
+  clang_analyzer_eval(v == 8); // expected-warning{{TRUE}}
+  clang_analyzer_eval(p == 8); // expected-warning{{TRUE}}
+
+  l();
+
+  clang_analyzer_eval(v == 9); // expected-warning{{TRUE}}
+  clang_analyzer_eval(p == 8); // expected-warning{{TRUE}}
+}
+
+// PR 24914
+struct StructPR24914{
+  int x;
+};
+
+void takesConstStructArgument(const StructPR24914&);
+void captureStructReference(const StructPR24914& s) {
+  [s]() {
+    takesConstStructArgument(s);
+  }();
+}
+
+
 // CHECK: [B2 (ENTRY)]
 // CHECK:   Succs (1): B1
 // CHECK: [B1]