[analyzer] Fix the assertion failure when static globals are used in lambda by reference

[george.karpenkov]

Summary

Also use the opportunity to clean up the code and remove unnecessary duplication.

[xazax.hun]

LGTM!

I am only wondering what is the policy regarding the tests? When should we add a new file or when should we just extend an existing one?

[george.karpenkov]

I am only wondering what is the policy regarding the tests?

@xazax.hun I don't think we have any, which is indeed often confusing. Which file did you have in mind?

[xazax.hun]

I was thinking about test/Analysis/lambdas.cpp as a possible candidate.

[NoQ]

I am only wondering what is the policy regarding the tests?

There doesn't seem to be any yet. We often have to create new files when we're disappointed with the run-line(s) of the existing files or want to use global declarations that conflict with those in existing files.

lib/StaticAnalyzer/Core/ExprEngine.cpp

void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, const NamedDecl *D,
   if (const auto *VD = dyn_cast<VarDecl>(D)) {
     // C permits "extern void v", and if you cast the address to a valid type,
     // you can even do things with it. We simply pretend
     assert(Ex->isGLValue() || VD->getType()->isVoidType());
     const LocationContext *LocCtxt = Pred->getLocationContext();
     const Decl *D = LocCtxt->getDecl();
     const auto *MD = D ? dyn_cast<CXXMethodDecl>(D) : nullptr;
     const auto *DeclRefEx = dyn_cast<DeclRefExpr>(Ex);
-    SVal V;
-    bool IsReference;
+    Optional<std::pair<SVal, QualType>> VInfo;
+
     if (AMgr.options.shouldInlineLambdas() && DeclRefEx &&
         DeclRefEx->refersToEnclosingVariableOrCapture() && MD &&
         MD->getParent()->isLambda()) {
       // Lookup the field of the lambda.
       const CXXRecordDecl *CXXRec = MD->getParent();
       llvm::DenseMap<const VarDecl *, FieldDecl *> LambdaCaptureFields;
       FieldDecl *LambdaThisCaptureField;
       CXXRec->getCaptureFields(LambdaCaptureFields, LambdaThisCaptureField);
-      const FieldDecl *FD = LambdaCaptureFields[VD];
-      if (!FD) {
-        // When a constant is captured, sometimes no corresponding field is
-        // created in the lambda object.
-        assert(VD->getType().isConstQualified());
-        V = state->getLValue(VD, LocCtxt);
-        IsReference = false;
-      } else {
+
+      // Sema follows a sequence of complex rules to determine whether the
+      // variable should be captured.
+      if (const FieldDecl *FD = LambdaCaptureFields[VD]) {
         Loc CXXThis =
             svalBuilder.getCXXThis(MD, LocCtxt->getCurrentStackFrame());
         SVal CXXThisVal = state->getSVal(CXXThis);
-        V = state->getLValue(FD, CXXThisVal);
-        IsReference = FD->getType()->isReferenceType();
+        VInfo = std::make_pair(state->getLValue(FD, CXXThisVal), FD->getType());
       }
-    } else {
-      V = state->getLValue(VD, LocCtxt);
-      IsReference = VD->getType()->isReferenceType();
     }
 
+    if (!VInfo)
+      VInfo = std::make_pair(state->getLValue(VD, LocCtxt), VD->getType());
+
+    SVal V = VInfo->first;
+    bool IsReference = VInfo->second->isReferenceType();
+
     // For references, the 'lvalue' is the pointer address stored in the
     // reference region.
     if (IsReference) {
       if (const MemRegion *R = V.getAsRegion())
         V = state->getSVal(R);
       else
         V = UnknownVal();
     }

test/Analysis/lambdas.cpp

auto lambda = []{
     // Don't treat localConstant as containing a garbage value
     float copy = localConstant; // no-warning
     (void)copy;
   };
 
   lambda();
 }
 
+
+static int b = 0;
+
+int f() {
+  b = 0;
+  auto &bm = b;
+  [&] {
+    bm++;
+    bm++;
+  }();
+  if (bm != 2) {
+    int *y = 0;
+    return *y; // no-warning
+  }
+  return 0;
+}
+
 // CHECK: [B2 (ENTRY)]
 // CHECK:   Succs (1): B1
 // CHECK: [B1]
 // CHECK:   1: x
 // CHECK:   2: [B1.1] (ImplicitCastExpr, NoOp, const struct X)
 // CHECK:   3: [B1.2] (CXXConstructExpr, struct X)
 // CHECK:   4: [x]     {
 // CHECK:    }
 // CHECK:   5: (void)[B1.4] (CStyleCastExpr, ToVoid, void)
 // CHECK:   Preds (1): B2
 // CHECK:   Succs (1): B0
 // CHECK: [B0 (EXIT)]
 // CHECK:   Preds (1): B1