diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp --- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp @@ -411,7 +411,6 @@ ASTContext &Ctx = getState()->getStateManager().getContext(); if (const Expr *E = getOriginExpr()) { E->printPretty(Out, nullptr, Ctx.getPrintingPolicy()); - Out << "\n"; return; } diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp --- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp +++ b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp @@ -26,6 +26,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/Support/Casting.h" #include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/FormatVariadic.h" #include #include @@ -655,7 +656,7 @@ ExprEngine &Eng, const EvalCallOptions &CallOpts) { for (auto *const Pred : Src) { - bool anyEvaluated = false; + Optional evaluatorChecker; ExplodedNodeSet checkDst; NodeBuilder B(Pred, checkDst, Eng.getBuilderContext()); @@ -674,10 +675,26 @@ CheckerContext C(B, Eng, Pred, L); evaluated = EvalCallChecker(Call, C); } - assert(!(evaluated && anyEvaluated) - && "There are more than one checkers evaluating the call"); +#ifndef NDEBUG + if (evaluated && evaluatorChecker) { + const auto toString = [](const CallEvent &Call) -> std::string { + std::string Buf; + llvm::raw_string_ostream OS(Buf); + Call.dump(OS); + OS.flush(); + return Buf; + }; + std::string AssertionMessage = llvm::formatv( + "The '{0}' call has been already evaluated by the {1} checker, " + "while the {2} checker also tried to evaluate the same call. At " + "most one checker supposed to evaluate a call.", + toString(Call), evaluatorChecker->getName(), + EvalCallChecker.Checker->getCheckerName()); + llvm_unreachable(AssertionMessage.c_str()); + } +#endif if (evaluated) { - anyEvaluated = true; + evaluatorChecker = EvalCallChecker.Checker->getCheckerName(); Dst.insert(checkDst); #ifdef NDEBUG break; // on release don't check that no other checker also evals. @@ -686,7 +703,7 @@ } // If none of the checkers evaluated the call, ask ExprEngine to handle it. - if (!anyEvaluated) { + if (!evaluatorChecker) { NodeBuilder B(Pred, Dst, Eng.getBuilderContext()); Eng.defaultEvalCall(B, Pred, Call, CallOpts); } diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt b/clang/unittests/StaticAnalyzer/CMakeLists.txt --- a/clang/unittests/StaticAnalyzer/CMakeLists.txt +++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt @@ -8,6 +8,7 @@ BugReportInterestingnessTest.cpp CallDescriptionTest.cpp CallEventTest.cpp + ConflictingEvalCallsTest.cpp FalsePositiveRefutationBRVisitorTest.cpp NoStateChangeFuncVisitorTest.cpp ParamRegionTest.cpp diff --git a/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp b/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp new file mode 100644 --- /dev/null +++ b/clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp @@ -0,0 +1,58 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "CheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace ento; + +namespace { +class EvalCallBase : public Checker { + const CallDescription Foo = {"foo", 0}; + +public: + bool evalCall(const CallEvent &Call, CheckerContext &C) const { + return Call.isCalled(Foo); + } +}; + +class EvalCallFoo1 : public EvalCallBase {}; +class EvalCallFoo2 : public EvalCallBase {}; +void addEvalFooCheckers(AnalysisASTConsumer &AnalysisConsumer, + AnalyzerOptions &AnOpts) { + AnOpts.CheckersAndPackages = {{"test.EvalFoo1", true}, + {"test.EvalFoo2", true}}; + AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) { + Registry.addChecker("test.EvalFoo1", "EmptyDescription", + "EmptyDocsUri"); + Registry.addChecker("test.EvalFoo2", "EmptyDescription", + "EmptyDocsUri"); + }); +} +} // namespace + +TEST(EvalCall, DetectConflictingEvalCalls) { +#ifdef NDEBUG + GTEST_SKIP() << "This test is only available for debug builds."; +#endif + const std::string Code = R"code( + void foo(); + void top() { + foo(); // crash + } + )code"; + constexpr auto Regex = + "The 'foo\\(\\)' call has been already evaluated by the test\\.EvalFoo1 " + "checker, while the test\\.EvalFoo2 checker also tried to evaluate the " + "same call\\. At most one checker supposed to evaluate a call\\."; + ASSERT_DEATH(runCheckerOnCode(Code), Regex); +}