[clang] 5644d15 - [analyzer][NFC] Add unittests for CallDescription and split the old ones

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 18 05:58:10 PDT 2021


Author: Balazs Benics
Date: 2021-10-18T14:57:24+02:00
New Revision: 5644d152578f4604f7dc8c908a0a3f91a726ad80

URL: https://github.com/llvm/llvm-project/commit/5644d152578f4604f7dc8c908a0a3f91a726ad80
DIFF: https://github.com/llvm/llvm-project/commit/5644d152578f4604f7dc8c908a0a3f91a726ad80.diff

LOG: [analyzer][NFC] Add unittests for CallDescription and split the old ones

This NFC change accomplishes three things:
1) Splits up the single unittest into reasonable segments.
2) Extends the test infra using a template to select the AST-node
   from which it is supposed to construct a `CallEvent`.
3) Adds a *lot* of different tests, documenting the current
   capabilities of the `CallDescription`. The corresponding tests are
   marked with `FIXME`s, where the current behavior should be different.

Both `CXXMemberCallExpr` and `CXXOperatorCallExpr` are derived from
`CallExpr`, so they are matched by using the default template parameter.
On the other hand, `CXXConstructExpr` is not derived from `CallExpr`.
In case we want to match for them, we need to pass the type explicitly
to the `CallDescriptionAction`.

About destructors:
They have no AST-node, but they are generated in the CFG machinery in
the analyzer. Thus, to be able to match against them, we would need to
construct a CFG and walk on that instead of simply walking the AST.

I'm also relaxing the `EXPECT`ation in the
`CallDescriptionConsumer::performTest()`, to check the `LookupResult`
only if we matched for the `CallDescription`.
This is necessary to allow tests in which we expect *no* matches at all.

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D111794

Added: 
    

Modified: 
    clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
index 2ebaa46b0f715..e232d0ff065bb 100644
--- a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -8,9 +8,11 @@
 
 #include "Reusables.h"
 
+#include "clang/AST/ExprCXX.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
+#include <type_traits>
 
 namespace clang {
 namespace ento {
@@ -48,25 +50,48 @@ class ResultMap {
 // we were supposed to find ("true" in the provided ResultMap) and that we
 // don't find the ones that we weren't supposed to find
 // ("false" in the ResultMap).
+template <typename MatchedExprT>
 class CallDescriptionConsumer : public ExprEngineConsumer {
   ResultMap &RM;
   void performTest(const Decl *D) {
     using namespace ast_matchers;
+    using T = MatchedExprT;
 
     if (!D->hasBody())
       return;
 
-    const CallExpr *CE = findNode<CallExpr>(D, callExpr());
     const StackFrameContext *SFC =
         Eng.getAnalysisDeclContextManager().getStackFrame(D);
-    ProgramStateRef State = Eng.getInitialState(SFC);
-    CallEventRef<> Call =
-        Eng.getStateManager().getCallEventManager().getCall(CE, State, SFC);
+    const ProgramStateRef State = Eng.getInitialState(SFC);
 
+    // FIXME: Maybe use std::variant and std::visit for these.
+    const auto MatcherCreator = []() {
+      if (std::is_same<T, CallExpr>::value)
+        return callExpr();
+      if (std::is_same<T, CXXConstructExpr>::value)
+        return cxxConstructExpr();
+      if (std::is_same<T, CXXMemberCallExpr>::value)
+        return cxxMemberCallExpr();
+      if (std::is_same<T, CXXOperatorCallExpr>::value)
+        return cxxOperatorCallExpr();
+      llvm_unreachable("Only these expressions are supported for now.");
+    };
+
+    const Expr *E = findNode<T>(D, MatcherCreator());
+
+    CallEventManager &CEMgr = Eng.getStateManager().getCallEventManager();
+    CallEventRef<> Call = [=, &CEMgr]() -> CallEventRef<CallEvent> {
+      if (std::is_base_of<CallExpr, T>::value)
+        return CEMgr.getCall(E, State, SFC);
+      if (std::is_same<T, CXXConstructExpr>::value)
+        return CEMgr.getCXXConstructorCall(cast<CXXConstructExpr>(E),
+                                           /*Target=*/nullptr, State, SFC);
+      llvm_unreachable("Only these expressions are supported for now.");
+    }();
+
+    // If the call actually matched, check if we really expected it to match.
     const bool *LookupResult = RM.lookup(*Call);
-    // Check that we've found the function in the map
-    // with the correct description.
-    EXPECT_TRUE(LookupResult && *LookupResult);
+    EXPECT_TRUE(!LookupResult || *LookupResult);
 
     // ResultMap is responsible for making sure that we've found *all* calls.
   }
@@ -83,6 +108,7 @@ class CallDescriptionConsumer : public ExprEngineConsumer {
   }
 };
 
+template <typename MatchedExprT = CallExpr>
 class CallDescriptionAction : public ASTFrontendAction {
   ResultMap RM;
 
@@ -93,63 +119,370 @@ class CallDescriptionAction : public ASTFrontendAction {
 
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
                                                  StringRef File) override {
-    return std::make_unique<CallDescriptionConsumer>(Compiler, RM);
+    return std::make_unique<CallDescriptionConsumer<MatchedExprT>>(Compiler,
+                                                                   RM);
   }
 };
 
-TEST(CallEvent, CallDescription) {
-  // Test simple name matching.
+TEST(CallDescription, SimpleNameMatching) {
   EXPECT_TRUE(tooling::runToolOnCode(
-      std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
+      std::unique_ptr<FrontendAction>(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(CallDescription, RequiredArguments) {
   EXPECT_TRUE(tooling::runToolOnCode(
-      std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
+      std::unique_ptr<FrontendAction>(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(CallDescription, LackOfRequiredArguments) {
   EXPECT_TRUE(tooling::runToolOnCode(
-      std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
+      std::unique_ptr<FrontendAction>(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); }"));
+}
+
+constexpr StringRef MockStdStringHeader = R"code(
+  namespace std { inline namespace __1 {
+    template<typename T> class basic_string {
+      class Allocator {};
+    public:
+      basic_string();
+      explicit basic_string(const char*, const Allocator & = Allocator());
+      ~basic_string();
+      T *c_str();
+    };
+  } // namespace __1
+  using string = __1::basic_string<char>;
+  } // namespace std
+)code";
 
-  // Test qualified names.
+TEST(CallDescription, QualifiedNames) {
+  constexpr StringRef AdditionalCode = R"code(
+    void foo() {
+      using namespace std;
+      basic_string<char> s;
+      s.c_str();
+    })code";
+  const std::string Code = (Twine{MockStdStringHeader} + AdditionalCode).str();
   EXPECT_TRUE(tooling::runToolOnCode(
-      std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
+      std::unique_ptr<FrontendAction>(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();"
-      "}"));
+      Code));
+}
 
-  // A negative test for qualified names.
+TEST(CallDescription, MatchConstructor) {
+  constexpr StringRef AdditionalCode = R"code(
+    void foo() {
+      using namespace std;
+      basic_string<char> s("hello");
+    })code";
+  // FIXME: We should match.
+  const std::string Code = (Twine{MockStdStringHeader} + AdditionalCode).str();
   EXPECT_TRUE(tooling::runToolOnCode(
-      std::unique_ptr<CallDescriptionAction>(new CallDescriptionAction({
+      std::unique_ptr<FrontendAction>(
+          new CallDescriptionAction<CXXConstructExpr>({
+              {{{"std", "basic_string", "basic_string"}, 2, 2}, false},
+          })),
+      Code));
+}
+
+// FIXME: Test matching destructors: {"std", "basic_string", "~basic_string"}
+//        This feature is actually implemented, but the test infra is not yet
+//        sophisticated enough for testing this. To do that, we will need to
+//        implement a much more advanced dispatching mechanism using the CFG for
+//        the implicit destructor events.
+
+TEST(CallDescription, MatchConversionOperator) {
+  constexpr StringRef Code = R"code(
+    namespace aaa {
+    namespace bbb {
+    struct Bar {
+      operator int();
+    };
+    } // bbb
+    } // aaa
+    void foo() {
+      aaa::bbb::Bar x;
+      int tmp = x;
+    })code";
+  // FIXME: We should match.
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+          {{{"aaa", "bbb", "Bar", "operator int"}}, false},
+      })),
+      Code));
+}
+
+TEST(CallDescription, RejectOverQualifiedNames) {
+  constexpr auto Code = R"code(
+    namespace my {
+    namespace std {
+      struct container {
+        const char *data() const;
+      };
+    } // namespace std
+    } // namespace my
+
+    void foo() {
+      using namespace my;
+      std::container v;
+      v.data();
+    })code";
+
+  // FIXME: We should **not** match.
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+          {{{"std", "container", "data"}}, true},
+      })),
+      Code));
+}
+
+TEST(CallDescription, DontSkipNonInlineNamespaces) {
+  constexpr auto Code = R"code(
+    namespace my {
+    /*not inline*/ namespace v1 {
+      void bar();
+    } // namespace v1
+    } // namespace my
+    void foo() {
+      my::v1::bar();
+    })code";
+
+  {
+    SCOPED_TRACE("my v1 bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"my", "v1", "bar"}}, true},
+        })),
+        Code));
+  }
+  {
+    // FIXME: We should **not** skip non-inline namespaces.
+    SCOPED_TRACE("my bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"my", "bar"}}, true},
+        })),
+        Code));
+  }
+}
+
+TEST(CallDescription, SkipTopInlineNamespaces) {
+  constexpr auto Code = R"code(
+    inline namespace my {
+    namespace v1 {
+      void bar();
+    } // namespace v1
+    } // namespace my
+    void foo() {
+      using namespace v1;
+      bar();
+    })code";
+
+  {
+    SCOPED_TRACE("my v1 bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"my", "v1", "bar"}}, true},
+        })),
+        Code));
+  }
+  {
+    SCOPED_TRACE("v1 bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"v1", "bar"}}, true},
+        })),
+        Code));
+  }
+}
+
+TEST(CallDescription, SkipAnonimousNamespaces) {
+  constexpr auto Code = R"code(
+    namespace {
+    namespace std {
+    namespace {
+    inline namespace {
+      struct container {
+        const char *data() const { return nullptr; };
+      };
+    } // namespace inline anonymous
+    } // namespace anonymous
+    } // namespace std
+    } // namespace anonymous
+
+    void foo() {
+      std::container v;
+      v.data();
+    })code";
+
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+          {{{"std", "container", "data"}}, true},
+      })),
+      Code));
+}
+
+TEST(CallDescription, AliasNames) {
+  constexpr StringRef AliasNamesCode = R"code(
+  namespace std {
+    struct container {
+      const char *data() const;
+    };
+    using cont = container;
+  } // std
+)code";
+
+  constexpr StringRef UseAliasInSpelling = R"code(
+    void foo() {
+      std::cont v;
+      v.data();
+    })code";
+  constexpr StringRef UseStructNameInSpelling = R"code(
+    void foo() {
+      std::container v;
+      v.data();
+    })code";
+  const std::string UseAliasInSpellingCode =
+      (Twine{AliasNamesCode} + UseAliasInSpelling).str();
+  const std::string UseStructNameInSpellingCode =
+      (Twine{AliasNamesCode} + UseStructNameInSpelling).str();
+
+  // Test if the code spells the alias, wile we match against the struct name,
+  // and again matching against the alias.
+  {
+    SCOPED_TRACE("Using alias in spelling");
+    {
+      SCOPED_TRACE("std container data");
+      EXPECT_TRUE(tooling::runToolOnCode(
+          std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+              {{{"std", "container", "data"}}, true},
+          })),
+          UseAliasInSpellingCode));
+    }
+    {
+      // FIXME: We should be able to see-through aliases.
+      SCOPED_TRACE("std cont data");
+      EXPECT_TRUE(tooling::runToolOnCode(
+          std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+              {{{"std", "cont", "data"}}, false},
+          })),
+          UseAliasInSpellingCode));
+    }
+  }
+
+  // Test if the code spells the struct name, wile we match against the struct
+  // name, and again matching against the alias.
+  {
+    SCOPED_TRACE("Using struct name in spelling");
+    {
+      SCOPED_TRACE("std container data");
+      EXPECT_TRUE(tooling::runToolOnCode(
+          std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+              {{{"std", "container", "data"}}, true},
+          })),
+          UseAliasInSpellingCode));
+    }
+    {
+      // FIXME: We should be able to see-through aliases.
+      SCOPED_TRACE("std cont data");
+      EXPECT_TRUE(tooling::runToolOnCode(
+          std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+              {{{"std", "cont", "data"}}, false},
+          })),
+          UseAliasInSpellingCode));
+    }
+  }
+}
+
+TEST(CallDescription, AliasSingleNamespace) {
+  constexpr StringRef Code = R"code(
+    namespace aaa {
+    namespace bbb {
+    namespace ccc {
+      void bar();
+    }} // namespace bbb::ccc
+    namespace bbb_alias = bbb;
+    } // namespace aaa
+    void foo() {
+      aaa::bbb_alias::ccc::bar();
+    })code";
+  {
+    SCOPED_TRACE("aaa bbb ccc bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"aaa", "bbb", "ccc", "bar"}}, true},
+        })),
+        Code));
+  }
+  {
+    // FIXME: We should be able to see-through namespace aliases.
+    SCOPED_TRACE("aaa bbb_alias ccc bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"aaa", "bbb_alias", "ccc", "bar"}}, false},
+        })),
+        Code));
+  }
+}
+
+TEST(CallDescription, AliasMultipleNamespaces) {
+  constexpr StringRef Code = R"code(
+    namespace aaa {
+    namespace bbb {
+    namespace ccc {
+      void bar();
+    }}} // namespace aaa::bbb::ccc
+    namespace aaa_bbb_ccc = aaa::bbb::ccc;
+    void foo() {
+      using namespace aaa_bbb_ccc;
+      bar();
+    })code";
+  {
+    SCOPED_TRACE("aaa bbb ccc bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"aaa", "bbb", "ccc", "bar"}}, true},
+        })),
+        Code));
+  }
+  {
+    // FIXME: We should be able to see-through namespace aliases.
+    SCOPED_TRACE("aaa_bbb_ccc bar");
+    EXPECT_TRUE(tooling::runToolOnCode(
+        std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
+            {{{"aaa_bbb_ccc", "bar"}}, false},
+        })),
+        Code));
+  }
+}
+
+TEST(CallDescription, NegativeMatchQualifiedNames) {
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::unique_ptr<FrontendAction>(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(CallDescription, MatchBuiltins) {
   // 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<FrontendAction>(new CallDescriptionAction<>(
+          {{{"memset", 3}, false}, {{CDF_MaybeBuiltin, "memset", 3}, true}})),
       "void foo() {"
       "  int x;"
       "  __builtin___memset_chk(&x, 0, sizeof(x),"


        


More information about the cfe-commits mailing list