[analyzer] CallDescriptionMap: Support CXXConstructExpr

[Charusso]

Summary

-

[Charusso]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

384

dyn_cast, whoops.

[Charusso]

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

176

Weird formatting.

[NoQ]

This is very handy, thank you!

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

388–391

In any case, i'd prefer ND->getNameAsString() here.

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

175

Let's also test {"std", "basic_string", "basic_string"}.

214

What do you think about having a constructor override for CallDescription that accepts an OverloadedOperatorKind? That'll be faster and more type-safe than matching operator name as string.

[Charusso]

• Refactor.

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

388–391

+1, but the API is that smart: assert(Name.isIdentifier() && "Name is not a simple identifier");. However, we have ND->getDeclName().getAsString() which is equivalent to dumping to the stream (literally) and handles unnamed declarations, my bad. I have learnt from @aaron.ballman dumping to a stream is safe and I believed this is the only solution.

@aaron.ballman, what if we introduce getNameMayUnnamed() for convenient use? It would immediately ring the bells to use the right API.

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

214

Great idea, thanks! Added in: D81059

[NoQ]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

388–391

assert(Name.isIdentifier() && "Name is not a simple identifier");

It's in getName(), not in getNameAsString(). The latter always succeeds.

[Charusso]

• Using getNameAsString().

[Charusso]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

388–391

Hm, I have miss read the API, whoops. It is being deprecated 11 years ago and I have believed it delegates the work on getName(). Also @aaron.ballman believed Out << *ND is so cool, meanwhile it is the same as getNameAsString(). So much mystery around that API, facepalm. Thanks!

[NoQ]

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

366

Why did you suddenly want to remove this cast? CXXConstructorDecl is still a FunctionDecl.

Wait, why do we even need a patch for this? Didn't it already work out of the box previously?

387

I agree this && FD was unnecessary though :D

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

 
 class CallEvent;
 class CallDescription;
 
 template<typename T = CallEvent>
 class CallEventRef : public IntrusiveRefCntPtr<const T> {
 public:
   CallEventRef(const T *Call) : IntrusiveRefCntPtr<const T>(Call) {}
-  CallEventRef(const CallEventRef &Orig) : IntrusiveRefCntPtr<const T>(Orig) {}
+  CallEventRef(const CallEventRef &Orig) = default;
 
   CallEventRef<T> cloneWithState(ProgramStateRef State) const {
     return this->get()->template cloneWithState<T>(State);
   }
 
   // Allow implicit conversions to a superclass type, since CallEventRef
   // behaves like a pointer-to-const.
   template <typename SuperT>

clang/lib/StaticAnalyzer/Core/CallEvent.cpp

ProgramPoint CallEvent::getProgramPoint(bool IsPreVisit,
   return PostImplicitCall(D, Loc, getLocationContext(), Tag);
 }
 
 bool CallEvent::isCalled(const CallDescription &CD) const {
   // FIXME: Add ObjC Message support.
   if (getKind() == CE_ObjCMessage)
     return false;
 
-  const IdentifierInfo *II = getCalleeIdentifier();
-  if (!II)
-    return false;
-  const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(getDecl());
-  if (!FD)
+  if (!getDecl())
     return false;
 
   if (CD.Flags & CDF_MaybeBuiltin) {
+    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(getDecl()))
       return CheckerContext::isCLibraryFunction(FD, CD.getFunctionName()) &&
              (!CD.RequiredArgs || CD.RequiredArgs <= getNumArgs()) &&
              (!CD.RequiredParams || CD.RequiredParams <= parameters().size());
+    return false;
   }
 
   if (!CD.IsLookupDone) {
     CD.IsLookupDone = true;
     CD.II = &getState()->getStateManager().getContext().Idents.get(
         CD.getFunctionName());
   }
 
+  if (const IdentifierInfo *II = getCalleeIdentifier()) {
     if (II != CD.II)
       return false;
+  } else {
+    const auto *ND = dyn_cast<NamedDecl>(getDecl());
+    if (!ND)
+      return false;
+
+    if (CD.getFunctionName() != ND->getNameAsString())
+      return false;
+  }
 
   // If CallDescription provides prefix names, use them to improve matching
   // accuracy.
-  if (CD.QualifiedName.size() > 1 && FD) {
-    const DeclContext *Ctx = FD->getDeclContext();
+  if (CD.QualifiedName.size() > 1) {
+    const DeclContext *Ctx = getDecl()->getDeclContext();
     // See if we'll be able to match them all.
     size_t NumUnmatched = CD.QualifiedName.size() - 1;
     for (; Ctx && isa<NamedDecl>(Ctx); Ctx = Ctx->getParent()) {
       if (NumUnmatched == 0)
         break;
 
       if (const auto *ND = dyn_cast<NamespaceDecl>(Ctx)) {
         if (ND->getName() == CD.QualifiedName[NumUnmatched - 1])

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

 class CallDescriptionConsumer : public ExprEngineConsumer {
   ResultMap &RM;
   void performTest(const Decl *D) {
     using namespace ast_matchers;
 
     if (!D->hasBody())
       return;
 
-    const CallExpr *CE = findNode<CallExpr>(D, callExpr());
+    const Expr *E =
+        findNode<Expr>(D, expr(anyOf(callExpr(), cxxConstructExpr())));
     const StackFrameContext *SFC =
         Eng.getAnalysisDeclContextManager().getStackFrame(D);
     ProgramStateRef State = Eng.getInitialState(SFC);
-    CallEventRef<> Call =
-        Eng.getStateManager().getCallEventManager().getCall(CE, State, SFC);
+    CallEventManager &CEMgr = Eng.getStateManager().getCallEventManager();
+
+    CallEventRef<> Call = isa<CallExpr>(E)
+                              ? (CallEventRef<>)CEMgr.getCall(E, State, SFC)
+                              : (CallEventRef<>)CEMgr.getCXXConstructorCall(
+                                    cast<CXXConstructExpr>(E),
+                                    /*Target=*/nullptr, State, SFC);
 
     const bool *LookupResult = RM.lookup(*Call);
     // Check that we've found the function in the map
     // with the correct description.
     EXPECT_TRUE(LookupResult && *LookupResult);
 
     // ResultMap is responsible for making sure that we've found *all* calls.
   }
 
 public:
-  CallDescriptionConsumer(CompilerInstance &C,
-                          ResultMap &RM)
+  CallDescriptionConsumer(CompilerInstance &C, ResultMap &RM)
       : ExprEngineConsumer(C), RM(RM) {}
 
   bool HandleTopLevelDecl(DeclGroupRef DG) override {
     for (const auto *D : DG)
       performTest(D);
     return true;
   }
 };
 
 class CallDescriptionAction : public ASTFrontendAction {
   ResultMap RM;
 
 public:
   CallDescriptionAction(
       std::initializer_list<std::pair<CallDescription, bool>> Data)
       : RM(Data) {}
 
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
                                                  StringRef File) override {
     return std::make_unique<CallDescriptionConsumer>(Compiler, RM);
   }
 };
 
-TEST(CallEvent, CallDescription) {
-  // Test simple name matching.
+TEST(CallDescriptionMap, SimpleName) {
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
           {{"bar"}, false}, // false: there's no call to 'bar' in this code.
           {{"foo"}, true},  // true: there's a call to 'foo' in this code.
-      })), "void foo(); void bar() { foo(); }"));
+      })),
+      "void foo(); void bar() { foo(); }"));
+}
 
-  // Test arguments check.
+TEST(CallDescriptionMap, HasArgument) {
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
           {{"foo", 1}, true},
           {{"foo", 2}, false},
-      })), "void foo(int); void foo(int, int); void bar() { foo(1); }"));
+      })),
+      "void foo(int); void foo(int, int); void bar() { foo(1); }"));
+}
 
-  // Test lack of arguments check.
+TEST(CallDescriptionMap, NoArgument) {
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
           {{"foo", None}, true},
           {{"foo", 2}, false},
-      })), "void foo(int); void foo(int, int); void bar() { foo(1); }"));
+      })),
+      "void foo(int); void foo(int, int); void bar() { foo(1); }"));
+}
 
-  // Test qualified names.
+TEST(CallDescriptionMap, QualifiedName) {
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
           {{{"std", "basic_string", "c_str"}}, true},
       })),
       "namespace std { inline namespace __1 {"
       "  template<typename T> class basic_string {"
       "  public:"
       "    T *c_str();"
       "  };"
       "}}"
       "void foo() {"
       "  using namespace std;"
       "  basic_string<char> s;"
       "  s.c_str();"
       "}"));
+}
 
-  // A negative test for qualified names.
+TEST(CallDescriptionMap, NonQualifiedName) {
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
           {{{"foo", "bar"}}, false},
           {{{"bar", "foo"}}, false},
           {{"foo"}, true},
-      })), "void foo(); struct bar { void foo(); }; void test() { foo(); }"));
+      })),
+      "void foo(); struct bar { void foo(); }; void test() { foo(); }"));
+}
 
+TEST(CallDescriptionMap, MaybeBuiltin) {
   // Test CDF_MaybeBuiltin - a flag that allows matching weird builtins.
   EXPECT_TRUE(tooling::runToolOnCode(
-      std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
-          {{"memset", 3}, false},
-          {{CDF_MaybeBuiltin, "memset", 3}, true}
-      })),
+      std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction(
+          {{{"memset", 3}, false}, {{CDF_MaybeBuiltin, "memset", 3}, true}})),
       "void foo() {"
       "  int x;"
       "  __builtin___memset_chk(&x, 0, sizeof(x),"
       "                         __builtin_object_size(&x, 0));"
       "}"));
 }
 
+TEST(CallDescriptionMap, CXXConstructExpr) {
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
+          {{{"std", "basic_string"}, 1}, true},
+          {{{"std", "basic_string"}, 0}, false},
+      })),
+      "namespace std { inline namespace __1 {"
+      "  template<typename T> class basic_string {"
+      "  public:"
+      "    basic_string();"
+      "    basic_string(const T *s);"
+      "  };"
+      "}}"
+      "void foo(const char *str) {"
+      "  using namespace std;"
+      "  basic_string<char> s(str);"
+      "}"));
+}
+
+TEST(CallDescriptionMap, CXXConstructExprFullyQualified) {
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
+          {{{"std", "basic_string", "basic_string"}}, true},
+      })),
+      "namespace std { inline namespace __1 {"
+      "  template<typename T> class basic_string {"
+      "  public:"
+      "    basic_string();"
+      "    basic_string(const T *s);"
+      "  };"
+      "}}"
+      "void foo(const char *str) {"
+      "  using namespace std;"
+      "  basic_string<char> s(str);"
+      "}"));
+}
+
+TEST(CallDescriptionMap, CXXConstructExprTypedef) {
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
+          {{{"std", "string"}}, false},
+          {{{"std", "basic_string"}}, true},
+      })),
+      "namespace std { inline namespace __1 {"
+      "  template<typename T> class basic_string {"
+      "  public:"
+      "    basic_string();"
+      "  };"
+      "  typedef basic_string<char> string;"
+      "}}"
+      "void foo() {"
+      "  using namespace std;"
+      "  std::string s;"
+      "}"));
+}
+
 } // namespace
 } // namespace ento
 } // namespace clang