Author: T-Gruber Date: 2025-03-04T17:00:55+01:00 New Revision: 9c542bcf0a1b243dd39c2ecffdd7331c15ae0fb1
URL: https://github.com/llvm/llvm-project/commit/9c542bcf0a1b243dd39c2ecffdd7331c15ae0fb1 DIFF: https://github.com/llvm/llvm-project/commit/9c542bcf0a1b243dd39c2ecffdd7331c15ae0fb1.diff LOG: [analyzer] performTrivialCopy triggers checkLocation before binding (#129016) The triggered callbacks for the default copy constructed instance and the instance used for initialization now behave in the same way. The LHS already calls checkBind. To keep this consistent, checkLocation is now triggered accordingly for the RHS. Further details on the previous discussion: https://discourse.llvm.org/t/checklocation-for-implicitcastexpr-of-kind-ck-noop/84729 --------- Authored-by: tobias.gruber <tobias.gru...@concentrio.io> Added: Modified: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index f7020da2e6da2..ff4fa58857fc6 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -69,6 +69,7 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred, assert(ThisRD); SVal V = Call.getArgSVal(0); + const Expr *VExpr = Call.getArgExpr(0); // If the value being copied is not unknown, load from its location to get // an aggregate rvalue. @@ -76,7 +77,12 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred, V = Pred->getState()->getSVal(*L); else assert(V.isUnknownOrUndef()); - evalBind(Dst, CallExpr, Pred, ThisVal, V, true); + + ExplodedNodeSet Tmp; + evalLocation(Tmp, CallExpr, VExpr, Pred, Pred->getState(), V, + /*isLoad=*/true); + for (ExplodedNode *N : Tmp) + evalBind(Dst, CallExpr, N, ThisVal, V, true); PostStmt PS(CallExpr, LCtx); for (ExplodedNode *N : Dst) { @@ -1199,4 +1205,4 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, ExplodedNode *Pred, // FIXME: Move all post/pre visits to ::Visit(). getCheckerManager().runCheckersForPostStmt(Dst, Tmp, LE, *this); -} +} \ No newline at end of file diff --git a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp index a8579f9d0f90c..b6eeb9ce37386 100644 --- a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp +++ b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp @@ -12,6 +12,8 @@ #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" +#include "llvm/Support/Casting.h" #include "gtest/gtest.h" using namespace clang; @@ -44,6 +46,33 @@ CREATE_EXPR_ENGINE_CHECKER(ExprEngineVisitPreChecker, PreStmt, GCCAsmStmt, CREATE_EXPR_ENGINE_CHECKER(ExprEngineVisitPostChecker, PostStmt, GCCAsmStmt, "GCCAsmStmtBug") +class MemAccessChecker : public Checker<check::Location, check::Bind> { +public: + void checkLocation(const SVal &Loc, bool IsLoad, const Stmt *S, + CheckerContext &C) const { + emitErrorReport(C, Bug, + "checkLocation: Loc = " + dumpToString(Loc) + + ", Stmt = " + S->getStmtClassName()); + } + + void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const { + emitErrorReport(C, Bug, + "checkBind: Loc = " + dumpToString(Loc) + + ", Val = " + dumpToString(Val) + + ", Stmt = " + S->getStmtClassName()); + } + +private: + const BugType Bug{this, "MemAccess"}; + + std::string dumpToString(SVal V) const { + std::string StrBuf; + llvm::raw_string_ostream StrStream{StrBuf}; + V.dumpToStream(StrStream); + return StrBuf; + } +}; + void addExprEngineVisitPreChecker(AnalysisASTConsumer &AnalysisConsumer, AnalyzerOptions &AnOpts) { AnOpts.CheckersAndPackages = {{"ExprEngineVisitPreChecker", true}}; @@ -62,6 +91,15 @@ void addExprEngineVisitPostChecker(AnalysisASTConsumer &AnalysisConsumer, }); } +void addMemAccessChecker(AnalysisASTConsumer &AnalysisConsumer, + AnalyzerOptions &AnOpts) { + AnOpts.CheckersAndPackages = {{"MemAccessChecker", true}}; + AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) { + Registry.addChecker<MemAccessChecker>("MemAccessChecker", "Desc", + "DocsURI"); + }); +} + TEST(ExprEngineVisitTest, checkPreStmtGCCAsmStmt) { std::string Diags; EXPECT_TRUE(runCheckerOnCode<addExprEngineVisitPreChecker>(R"( @@ -84,4 +122,32 @@ TEST(ExprEngineVisitTest, checkPostStmtGCCAsmStmt) { EXPECT_EQ(Diags, "ExprEngineVisitPostChecker: checkPostStmt<GCCAsmStmt>\n"); } +TEST(ExprEngineVisitTest, checkLocationAndBind) { + std::string Diags; + EXPECT_TRUE(runCheckerOnCode<addMemAccessChecker>(R"( + class MyClass{ + public: + int Value; + }; + extern MyClass MyClassWrite, MyClassRead; + void top() { + MyClassWrite = MyClassRead; + } + )", + Diags)); + + std::string LocMsg = "checkLocation: Loc = lazyCompoundVal{0x0,MyClassRead}, " + "Stmt = ImplicitCastExpr"; + std::string BindMsg = + "checkBind: Loc = &MyClassWrite, Val = lazyCompoundVal{0x0,MyClassRead}, " + "Stmt = CXXOperatorCallExpr"; + std::size_t LocPos = Diags.find(LocMsg); + std::size_t BindPos = Diags.find(BindMsg); + EXPECT_NE(LocPos, std::string::npos); + EXPECT_NE(BindPos, std::string::npos); + // Check order: first checkLocation is called, then checkBind. + // In the diagnosis, however, the messages appear in reverse order. + EXPECT_TRUE(LocPos > BindPos); +} + } // namespace _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
