[Analyzer] Correctly handle parameters passed by reference when bodyfarming std::call_once

[george.karpenkov]

Summary

Also explicitly do not support functors for now, since that entails figuring out which call operator corresponds to the passed parameters.
Resolution: we really should not do body farming for templated C++ functions, even in seemingly trivial cases!

[danielmarjamaki]

Stylistically this looks pretty good to me. Just a minor nit.

lib/Analysis/BodyFarm.cpp

389

ehm.. I would remove this blank

[dcoughlin]

Some nits inline, but looks good to me!

lib/Analysis/BodyFarm.cpp

388

Nit: 'i' doesn't match the naming conventions and it is not particularly descriptive. Perhaps "ParamIndex"?

391

Nit: You can move PTy inside the if block so it is not calculated when it is not needed.

lib/Analysis/BodyFarm.cpp

return new (C) CallExpr(
       /*args=*/CallArgs,
       /*QualType=*/C.VoidTy,
       /*ExprValueType=*/VK_RValue,
       /*SourceLocation=*/SourceLocation());
 }
 
 static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M,
                                               const ParmVarDecl *Callback,
-                                              QualType CallbackType,
+                                              CXXRecordDecl *CallbackDecl,
                                               ArrayRef<Expr *> CallArgs) {
-
-  CXXRecordDecl *CallbackDecl = CallbackType->getAsCXXRecordDecl();
-
   assert(CallbackDecl != nullptr);
   assert(CallbackDecl->isLambda());
   FunctionDecl *callOperatorDecl = CallbackDecl->getLambdaCallOperator();
   assert(callOperatorDecl != nullptr);
 
   DeclRefExpr *callOperatorDeclRef =
       DeclRefExpr::Create(/* Ctx = */ C,
                           /* QualifierLoc = */ NestedNameSpecifierLoc(),
static Stmt *create_call_once(ASTContext &C, const FunctionDecl *D) {
   if (D->param_size() < 2)
     return nullptr;
 
   ASTMaker M(C);
 
   const ParmVarDecl *Flag = D->getParamDecl(0);
   const ParmVarDecl *Callback = D->getParamDecl(1);
   QualType CallbackType = Callback->getType().getNonReferenceType();
+
+  // Nullable pointer, non-null iff function is a CXXRecordDecl.
+  CXXRecordDecl *CallbackRecordDecl = CallbackType->getAsCXXRecordDecl();
   QualType FlagType = Flag->getType().getNonReferenceType();
   auto *FlagRecordDecl = dyn_cast_or_null<RecordDecl>(FlagType->getAsTagDecl());
 
   if (!FlagRecordDecl) {
     DEBUG(llvm::dbgs() << "Flag field is not a record: "
                        << "unknown std::call_once implementation, "
                        << "ignoring the call.\n");
     return nullptr;
static Stmt *create_call_once(ASTContext &C, const FunctionDecl *D) {
 
   if (!FlagFieldDecl) {
     DEBUG(llvm::dbgs() << "No field _M_once found on std::once flag struct: "
                        << "unknown std::call_once implementation, "
                        << "ignoring the call");
     return nullptr;
   }
 
-  bool isLambdaCall = CallbackType->getAsCXXRecordDecl() &&
-                      CallbackType->getAsCXXRecordDecl()->isLambda();
+  bool isLambdaCall = CallbackRecordDecl && CallbackRecordDecl->isLambda();
+  if (CallbackRecordDecl && !isLambdaCall) {
+    DEBUG(llvm::dbgs() << "Not supported: synthesizing body for functors when "
+                       << "body farming std::call_once, ignoring the call.");
+    return nullptr;
+  }
 
   SmallVector<Expr *, 5> CallArgs;
+  const FunctionProtoType *CallbackFunctionType;
+  if (isLambdaCall) {
 
-  if (isLambdaCall)
     // Lambda requires callback itself inserted as a first parameter.
     CallArgs.push_back(
         M.makeDeclRefExpr(Callback,
                           /* RefersToEnclosingVariableOrCapture= */ true));
+    CallbackFunctionType = CallbackRecordDecl->getLambdaCallOperator()
+                               ->getType()
+                               ->getAs<FunctionProtoType>();
+  } else {
+    CallbackFunctionType =
+        CallbackType->getPointeeType()->getAs<FunctionProtoType>();
+  }
 
-  // All arguments past first two ones are passed to the callback.
-  for (unsigned int i = 2; i < D->getNumParams(); i++)
-    CallArgs.push_back(
-        M.makeLvalueToRvalue(D->getParamDecl(i),
-                             /* RefersToEnclosingVariableOrCapture= */ false));
+  if (!CallbackFunctionType)
+    return nullptr;
+
+  // First two arguments are used for the flag and for the callback.
+  if (D->getNumParams() != CallbackFunctionType->getNumParams() + 2) {
+    DEBUG(llvm::dbgs() << "Number of params of the callback does not match "
+                       << "the number of params passed to std::call_once, "
+                       << "ignoring the call");
+    return nullptr;
+  }
+
+  // All arguments past first two ones are passed to the callback,
+  // and we turn lvalues into rvalues if the argument is not passed by
+  // reference.
+  for (unsigned int i = 2; i < D->getNumParams(); i++) {
+
+    const ParmVarDecl *PDecl = D->getParamDecl(i);
+    QualType PTy = PDecl->getType().getNonReferenceType();
+    Expr *ParamExpr = M.makeDeclRefExpr(PDecl);
+    if (!CallbackFunctionType->getParamType(i - 2)->isReferenceType()) {
+      ParamExpr = M.makeLvalueToRvalue(ParamExpr, PTy);
+    }
+    CallArgs.push_back(ParamExpr);
+  }
 
   CallExpr *CallbackCall;
   if (isLambdaCall) {
 
-    CallbackCall =
-        create_call_once_lambda_call(C, M, Callback, CallbackType, CallArgs);
+    CallbackCall = create_call_once_lambda_call(C, M, Callback,
+                                                CallbackRecordDecl, CallArgs);
   } else {
 
     // Function pointer case.
     CallbackCall = create_call_once_funcptr_call(C, M, Callback, CallArgs);
   }
 
   DeclRefExpr *FlagDecl =
       M.makeDeclRefExpr(Flag,

test/Analysis/call_once.cpp

 namespace std {
 template <typename d, typename e>
 void call_once(d, e);
 }
 void g();
 void test_no_segfault_on_different_impl() {
   std::call_once(g, false); // no-warning
 }
+
+void test_lambda_refcapture() {
+  static std::once_flag flag;
+  int a = 6;
+  std::call_once(flag, [&](int &a) { a = 42; }, a);
+  clang_analyzer_eval(a == 42); // expected-warning{{TRUE}}
+}
+
+void test_lambda_refcapture2() {
+  static std::once_flag flag;
+  int a = 6;
+  std::call_once(flag, [=](int &a) { a = 42; }, a);
+  clang_analyzer_eval(a == 42); // expected-warning{{TRUE}}
+}
+
+void test_lambda_fail_refcapture() {
+  static std::once_flag flag;
+  int a = 6;
+  std::call_once(flag, [=](int a) { a = 42; }, a);
+  clang_analyzer_eval(a == 42); // expected-warning{{FALSE}}
+}
+
+void mutator(int &param) {
+  param = 42;
+}
+void test_reftypes_funcptr() {
+  static std::once_flag flag;
+  int a = 6;
+  std::call_once(flag, &mutator, a);
+  clang_analyzer_eval(a == 42); // expected-warning{{TRUE}}
+}
+
+void fail_mutator(int param) {
+  param = 42;
+}
+void test_mutator_noref() {
+  static std::once_flag flag;
+  int a = 6;
+  std::call_once(flag, &fail_mutator, a);
+  clang_analyzer_eval(a == 42); // expected-warning{{FALSE}}
+}