This revision was automatically updated to reflect the committed changes. steakhal marked 2 inline comments as done. Closed by commit rG9b5c9c469d90: [analyzer] Dump checker name if multiple checkers evaluate the same call (authored by steakhal). Herald added a project: clang. Herald added a subscriber: cfe-commits.
Changed prior to commit: https://reviews.llvm.org/D112889?vs=383659&id=384067#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112889/new/ https://reviews.llvm.org/D112889 Files: clang/lib/StaticAnalyzer/Core/CallEvent.cpp clang/lib/StaticAnalyzer/Core/CheckerManager.cpp clang/unittests/StaticAnalyzer/CMakeLists.txt clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
Index: clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp =================================================================== --- /dev/null +++ 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<eval::Call> { + 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<EvalCallFoo1>("test.EvalFoo1", "EmptyDescription", + "EmptyDocsUri"); + Registry.addChecker<EvalCallFoo2>("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<addEvalFooCheckers>(Code), Regex); +} Index: clang/unittests/StaticAnalyzer/CMakeLists.txt =================================================================== --- clang/unittests/StaticAnalyzer/CMakeLists.txt +++ clang/unittests/StaticAnalyzer/CMakeLists.txt @@ -8,6 +8,7 @@ BugReportInterestingnessTest.cpp CallDescriptionTest.cpp CallEventTest.cpp + ConflictingEvalCallsTest.cpp FalsePositiveRefutationBRVisitorTest.cpp NoStateChangeFuncVisitorTest.cpp ParamRegionTest.cpp Index: clang/lib/StaticAnalyzer/Core/CheckerManager.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/CheckerManager.cpp +++ 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 <cassert> #include <vector> @@ -655,7 +656,7 @@ ExprEngine &Eng, const EvalCallOptions &CallOpts) { for (auto *const Pred : Src) { - bool anyEvaluated = false; + Optional<CheckerNameRef> 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); } Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/CallEvent.cpp +++ 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; }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
