[analyzer][NFC] Require explicit matching mode for CallDescriptions (#…

…92454)

This commit deletes the "simple" constructor of `CallDescription` which
did not require a `CallDescription::Mode` argument and always used the
"wildcard" mode `CDM::Unspecified`.

A few months ago, this vague matching mode was used by many checkers,
which caused bugs like #81597
and #88181. Since then, my
commits improved the available matching modes and ensured that all
checkers explicitly specify the right matching mode.

After those commits, the only remaining references to the "simple"
constructor were some unit tests; this commit updates them to use an
explicitly specified matching mode (often `CDM::SimpleFunc`).

The mode `CDM::Unspecified` was not deleted in this commit because it's
still a reasonable choice in `GenericTaintChecker` and a few unit tests.

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

@@ -56,9 +56,10 @@ class CallDescription {
     /// overloaded operator, a constructor or a destructor).
     CXXMethod,
 
-    /// Match any CallEvent that is not an ObjCMethodCall.
-    /// FIXME: Previously this was the default behavior of CallDescription, but
-    /// its use should be replaced by a more specific mode almost everywhere.
+    /// Match any CallEvent that is not an ObjCMethodCall. This should not be
+    /// used when the checker looks for a concrete function (and knows whether
+    /// it is a method); but GenericTaintChecker uses this mode to match
+    /// functions whose name was configured by the user.
     Unspecified,
 
     /// FIXME: Add support for ObjCMethodCall events (I'm not adding it because
@@ -100,13 +101,6 @@ class CallDescription {
                   MaybeCount RequiredArgs = std::nullopt,
                   MaybeCount RequiredParams = std::nullopt);
 
-  /// Construct a CallDescription with default flags.
-  CallDescription(ArrayRef<StringRef> QualifiedName,
-                  MaybeCount RequiredArgs = std::nullopt,
-                  MaybeCount RequiredParams = std::nullopt);
-
-  CallDescription(std::nullptr_t) = delete;
-
   /// Get the name of the function that this object matches.
   StringRef getFunctionName() const { return QualifiedName.back(); }
 

clang/lib/StaticAnalyzer/Core/CallDescription.cpp

@@ -48,13 +48,6 @@ ento::CallDescription::CallDescription(Mode MatchAs,
                   [](StringRef From) { return From.str(); });
 }
 
-/// Construct a CallDescription with default flags.
-ento::CallDescription::CallDescription(ArrayRef<StringRef> QualifiedName,
-                                       MaybeCount RequiredArgs /*= None*/,
-                                       MaybeCount RequiredParams /*= None*/)
-    : CallDescription(Mode::Unspecified, QualifiedName, RequiredArgs,
-                      RequiredParams) {}
-
 bool ento::CallDescription::matches(const CallEvent &Call) const {
   // FIXME: Add ObjC Message support.
   if (Call.getKind() == CE_ObjCMessage)

clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp

@@ -33,11 +33,13 @@ class InterestingnessTestChecker : public Checker<check::PreCall> {
                                        const CallEvent &, CheckerContext &)>;
 
   CallDescriptionMap<HandlerFn> Handlers = {
-      {{{"setInteresting"}, 1}, &InterestingnessTestChecker::handleInteresting},
-      {{{"setNotInteresting"}, 1},
+      {{CDM::SimpleFunc, {"setInteresting"}, 1},
+       &InterestingnessTestChecker::handleInteresting},
+      {{CDM::SimpleFunc, {"setNotInteresting"}, 1},
        &InterestingnessTestChecker::handleNotInteresting},
-      {{{"check"}, 1}, &InterestingnessTestChecker::handleCheck},
-      {{{"bug"}, 1}, &InterestingnessTestChecker::handleBug},
+      {{CDM::SimpleFunc, {"check"}, 1},
+       &InterestingnessTestChecker::handleCheck},
+      {{CDM::SimpleFunc, {"bug"}, 1}, &InterestingnessTestChecker::handleBug},
   };
 
   void handleInteresting(const CallEvent &Call, CheckerContext &C) const;

clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

@@ -138,26 +138,28 @@ class CallDescriptionAction : public ASTFrontendAction {
 TEST(CallDescription, SimpleNameMatching) {
   EXPECT_TRUE(tooling::runToolOnCode(
       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.
+          {{CDM::SimpleFunc, {"bar"}},
+           false}, // false: there's no call to 'bar' in this code.
+          {{CDM::SimpleFunc, {"foo"}},
+           true}, // true: there's a call to 'foo' in this code.
       })),
       "void foo(); void bar() { foo(); }"));
 }
 
 TEST(CallDescription, RequiredArguments) {
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-          {{{"foo"}, 1}, true},
-          {{{"foo"}, 2}, false},
+          {{CDM::SimpleFunc, {"foo"}, 1}, true},
+          {{CDM::SimpleFunc, {"foo"}, 2}, false},
       })),
       "void foo(int); void foo(int, int); void bar() { foo(1); }"));
 }
 
 TEST(CallDescription, LackOfRequiredArguments) {
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-          {{{"foo"}, std::nullopt}, true},
-          {{{"foo"}, 2}, false},
+          {{CDM::SimpleFunc, {"foo"}, std::nullopt}, true},
+          {{CDM::SimpleFunc, {"foo"}, 2}, false},
       })),
       "void foo(int); void foo(int, int); void bar() { foo(1); }"));
 }
@@ -187,7 +189,7 @@ TEST(CallDescription, QualifiedNames) {
   const std::string Code = (Twine{MockStdStringHeader} + AdditionalCode).str();
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-          {{{"std", "basic_string", "c_str"}}, true},
+          {{CDM::CXXMethod, {"std", "basic_string", "c_str"}}, true},
       })),
       Code));
 }
@@ -202,7 +204,8 @@ TEST(CallDescription, MatchConstructor) {
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(
           new CallDescriptionAction<CXXConstructExpr>({
-              {{{"std", "basic_string", "basic_string"}, 2, 2}, true},
+              {{CDM::CXXMethod, {"std", "basic_string", "basic_string"}, 2, 2},
+               true},
           })),
       Code));
 }
@@ -228,7 +231,7 @@ TEST(CallDescription, MatchConversionOperator) {
     })code";
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-          {{{"aaa", "bbb", "Bar", "operator int"}}, true},
+          {{CDM::CXXMethod, {"aaa", "bbb", "Bar", "operator int"}}, true},
       })),
       Code));
 }
@@ -252,7 +255,7 @@ TEST(CallDescription, RejectOverQualifiedNames) {
   // FIXME: We should **not** match.
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-          {{{"std", "container", "data"}}, true},
+          {{CDM::CXXMethod, {"std", "container", "data"}}, true},
       })),
       Code));
 }
@@ -272,7 +275,7 @@ TEST(CallDescription, DontSkipNonInlineNamespaces) {
     SCOPED_TRACE("my v1 bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-            {{{"my", "v1", "bar"}}, true},
+            {{CDM::SimpleFunc, {"my", "v1", "bar"}}, true},
         })),
         Code));
   }
@@ -281,7 +284,7 @@ TEST(CallDescription, DontSkipNonInlineNamespaces) {
     SCOPED_TRACE("my bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-            {{{"my", "bar"}}, true},
+            {{CDM::SimpleFunc, {"my", "bar"}}, true},
         })),
         Code));
   }
@@ -303,15 +306,15 @@ TEST(CallDescription, SkipTopInlineNamespaces) {
     SCOPED_TRACE("my v1 bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-            {{{"my", "v1", "bar"}}, true},
+            {{CDM::SimpleFunc, {"my", "v1", "bar"}}, true},
         })),
         Code));
   }
   {
     SCOPED_TRACE("v1 bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-            {{{"v1", "bar"}}, true},
+            {{CDM::SimpleFunc, {"v1", "bar"}}, true},
         })),
         Code));
   }
@@ -338,7 +341,7 @@ TEST(CallDescription, SkipAnonimousNamespaces) {
 
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-          {{{"std", "container", "data"}}, true},
+          {{CDM::CXXMethod, {"std", "container", "data"}}, true},
       })),
       Code));
 }
@@ -376,7 +379,7 @@ TEST(CallDescription, AliasNames) {
       SCOPED_TRACE("std container data");
       EXPECT_TRUE(tooling::runToolOnCode(
           std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-              {{{"std", "container", "data"}}, true},
+              {{CDM::CXXMethod, {"std", "container", "data"}}, true},
           })),
           UseAliasInSpellingCode));
     }
@@ -385,7 +388,7 @@ TEST(CallDescription, AliasNames) {
       SCOPED_TRACE("std cont data");
       EXPECT_TRUE(tooling::runToolOnCode(
           std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-              {{{"std", "cont", "data"}}, false},
+              {{CDM::CXXMethod, {"std", "cont", "data"}}, false},
           })),
           UseAliasInSpellingCode));
     }
@@ -399,7 +402,7 @@ TEST(CallDescription, AliasNames) {
       SCOPED_TRACE("std container data");
       EXPECT_TRUE(tooling::runToolOnCode(
           std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-              {{{"std", "container", "data"}}, true},
+              {{CDM::CXXMethod, {"std", "container", "data"}}, true},
           })),
           UseAliasInSpellingCode));
     }
@@ -408,7 +411,7 @@ TEST(CallDescription, AliasNames) {
       SCOPED_TRACE("std cont data");
       EXPECT_TRUE(tooling::runToolOnCode(
           std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-              {{{"std", "cont", "data"}}, false},
+              {{CDM::CXXMethod, {"std", "cont", "data"}}, false},
           })),
           UseAliasInSpellingCode));
     }
@@ -431,7 +434,7 @@ TEST(CallDescription, AliasSingleNamespace) {
     SCOPED_TRACE("aaa bbb ccc bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-            {{{"aaa", "bbb", "ccc", "bar"}}, true},
+            {{CDM::SimpleFunc, {"aaa", "bbb", "ccc", "bar"}}, true},
         })),
         Code));
   }
@@ -440,7 +443,7 @@ TEST(CallDescription, AliasSingleNamespace) {
     SCOPED_TRACE("aaa bbb_alias ccc bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-            {{{"aaa", "bbb_alias", "ccc", "bar"}}, false},
+            {{CDM::SimpleFunc, {"aaa", "bbb_alias", "ccc", "bar"}}, false},
         })),
         Code));
   }
@@ -462,7 +465,7 @@ TEST(CallDescription, AliasMultipleNamespaces) {
     SCOPED_TRACE("aaa bbb ccc bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-            {{{"aaa", "bbb", "ccc", "bar"}}, true},
+            {{CDM::SimpleFunc, {"aaa", "bbb", "ccc", "bar"}}, true},
         })),
         Code));
   }
@@ -471,7 +474,7 @@ TEST(CallDescription, AliasMultipleNamespaces) {
     SCOPED_TRACE("aaa_bbb_ccc bar");
     EXPECT_TRUE(tooling::runToolOnCode(
         std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-            {{{"aaa_bbb_ccc", "bar"}}, false},
+            {{CDM::SimpleFunc, {"aaa_bbb_ccc", "bar"}}, false},
         })),
         Code));
   }
@@ -480,9 +483,9 @@ TEST(CallDescription, AliasMultipleNamespaces) {
 TEST(CallDescription, NegativeMatchQualifiedNames) {
   EXPECT_TRUE(tooling::runToolOnCode(
       std::unique_ptr<FrontendAction>(new CallDescriptionAction<>({
-          {{{"foo", "bar"}}, false},
-          {{{"bar", "foo"}}, false},
-          {{{"foo"}}, true},
+          {{CDM::Unspecified, {"foo", "bar"}}, false},
+          {{CDM::Unspecified, {"bar", "foo"}}, false},
+          {{CDM::Unspecified, {"foo"}}, true},
       })),
       "void foo(); struct bar { void foo(); }; void test() { foo(); }"));
 }
@@ -598,7 +601,7 @@ TEST(CallDescription, MatchBuiltins) {
 
 class CallDescChecker
     : public Checker<check::PreCall, check::PreStmt<CallExpr>> {
-  CallDescriptionSet Set = {{{"bar"}, 0}};
+  CallDescriptionSet Set = {{CDM::SimpleFunc, {"bar"}, 0}};
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const {

clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp

@@ -18,7 +18,7 @@ using namespace ento;
 
 namespace {
 class EvalCallBase : public Checker<eval::Call> {
-  const CallDescription Foo = {{"foo"}, 0};
+  const CallDescription Foo = {CDM::SimpleFunc, {"foo"}, 0};
 
 public:
   bool evalCall(const CallEvent &Call, CheckerContext &C) const {

clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp

@@ -30,9 +30,11 @@ class FalsePositiveGenerator : public Checker<eval::Call> {
   using HandlerFn = bool (Self::*)(const CallEvent &Call,
                                    CheckerContext &) const;
   CallDescriptionMap<HandlerFn> Callbacks = {
-      {{{"reachedWithContradiction"}, 0}, &Self::reachedWithContradiction},
-      {{{"reachedWithNoContradiction"}, 0}, &Self::reachedWithNoContradiction},
-      {{{"reportIfCanBeTrue"}, 1}, &Self::reportIfCanBeTrue},
+      {{CDM::SimpleFunc, {"reachedWithContradiction"}, 0},
+       &Self::reachedWithContradiction},
+      {{CDM::SimpleFunc, {"reachedWithNoContradiction"}, 0},
+       &Self::reachedWithNoContradiction},
+      {{CDM::SimpleFunc, {"reportIfCanBeTrue"}, 1}, &Self::reportIfCanBeTrue},
   };
 
   bool report(CheckerContext &C, ProgramStateRef State,

clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp

@@ -38,7 +38,8 @@ class DescriptiveNameChecker : public Checker<check::PreCall> {
 
 private:
   const BugType Bug{this, "DescriptiveNameBug"};
-  const CallDescription HandlerFn = {{"reportDescriptiveName"}, 1};
+  const CallDescription HandlerFn = {
+      CDM::SimpleFunc, {"reportDescriptiveName"}, 1};
 };
 
 void addDescriptiveNameChecker(AnalysisASTConsumer &AnalysisConsumer,

clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp

@@ -86,17 +86,17 @@ class StatefulChecker : public Checker<check::PreCall> {
 
 public:
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const {
-    if (CallDescription{{"preventError"}, 0}.matches(Call)) {
+    if (CallDescription{CDM::SimpleFunc, {"preventError"}, 0}.matches(Call)) {
       C.addTransition(C.getState()->set<ErrorPrevented>(true));
       return;
     }
 
-    if (CallDescription{{"allowError"}, 0}.matches(Call)) {
+    if (CallDescription{CDM::SimpleFunc, {"allowError"}, 0}.matches(Call)) {
       C.addTransition(C.getState()->set<ErrorPrevented>(false));
       return;
     }
 
-    if (CallDescription{{"error"}, 0}.matches(Call)) {
+    if (CallDescription{CDM::SimpleFunc, {"error"}, 0}.matches(Call)) {
       if (C.getState()->get<ErrorPrevented>())
         return;
       const ExplodedNode *N = C.generateErrorNode();