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

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov created this revision.
Herald added subscribers: szepet, kristof.beyls, xazax.hun, javed.absar, 
aemerson.

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!


https://reviews.llvm.org/D39031

Files:
  lib/Analysis/BodyFarm.cpp
  test/Analysis/call_once.cpp


Index: test/Analysis/call_once.cpp
===================================================================
--- test/Analysis/call_once.cpp
+++ test/Analysis/call_once.cpp
@@ -249,3 +249,44 @@
 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}}
+}
Index: lib/Analysis/BodyFarm.cpp
===================================================================
--- lib/Analysis/BodyFarm.cpp
+++ lib/Analysis/BodyFarm.cpp
@@ -264,11 +264,8 @@
 
 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();
@@ -319,6 +316,9 @@
   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());
 
@@ -348,28 +348,59 @@
     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.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits