[analyzer] DeadStoresChecker: Treat locals captured by reference in C++ lambdas as escaped.

[dcoughlin]

Summary

The analyzer currently reports dead store false positives when a local variable is captured by reference in a C++ lambda.

For example:

int local = 0; 
auto lambda = [&local]() {
  local++;
};
local = 7; // False Positive: Value stored to 'local' is never read
lambda();

In this case, the assignment setting local to 7 is not a dead store because the called lambda will later read that assigned value.

This patch silences this source of false positives by treating locals captured by reference in C++ lambdas as escaped, similarly to how the DeadStoresChecker deals with locals whose address is taken.

[dcoughlin]

This is PR 22834. https://llvm.org/bugs/show_bug.cgi?id=22834.

[xazax.hun]

This looks good to me, I have one question inline.

lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp

425

As far as I can see ThisCaptureField is not used. Does this solution work, when there is a dead store to a class member (through captured this)?

[xazax.hun]

In the meantime I realized dead store analysis only works with local variables, so I think I answered my comment.

It was commited in r253630.

lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp

     // Check for '&'. Any VarDecl whose address has been taken we treat as
     // escaped.
     // FIXME: What about references?
+    if (auto *LE = dyn_cast<LambdaExpr>(S)) {
+      findLambdaReferenceCaptures(LE);
+      return;
+    }
+
     const UnaryOperator *U = dyn_cast<UnaryOperator>(S);
     if (!U)
       return;

       if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl()))
         Escaped.insert(VD);
   }
+
+  // Treat local variables captured by reference in C++ lambdas as escaped.
+  void findLambdaReferenceCaptures(const LambdaExpr *LE)  {
+    const CXXRecordDecl *LambdaClass = LE->getLambdaClass();
+    llvm::DenseMap<const VarDecl *, FieldDecl *> CaptureFields;
+    FieldDecl *ThisCaptureField;
+    LambdaClass->getCaptureFields(CaptureFields, ThisCaptureField);
+
+    for (const LambdaCapture &C : LE->captures()) {
+      if (!C.capturesVariable())
+        continue;
+
+      VarDecl *VD = C.getCapturedVar();
+      const FieldDecl *FD = CaptureFields[VD];
+      if (!FD)
+        continue;
+
+      // If the capture field is a reference type, it is capture-by-reference.
+      if (FD->getType()->isReferenceType())
+        Escaped.insert(VD);
+    }
+  }
 };
 } // end anonymous namespace
 

test/Analysis/lambdas.cpp

-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config inline-lambdas=true -verify %s 
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,deadcode,debug.ExprInspection -analyzer-config inline-lambdas=true -verify %s
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,debug.DumpCFG -analyzer-config inline-lambdas=true %s > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 

   }();
 }
 
+// Lambda capture counts as use for dead-store checking.
+
+int returnsValue();
+
+void captureByCopyCausesUse() {
+  int local1 = returnsValue(); // no-warning
+  int local2 = returnsValue(); // no-warning
+  int local3 = returnsValue(); // expected-warning{{Value stored to 'local3' during its initialization is never read}}
+
+  (void)[local1, local2]() { }; // Explicit capture by copy counts as use.
+
+  int local4 = returnsValue(); // no-warning
+  int local5 = returnsValue(); // expected-warning{{Value stored to 'local5' during its initialization is never read}}
+
+  (void)[=]() {
+    (void)local4; // Implicit capture by copy counts as use
+  };
+}
+
+void captureByReference() {
+  int local1 = returnsValue(); // no-warning
+
+  auto lambda1 = [&local1]() { // Explicit capture by reference
+    local1++;
+  };
+
+  // Don't treat as a dead store because local1 was was captured by reference.
+  local1 = 7; // no-warning
+
+  lambda1();
+
+  int local2 = returnsValue(); // no-warning
+
+  auto lambda2 = [&]() {
+    local2++; // Implicit capture by reference
+  };
+
+  // Don't treat as a dead store because local2 was was captured by reference.
+  local2 = 7; // no-warning
+
+  lambda2();
+}
+
 
 // CHECK: [B2 (ENTRY)]
 // CHECK:   Succs (1): B1