Author: Donát Nagy Date: 2024-05-17T13:08:45+02:00 New Revision: 58bad2862cf136f9483eb005bbfa6915d459b46d
URL: https://github.com/llvm/llvm-project/commit/58bad2862cf136f9483eb005bbfa6915d459b46d DIFF: https://github.com/llvm/llvm-project/commit/58bad2862cf136f9483eb005bbfa6915d459b46d.diff LOG: [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 https://github.com/llvm/llvm-project/issues/81597 and https://github.com/llvm/llvm-project/issues/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. Added: Modified: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h clang/lib/StaticAnalyzer/Core/CallDescription.cpp clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h index ccfe8d47c290b..a99c11766f110 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h +++ b/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(); } diff --git a/clang/lib/StaticAnalyzer/Core/CallDescription.cpp b/clang/lib/StaticAnalyzer/Core/CallDescription.cpp index 0bb0fe66e54ff..cd23b381f879a 100644 --- a/clang/lib/StaticAnalyzer/Core/CallDescription.cpp +++ b/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) diff --git a/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp b/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp index 5f562b1c98b0e..70a58026da95f 100644 --- a/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp +++ b/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; diff --git a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp index 238f954d71331..aed49a1f5f78d 100644 --- a/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp +++ b/clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp @@ -138,8 +138,10 @@ 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(); }")); } @@ -147,8 +149,8 @@ TEST(CallDescription, SimpleNameMatching) { 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); }")); } @@ -156,8 +158,8 @@ TEST(CallDescription, RequiredArguments) { 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,7 +306,7 @@ 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)); } @@ -311,7 +314,7 @@ TEST(CallDescription, SkipTopInlineNamespaces) { 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 { diff --git a/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp b/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp index d3eb4b7a94d32..e410cca076637 100644 --- a/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp +++ b/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 { diff --git a/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp b/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp index 3e12eec549c4e..8f0a96d41e752 100644 --- a/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp +++ b/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, diff --git a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp b/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp index ba0c4d25e13b4..03aee56a200f6 100644 --- a/clang/unittests/StaticAnalyzer/MemRegionDescriptiveNameTest.cpp +++ b/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, diff --git a/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp b/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp index 51aca42a26b05..a9033425dfb51 100644 --- a/clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp +++ b/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(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
