[Analyzer] Do not segfault on unexpected call_once implementation

[george.karpenkov]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=34869

@dcoughlin Any advice on how to handle different stdlib implementations?
Can we conjure a separate symbol instead of relying on a particular struct layout?
For now this implementation will simply not go inside a differently implemented call_once.

[xazax.hun]

Did you link the correct bug in the description? The one you linked was closed long ago.

[george.karpenkov]

Ooops, updated to https://bugs.llvm.org/show_bug.cgi?id=34869

[dcoughlin]

@dcoughlin Any advice on how to handle different stdlib implementations?
Can we conjure a separate symbol instead of relying on a particular struct layout?
For now this implementation will simply not go inside a differently implemented call_once.

I think that for now your solution is the best to avoid the crashes. Let's see what Alexander has to say about the standard library causing the crashes. Ideally, we don't want to fall down too hard on libstdc++.

If we really need to handle a variety of standard libraries (or versions of standard libraries) we'll probably want to to treat std::call_once more abstractly and write a checker that models its behavior instead of body farming it.

lib/Analysis/BodyFarm.cpp

365 ↗

(On Diff #118258)

LLVM style is to write this null check as if (!FlagCXXDecl).

369 ↗

(On Diff #118258)

This return will leak the allocated AST nodes (as will the return for __state__ below). Can you hoist the validation checks to above the AST creation?

[george.karpenkov]

Review comments.

[dcoughlin]

Looks good to me!

cfe/trunk/lib/Analysis/BodyFarm.cpp

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();
+  QualType FlagType = Flag->getType().getNonReferenceType();
+  CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl();
+  if (!FlagCXXDecl) {
+    DEBUG(llvm::dbgs() << "Flag field is not a CXX record: "
+                       << "unknown std::call_once implementation."
+                       << "Ignoring the call.\n");
+    return nullptr;
+  }
+
+  // Note: here we are assuming libc++ implementation of call_once,
+  // which has a struct with a field `__state_`.
+  // Body farming might not work for other `call_once` implementations.
+  NamedDecl *FoundDecl = M.findMemberField(FlagCXXDecl, "__state_");
+  ValueDecl *FieldDecl;
+  if (FoundDecl) {
+    FieldDecl = dyn_cast<ValueDecl>(FoundDecl);
+  } else {
+    DEBUG(llvm::dbgs() << "No field __state_ found on std::once_flag struct, "
+                       << "unable to synthesize call_once body, ignoring "
+                       << "the call.\n");
+    return nullptr;
+  }
 
   bool isLambdaCall = CallbackType->getAsCXXRecordDecl() &&
                       CallbackType->getAsCXXRecordDecl()->isLambda();
 
   SmallVector<Expr *, 5> CallArgs;
 
   if (isLambdaCall)
     // Lambda requires callback itself inserted as a first parameter.
if (isLambdaCall) {
     CallbackCall =
         create_call_once_lambda_call(C, M, Callback, CallbackType, CallArgs);
   } else {
 
     // Function pointer case.
     CallbackCall = create_call_once_funcptr_call(C, M, Callback, CallArgs);
   }
 
-  QualType FlagType = Flag->getType().getNonReferenceType();
   DeclRefExpr *FlagDecl =
       M.makeDeclRefExpr(Flag,
                         /* RefersToEnclosingVariableOrCapture=*/true,
                         /* GetNonReferenceType=*/true);
 
-  CXXRecordDecl *FlagCXXDecl = FlagType->getAsCXXRecordDecl();
-
-  // Note: here we are assuming libc++ implementation of call_once,
-  // which has a struct with a field `__state_`.
-  // Body farming might not work for other `call_once` implementations.
-  NamedDecl *FoundDecl = M.findMemberField(FlagCXXDecl, "__state_");
-  ValueDecl *FieldDecl;
-  if (FoundDecl) {
-    FieldDecl = dyn_cast<ValueDecl>(FoundDecl);
-  } else {
-    DEBUG(llvm::dbgs() << "No field __state_ found on std::once_flag struct, "
-                       << "unable to synthesize call_once body, ignoring "
-                       << "the call.\n");
-    return nullptr;
-  }
 
   MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FieldDecl);
   assert(Deref->isLValue());
   QualType DerefType = Deref->getType();
 
   // Negation predicate.
   UnaryOperator *FlagCheck = new (C) UnaryOperator(
       /* input= */

cfe/trunk/test/Analysis/call_once.cpp

 int call_once() {
   return 5;
 }
 
 void test_non_std_call_once() {
   int x = call_once();
   clang_analyzer_eval(x == 5); // expected-warning{{TRUE}}
 }
+
+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
+}