https://github.com/fangyi-zhou updated https://github.com/llvm/llvm-project/pull/137182
>From 79e5875e75d46edcf15c5df536ac8f1d93e13a16 Mon Sep 17 00:00:00 2001 From: Fangyi Zhou <m...@fangyi.io> Date: Thu, 24 Apr 2025 15:12:12 +0100 Subject: [PATCH 1/2] [clang][analyzer][NFC] Add a helper for conjuring symbols at call events Per suggestion in https://github.com/llvm/llvm-project/pull/128251#discussion_r2055916229, adding a new helper function in `SValBuilder` to conjure a symbol when given a `CallEvent`. Tested manually (with assertions) that the `LocationContext *` obtained from the `CallEvent` are identical to those passed in the original argument. --- .../Core/PathSensitive/SValBuilder.h | 8 ++++- .../Checkers/CStringChecker.cpp | 32 +++++++------------ .../Checkers/ErrnoTesterChecker.cpp | 3 +- .../RetainCountChecker/RetainCountChecker.cpp | 3 +- .../Checkers/SmartPtrModeling.cpp | 2 +- .../Checkers/StdLibraryFunctionsChecker.cpp | 10 +++--- .../Checkers/cert/InvalidPtrChecker.cpp | 4 +-- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp | 25 +++++++++------ 8 files changed, 42 insertions(+), 45 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h index 54430d426a82a..3f3e6bdb9ff3d 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h @@ -46,10 +46,10 @@ class Stmt; namespace ento { +class CallEvent; class ConditionTruthVal; class ProgramStateManager; class StoreRef; - class SValBuilder { virtual void anchor(); @@ -209,6 +209,12 @@ class SValBuilder { const LocationContext *LCtx, QualType type, unsigned visitCount); + DefinedOrUnknownSVal conjureSymbolVal(const CallEvent &call, QualType type, + unsigned visitCount, + const void *symbolTag = nullptr); + DefinedOrUnknownSVal conjureSymbolVal(const CallEvent &call, + unsigned visitCount, + const void *symbolTag = nullptr); /// Conjure a symbol representing heap allocated memory region. /// diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp index 39dcaf02dbe25..9d3eda8f7f982 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -1514,8 +1514,7 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallEvent &Call, // If we don't know how much we copied, we can at least // conjure a return value for later. if (lastElement.isUnknown()) - lastElement = C.getSValBuilder().conjureSymbolVal( - nullptr, Call.getOriginExpr(), LCtx, C.blockCount()); + lastElement = C.getSValBuilder().conjureSymbolVal(Call, C.blockCount()); // The byte after the last byte copied is the return value. state = state->BindExpr(Call.getOriginExpr(), LCtx, lastElement); @@ -1665,8 +1664,7 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallEvent &Call, State = CheckBufferAccess(C, State, Left, Size, AccessKind::read, CK); if (State) { // The return value is the comparison result, which we don't know. - SVal CmpV = Builder.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx, - C.blockCount()); + SVal CmpV = Builder.conjureSymbolVal(Call, C.blockCount()); State = State->BindExpr(Call.getOriginExpr(), LCtx, CmpV); C.addTransition(State); } @@ -1769,8 +1767,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, // no guarantee the full string length will actually be returned. // All we know is the return value is the min of the string length // and the limit. This is better than nothing. - result = C.getSValBuilder().conjureSymbolVal( - nullptr, Call.getOriginExpr(), LCtx, C.blockCount()); + result = C.getSValBuilder().conjureSymbolVal(Call, C.blockCount()); NonLoc resultNL = result.castAs<NonLoc>(); if (strLengthNL) { @@ -1793,8 +1790,7 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, // If we don't know the length of the string, conjure a return // value, so it can be used in constraints, at least. if (result.isUnknown()) { - result = C.getSValBuilder().conjureSymbolVal( - nullptr, Call.getOriginExpr(), LCtx, C.blockCount()); + result = C.getSValBuilder().conjureSymbolVal(Call, C.blockCount()); } } @@ -2261,8 +2257,7 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallEvent &Call, // If this is a stpcpy-style copy, but we were unable to check for a buffer // overflow, we still need a result. Conjure a return value. if (ReturnEnd && Result.isUnknown()) { - Result = svalBuilder.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx, - C.blockCount()); + Result = svalBuilder.conjureSymbolVal(Call, C.blockCount()); } } // Set the return value. @@ -2361,8 +2356,7 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallEvent &Call, const StringLiteral *RightStrLiteral = getCStringLiteral(C, state, Right.Expression, RightVal); bool canComputeResult = false; - SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, Call.getOriginExpr(), - LCtx, C.blockCount()); + SVal resultVal = svalBuilder.conjureSymbolVal(Call, C.blockCount()); if (LeftStrLiteral && RightStrLiteral) { StringRef LeftStrRef = LeftStrLiteral->getString(); @@ -2467,16 +2461,13 @@ void CStringChecker::evalStrsep(CheckerContext &C, // Overwrite the search string pointer. The new value is either an address // further along in the same string, or NULL if there are no more tokens. - State = - State->bindLoc(*SearchStrLoc, - SVB.conjureSymbolVal(getTag(), Call.getOriginExpr(), - LCtx, CharPtrTy, C.blockCount()), - LCtx); + State = State->bindLoc( + *SearchStrLoc, + SVB.conjureSymbolVal(Call, CharPtrTy, C.blockCount(), getTag()), LCtx); } else { assert(SearchStrVal.isUnknown()); // Conjure a symbolic value. It's the best we can do. - Result = SVB.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx, - C.blockCount()); + Result = SVB.conjureSymbolVal(Call, C.blockCount()); } // Set the return value, and finish. @@ -2519,8 +2510,7 @@ void CStringChecker::evalStdCopyCommon(CheckerContext &C, SValBuilder &SVB = C.getSValBuilder(); - SVal ResultVal = - SVB.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx, C.blockCount()); + SVal ResultVal = SVB.conjureSymbolVal(Call, C.blockCount()); State = State->BindExpr(Call.getOriginExpr(), LCtx, ResultVal); C.addTransition(State); diff --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp index 6076a6bc78973..74eda4eaa599c 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp @@ -130,8 +130,7 @@ void ErrnoTesterChecker::evalSetErrnoIfErrorRange(CheckerContext &C, ProgramStateRef StateFailure = State->BindExpr( Call.getOriginExpr(), C.getLocationContext(), SVB.makeIntVal(1, true)); - DefinedOrUnknownSVal ErrnoVal = SVB.conjureSymbolVal( - nullptr, Call.getOriginExpr(), C.getLocationContext(), C.blockCount()); + DefinedOrUnknownSVal ErrnoVal = SVB.conjureSymbolVal(Call, C.blockCount()); StateFailure = StateFailure->assume(ErrnoVal, true); assert(StateFailure && "Failed to assume on an initial value."); StateFailure = diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp index cc1ced7358710..85e22daaedeec 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -931,8 +931,7 @@ bool RetainCountChecker::evalCall(const CallEvent &Call, if (RetVal.isUnknown() || (hasTrustedImplementationAnnotation && !ResultTy.isNull())) { SValBuilder &SVB = C.getSValBuilder(); - RetVal = - SVB.conjureSymbolVal(nullptr, CE, LCtx, ResultTy, C.blockCount()); + RetVal = SVB.conjureSymbolVal(Call, C.blockCount()); } // Bind the value. diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp index 321388ad857f4..2feb05b142a36 100644 --- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp @@ -853,7 +853,7 @@ void SmartPtrModeling::handleBoolConversion(const CallEvent &Call, const LocationContext *LC = C.getLocationContext(); InnerPointerVal = C.getSValBuilder().conjureSymbolVal( - CallExpr, LC, InnerPointerType, C.blockCount()); + Call, InnerPointerType, C.blockCount()); State = State->set<TrackedRegionMap>(ThisRegion, InnerPointerVal); } diff --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp index 9c0b79ab58618..4b97c51ca39fa 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp @@ -584,11 +584,9 @@ class StdLibraryFunctionsChecker const Summary &Summary, CheckerContext &C) const override { SValBuilder &SVB = C.getSValBuilder(); - NonLoc ErrnoSVal = - SVB.conjureSymbolVal(&Tag, Call.getOriginExpr(), - C.getLocationContext(), C.getASTContext().IntTy, - C.blockCount()) - .castAs<NonLoc>(); + NonLoc ErrnoSVal = SVB.conjureSymbolVal(Call, C.getASTContext().IntTy, + C.blockCount(), &Tag) + .castAs<NonLoc>(); return errno_modeling::setErrnoForStdFailure(State, C, ErrnoSVal); } }; @@ -1482,7 +1480,7 @@ bool StdLibraryFunctionsChecker::evalCall(const CallEvent &Call, const LocationContext *LC = C.getLocationContext(); const auto *CE = cast<CallExpr>(Call.getOriginExpr()); SVal V = C.getSValBuilder().conjureSymbolVal( - CE, LC, CE->getType().getCanonicalType(), C.blockCount()); + Call, CE->getType().getCanonicalType(), C.blockCount()); State = State->BindExpr(CE, LC, V); C.addTransition(State); diff --git a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp index fefe846b6911f..850411dc3354f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp @@ -206,8 +206,8 @@ void InvalidPtrChecker::postPreviousReturnInvalidatingCall( const auto *CE = cast<CallExpr>(Call.getOriginExpr()); // Function call will return a pointer to the new symbolic region. - DefinedOrUnknownSVal RetVal = C.getSValBuilder().conjureSymbolVal( - CE, LCtx, CE->getType(), C.blockCount()); + DefinedOrUnknownSVal RetVal = + C.getSValBuilder().conjureSymbolVal(Call, CE->getType(), C.blockCount()); State = State->BindExpr(CE, LCtx, RetVal); const auto *SymRegOfRetVal = diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp index eb5054708fece..9e0800ba9e2fa 100644 --- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -24,6 +24,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h" #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" @@ -192,18 +193,22 @@ DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const Stmt *stmt, const LocationContext *LCtx, QualType type, unsigned visitCount) { - if (type->isNullPtrType()) - return makeZeroVal(type); - - if (!SymbolManager::canSymbolicate(type)) - return UnknownVal(); - - SymbolRef sym = SymMgr.conjureSymbol(stmt, LCtx, type, visitCount); + return conjureSymbolVal(/*symbolTag=*/nullptr, stmt, LCtx, type, visitCount); +} - if (Loc::isLocType(type)) - return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym)); +DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const CallEvent &call, + unsigned visitCount, + const void *symbolTag) { + return conjureSymbolVal(symbolTag, call.getOriginExpr(), + call.getLocationContext(), visitCount); +} - return nonloc::SymbolVal(sym); +DefinedOrUnknownSVal SValBuilder::conjureSymbolVal(const CallEvent &call, + QualType type, + unsigned visitCount, + const void *symbolTag) { + return conjureSymbolVal(symbolTag, call.getOriginExpr(), + call.getLocationContext(), type, visitCount); } DefinedSVal SValBuilder::getConjuredHeapSymbolVal(const Expr *E, >From 2aaeb5e8bd14fb5c2bbc4c418350f98f35cf678b Mon Sep 17 00:00:00 2001 From: Fangyi Zhou <m...@fangyi.io> Date: Thu, 24 Apr 2025 19:49:36 +0100 Subject: [PATCH 2/2] Remove an unused variable --- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp index 2feb05b142a36..d56e6838ce6cc 100644 --- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp @@ -851,7 +851,6 @@ void SmartPtrModeling::handleBoolConversion(const CallEvent &Call, if (InnerPointerType.isNull()) return; - const LocationContext *LC = C.getLocationContext(); InnerPointerVal = C.getSValBuilder().conjureSymbolVal( Call, InnerPointerType, C.blockCount()); State = State->set<TrackedRegionMap>(ThisRegion, InnerPointerVal); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
