[analyzer] Fix yet-another-crash in body-farming std::call_once

[george.karpenkov]

Summary

Crash occurs when parameters to the callback and to std::call_once mismatch, and C++ is supposed to auto-construct an argument.

Filed by Alexander Kornienko in https://bugs.llvm.org/show_bug.cgi?id=36149

rdar://37034403

[dcoughlin]

This seems reasonable to me.

@NoQ can you take a close look at this too?

[NoQ]

Yep, looks great.

test/Analysis/call_once.cpp

382

Could you add a more explicit TODO/FIXME, a no-crash where there used to be the crash, and maybe a bit of background for what this test tests? Cause recently i've been reading a lot of tests that i don't understand :)

[NoQ]

lib/Analysis/BodyFarm.cpp

413–414

Actually, this should probably go after the check?

[george.karpenkov]

Less intrusive patch.

lib/Analysis/BodyFarm.cpp

if (isLambdaCall) {
     CallbackFunctionType = CallbackType->getAs<FunctionProtoType>();
   }
 
   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() << "Types of params of the callback do not match "
-                       << "params passed to std::call_once, "
+    DEBUG(llvm::dbgs() << "Length of params of the callback does not match "
+                       << "length params passed to std::call_once, "
                        << "ignoring the call\n");
     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 ParamIdx = 2; ParamIdx < D->getNumParams(); ParamIdx++) {
     const ParmVarDecl *PDecl = D->getParamDecl(ParamIdx);
     Expr *ParamExpr = M.makeDeclRefExpr(PDecl);
-    if (!CallbackFunctionType->getParamType(ParamIdx - 2)->isReferenceType()) {
-      QualType PTy = PDecl->getType().getNonReferenceType();
-      ParamExpr = M.makeLvalueToRvalue(ParamExpr, PTy);
+    QualType CallbackParamType =
+      CallbackFunctionType->getParamType(ParamIdx - 2);
+    QualType PTy = PDecl->getType();
+    if (!CallbackParamType->isReferenceType())
+      ParamExpr = M.makeLvalueToRvalue(ParamExpr, PTy.getNonReferenceType());
+
+    if (CallbackParamType.getNonReferenceType().getCanonicalType() !=
+        PTy.getNonReferenceType().getCanonicalType()) {
+      DEBUG(llvm::dbgs() << "Types of params of the callback do not match "
+          << "params passed to std::call_once, "
+          << "ignoring the call\n");
+      return nullptr;
     }
     CallArgs.push_back(ParamExpr);
   }
 
   CallExpr *CallbackCall;
   if (isLambdaCall) {
 
     CallbackCall = create_call_once_lambda_call(C, M, Callback,
Stmt *BodyFarm::getBody(const ObjCMethodDecl *D) {
   //
   if (D->param_size() != 0)
     return nullptr;
 
   Val = createObjCPropertyGetter(C, Prop);
 
   return Val.getValue();
 }
-

test/Analysis/call_once.cpp

 // RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -verify %s -o %t.report
 // RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBSTDCPP -verify %s -o %t.report
 
 // We do NOT model libcxx03 implementation, but the analyzer should still
 // not crash.
 // RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBCXX03 -verify %s -o %t.report
 // RUN: %clang_analyze_cc1 -std=c++11 -fblocks -analyzer-checker=core,debug.ExprInspection -DEMULATE_LIBCXX03 -DEMULATE_LIBSTDCPP -verify %s -o %t.report
 // RUN: rm -rf %t.report
 
 void clang_analyzer_eval(bool);
 
-// Faking std::std::call_once implementation.
+// Faking std::call_once implementation.
 namespace std {
 
+// Fake std::function implementation.
+template <typename>
+class function;
+class function_base {
+ public:
+  long field;
+};
+template <typename R, typename... P>
+class function<R(P...)> : function_base {
+ public:
+   R operator()(P...) const {
+
+     // Read from a super-class necessary to reproduce a crash.
+     bool a = field;
+   }
+};
+
 #ifndef EMULATE_LIBSTDCPP
 typedef struct once_flag_s {
   unsigned long __state_ = 0;
 } once_flag;
 #else
 typedef struct once_flag_s {
   int _M_once = 0;
 } once_flag;
void test_implicit_funcptr() {
   int x = 0;
   static std::once_flag flagn;
 
   std::call_once(flagn, callbackn, x);
 #ifndef EMULATE_LIBCXX03
   clang_analyzer_eval(x == 42); // expected-warning{{TRUE}}
 #endif
 }
+
+int param_passed(int *x) {
+  return *x; // no-warning, as std::function is not working yet.
+}
+
+void callback_taking_func_ok(std::function<void(int*)> &innerCallback) {
+  innerCallback(nullptr);
+}
+
+void callback_with_implicit_cast_ok() {
+  std::once_flag flag;
+  call_once(flag, callback_taking_func_ok, &param_passed);
+}
+
+void callback_taking_func(std::function<void()> &innerCallback) {
+  innerCallback();
+}
+
+void callback_with_implicit_cast() {
+  std::once_flag flag;
+  call_once(flag, callback_taking_func, callback_with_implicit_cast);
+}