[clang] [Clang] [analyzer] replace Stmt* with ConstCFGElementRef in SymbolConjured (PR #128251)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 21 17:23:35 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Fangyi Zhou (fangyi-zhou)
<details>
<summary>Changes</summary>
Closes #<!-- -->57270.
This PR changes the `Stmt *` field in `SymbolConjured` with `CFGBlock::ConstCFGElementRef`. The motivation is that, when conjuring a symbol, there might not always be a statement available, causing information to be lost for conjured symbols, whereas the CFGElementRef can always be provided at the callsite.
Following the idea, this PR changes callsites of functions to create conjured symbols, and replaces them with appropriate `CFGElementRef`s.
---
Patch is 68.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/128251.diff
25 Files Affected:
- (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (+5)
- (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h (+52-39)
- (modified) clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h (+37-20)
- (modified) clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (+21-13)
- (modified) clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp (+17-16)
- (modified) clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp (+1-1)
- (modified) clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp (+2-1)
- (modified) clang/lib/StaticAnalyzer/Checkers/Iterator.cpp (+6-5)
- (modified) clang/lib/StaticAnalyzer/Checkers/Iterator.h (+5-4)
- (modified) clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp (+17-12)
- (modified) clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (+4-2)
- (modified) clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp (+2-2)
- (modified) clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp (+8-5)
- (modified) clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp (+4-4)
- (modified) clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp (+3-2)
- (modified) clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp (+2-1)
- (modified) clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp (+1-1)
- (modified) clang/lib/StaticAnalyzer/Core/ExprEngine.cpp (+9-9)
- (modified) clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp (+30-25)
- (modified) clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (+7-6)
- (modified) clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (+4-2)
- (modified) clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp (+9-7)
- (modified) clang/lib/StaticAnalyzer/Core/RegionStore.cpp (+31-26)
- (modified) clang/lib/StaticAnalyzer/Core/SValBuilder.cpp (+54-39)
- (modified) clang/lib/StaticAnalyzer/Core/SymbolManager.cpp (+1-1)
``````````diff
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
index 168983fd5cb68..02bd4a91961a9 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
@@ -151,6 +151,11 @@ class CheckerContext {
return Pred->getSVal(S);
}
+ /// Get the CFG Element Ref from the ExprEngine
+ CFGBlock::ConstCFGElementRef getCFGElementRef() const {
+ return Eng.getCFGElementRef();
+ }
+
/// Returns true if the value of \p E is greater than or equal to \p
/// Val under unsigned comparison
bool isGreaterOrEqual(const Expr *E, unsigned long long Val);
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 54430d426a82a..6fb5f15822585 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -19,6 +19,7 @@
#include "clang/AST/Expr.h"
#include "clang/AST/ExprObjC.h"
#include "clang/AST/Type.h"
+#include "clang/Analysis/CFG.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/LangOptions.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h"
@@ -171,20 +172,27 @@ class SValBuilder {
// Forwarding methods to SymbolManager.
- const SymbolConjured* conjureSymbol(const Stmt *stmt,
- const LocationContext *LCtx,
- QualType type,
- unsigned visitCount,
- const void *symbolTag = nullptr) {
- return SymMgr.conjureSymbol(stmt, LCtx, type, visitCount, symbolTag);
+ const SymbolConjured *
+ conjureSymbol(const CFGBlock::ConstCFGElementRef ElemRef,
+ const LocationContext *LCtx, QualType type, unsigned visitCount,
+ const void *symbolTag = nullptr) {
+ return SymMgr.conjureSymbol(ElemRef, LCtx, type, visitCount, symbolTag);
}
- const SymbolConjured* conjureSymbol(const Expr *expr,
- const LocationContext *LCtx,
- unsigned visitCount,
- const void *symbolTag = nullptr) {
- return SymMgr.conjureSymbol(expr, LCtx, visitCount, symbolTag);
- }
+ // const SymbolConjured* conjureSymbol(const Stmt *stmt,
+ // const LocationContext *LCtx,
+ // QualType type,
+ // unsigned visitCount,
+ // const void *symbolTag = nullptr) {
+ // return SymMgr.conjureSymbol(stmt, LCtx, type, visitCount, symbolTag);
+ // }
+
+ // const SymbolConjured* conjureSymbol(const Expr *expr,
+ // const LocationContext *LCtx,
+ // unsigned visitCount,
+ // const void *symbolTag = nullptr) {
+ // return SymMgr.conjureSymbol(expr, LCtx, visitCount, symbolTag);
+ // }
/// Construct an SVal representing '0' for the specified type.
DefinedOrUnknownSVal makeZeroVal(QualType type);
@@ -198,33 +206,38 @@ class SValBuilder {
/// The advantage of symbols derived/built from other symbols is that we
/// preserve the relation between related(or even equivalent) expressions, so
/// conjured symbols should be used sparingly.
- DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag,
- const Expr *expr,
- const LocationContext *LCtx,
- unsigned count);
- DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag, const Stmt *S,
- const LocationContext *LCtx,
- QualType type, unsigned count);
- DefinedOrUnknownSVal conjureSymbolVal(const Stmt *stmt,
- const LocationContext *LCtx,
- QualType type,
- unsigned visitCount);
-
- /// Conjure a symbol representing heap allocated memory region.
- ///
- /// Note, the expression should represent a location.
- DefinedSVal getConjuredHeapSymbolVal(const Expr *E,
- const LocationContext *LCtx,
- unsigned Count);
-
- /// Conjure a symbol representing heap allocated memory region.
- ///
- /// Note, now, the expression *doesn't* need to represent a location.
- /// But the type need to!
- DefinedSVal getConjuredHeapSymbolVal(const Expr *E,
- const LocationContext *LCtx,
- QualType type, unsigned Count);
-
+ // DefinedOrUnknownSVal
+ // conjureSymbolVal(const void *symbolTag,
+ // const CFGBlock::ConstCFGElementRef elemRef,
+ // const LocationContext *LCtx, unsigned count);
+ DefinedOrUnknownSVal
+ conjureSymbolVal(const void *symbolTag,
+ const CFGBlock::ConstCFGElementRef elemRef,
+ const LocationContext *LCtx, QualType type, unsigned count);
+ DefinedOrUnknownSVal
+ conjureSymbolVal(const CFGBlock::ConstCFGElementRef elemRef,
+ const LocationContext *LCtx, QualType type,
+ unsigned visitCount);
+
+ // /// Conjure a symbol representing heap allocated memory region.
+ // ///
+ // /// Note, the expression should represent a location.
+ // DefinedSVal getConjuredHeapSymbolVal(const Expr *E,
+ // const LocationContext *LCtx,
+ // unsigned Count);
+
+ // /// Conjure a symbol representing heap allocated memory region.
+ // ///
+ // /// Note, now, the expression *doesn't* need to represent a location.
+ // /// But the type need to!
+ // DefinedSVal getConjuredHeapSymbolVal(const Expr *E,
+ // const LocationContext *LCtx,
+ // QualType type, unsigned Count);
+
+ DefinedSVal
+ getConjuredHeapSymbolVal(const CFGBlock::ConstCFGElementRef elemRef,
+ const LocationContext *LCtx, QualType type,
+ unsigned Count);
/// Create an SVal representing the result of an alloca()-like call, that is,
/// an AllocaRegion on the stack.
///
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index cbbea1b56bb40..4e24c9a81ae1f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -17,6 +17,7 @@
#include "clang/AST/Expr.h"
#include "clang/AST/Type.h"
#include "clang/Analysis/AnalysisDeclContext.h"
+#include "clang/Analysis/CFG.h"
#include "clang/Basic/LLVM.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/APSIntPtr.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
@@ -80,17 +81,18 @@ class SymbolRegionValue : public SymbolData {
/// A symbol representing the result of an expression in the case when we do
/// not know anything about what the expression is.
class SymbolConjured : public SymbolData {
- const Stmt *S;
+ const CFGBlock::ConstCFGElementRef ElemRef;
QualType T;
unsigned Count;
const LocationContext *LCtx;
const void *SymbolTag;
friend class SymExprAllocator;
- SymbolConjured(SymbolID sym, const Stmt *s, const LocationContext *lctx,
- QualType t, unsigned count, const void *symbolTag)
- : SymbolData(SymbolConjuredKind, sym), S(s), T(t), Count(count),
- LCtx(lctx), SymbolTag(symbolTag) {
+ SymbolConjured(SymbolID sym, CFGBlock::ConstCFGElementRef elemRef,
+ const LocationContext *lctx, QualType t, unsigned count,
+ const void *symbolTag)
+ : SymbolData(SymbolConjuredKind, sym), ElemRef(elemRef), T(t),
+ Count(count), LCtx(lctx), SymbolTag(symbolTag) {
// FIXME: 's' might be a nullptr if we're conducting invalidation
// that was caused by a destructor call on a temporary object,
// which has no statement associated with it.
@@ -102,7 +104,12 @@ class SymbolConjured : public SymbolData {
public:
/// It might return null.
- const Stmt *getStmt() const { return S; }
+ const Stmt *getStmt() const {
+ if (auto Stmt = ElemRef->getAs<CFGStmt>()) {
+ return Stmt->getStmt();
+ }
+ return nullptr;
+ }
unsigned getCount() const { return Count; }
/// It might return null.
const void *getTag() const { return SymbolTag; }
@@ -113,11 +120,13 @@ class SymbolConjured : public SymbolData {
void dumpToStream(raw_ostream &os) const override;
- static void Profile(llvm::FoldingSetNodeID &profile, const Stmt *S,
+ static void Profile(llvm::FoldingSetNodeID &profile,
+ const CFGBlock::ConstCFGElementRef ElemRef,
const LocationContext *LCtx, QualType T, unsigned Count,
const void *SymbolTag) {
profile.AddInteger((unsigned)SymbolConjuredKind);
- profile.AddPointer(S);
+ // profile.Add(ElemRef);
+ // profile.AddPointer(S);
profile.AddPointer(LCtx);
profile.Add(T);
profile.AddInteger(Count);
@@ -125,7 +134,7 @@ class SymbolConjured : public SymbolData {
}
void Profile(llvm::FoldingSetNodeID& profile) override {
- Profile(profile, S, LCtx, T, Count, SymbolTag);
+ Profile(profile, ElemRef, LCtx, T, Count, SymbolTag);
}
// Implement isa<T> support.
@@ -533,20 +542,28 @@ class SymbolManager {
template <typename SymExprT, typename... Args>
const SymExprT *acquire(Args &&...args);
- const SymbolConjured *conjureSymbol(const Stmt *E,
- const LocationContext *LCtx, QualType T,
- unsigned VisitCount,
- const void *SymbolTag = nullptr) {
- return acquire<SymbolConjured>(E, LCtx, T, VisitCount, SymbolTag);
- }
+ // const SymbolConjured *conjureSymbol(const Stmt *E,
+ // const LocationContext *LCtx, QualType
+ // T, unsigned VisitCount, const void
+ // *SymbolTag = nullptr) {
+ // return acquire<SymbolConjured>(E, LCtx, T, VisitCount, SymbolTag);
+ // }
+
+ const SymbolConjured *
+ conjureSymbol(const CFGBlock::ConstCFGElementRef ElemRef,
+ const LocationContext *LCtx, QualType T, unsigned VisitCount,
+ const void *SymbolTag = nullptr) {
- const SymbolConjured* conjureSymbol(const Expr *E,
- const LocationContext *LCtx,
- unsigned VisitCount,
- const void *SymbolTag = nullptr) {
- return conjureSymbol(E, LCtx, E->getType(), VisitCount, SymbolTag);
+ return acquire<SymbolConjured>(ElemRef, LCtx, T, VisitCount, SymbolTag);
}
+ // const SymbolConjured* conjureSymbol(const Expr *E,
+ // const LocationContext *LCtx,
+ // unsigned VisitCount,
+ // const void *SymbolTag = nullptr) {
+ // return conjureSymbol(E, LCtx, E->getType(), VisitCount, SymbolTag);
+ // }
+
QualType getType(const SymExpr *SE) const {
return SE->getType();
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 39dcaf02dbe25..ea3b815a95bc1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1515,7 +1515,8 @@ void CStringChecker::evalCopyCommon(CheckerContext &C, const CallEvent &Call,
// conjure a return value for later.
if (lastElement.isUnknown())
lastElement = C.getSValBuilder().conjureSymbolVal(
- nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
+ nullptr, Call.getCFGElementRef(), LCtx,
+ Call.getOriginExpr()->getType(), C.blockCount());
// The byte after the last byte copied is the return value.
state = state->BindExpr(Call.getOriginExpr(), LCtx, lastElement);
@@ -1665,8 +1666,9 @@ 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(
+ nullptr, Call.getCFGElementRef(), LCtx,
+ Call.getOriginExpr()->getType(), C.blockCount());
State = State->BindExpr(Call.getOriginExpr(), LCtx, CmpV);
C.addTransition(State);
}
@@ -1770,7 +1772,8 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C,
// 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());
+ nullptr, Call.getCFGElementRef(), LCtx,
+ Call.getOriginExpr()->getType(), C.blockCount());
NonLoc resultNL = result.castAs<NonLoc>();
if (strLengthNL) {
@@ -1794,7 +1797,8 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C,
// value, so it can be used in constraints, at least.
if (result.isUnknown()) {
result = C.getSValBuilder().conjureSymbolVal(
- nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
+ nullptr, Call.getCFGElementRef(), LCtx,
+ Call.getOriginExpr()->getType(), C.blockCount());
}
}
@@ -2261,8 +2265,9 @@ 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(
+ nullptr, Call.getCFGElementRef(), LCtx,
+ Call.getOriginExpr()->getType(), C.blockCount());
}
}
// Set the return value.
@@ -2361,8 +2366,9 @@ 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(
+ nullptr, Call.getCFGElementRef(), LCtx, Call.getOriginExpr()->getType(),
+ C.blockCount());
if (LeftStrLiteral && RightStrLiteral) {
StringRef LeftStrRef = LeftStrLiteral->getString();
@@ -2469,14 +2475,15 @@ void CStringChecker::evalStrsep(CheckerContext &C,
// further along in the same string, or NULL if there are no more tokens.
State =
State->bindLoc(*SearchStrLoc,
- SVB.conjureSymbolVal(getTag(), Call.getOriginExpr(),
+ SVB.conjureSymbolVal(getTag(), Call.getCFGElementRef(),
LCtx, CharPtrTy, C.blockCount()),
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(nullptr, Call.getCFGElementRef(), LCtx,
+ Call.getOriginExpr()->getType(), C.blockCount());
}
// Set the return value, and finish.
@@ -2520,7 +2527,8 @@ void CStringChecker::evalStdCopyCommon(CheckerContext &C,
SValBuilder &SVB = C.getSValBuilder();
SVal ResultVal =
- SVB.conjureSymbolVal(nullptr, Call.getOriginExpr(), LCtx, C.blockCount());
+ SVB.conjureSymbolVal(nullptr, Call.getCFGElementRef(), LCtx,
+ Call.getOriginExpr()->getType(), C.blockCount());
State = State->BindExpr(Call.getOriginExpr(), LCtx, ResultVal);
C.addTransition(State);
diff --git a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
index 55ed809bfed6c..74a7b8e0f54ff 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -107,13 +107,13 @@ bool frontModifiable(ProgramStateRef State, const MemRegion *Reg);
bool backModifiable(ProgramStateRef State, const MemRegion *Reg);
SymbolRef getContainerBegin(ProgramStateRef State, const MemRegion *Cont);
SymbolRef getContainerEnd(ProgramStateRef State, const MemRegion *Cont);
-ProgramStateRef createContainerBegin(ProgramStateRef State,
+ProgramStateRef createContainerBegin(CheckerContext &C, ProgramStateRef State,
const MemRegion *Cont, const Expr *E,
QualType T, const LocationContext *LCtx,
unsigned BlockCount);
-ProgramStateRef createContainerEnd(ProgramStateRef State, const MemRegion *Cont,
- const Expr *E, QualType T,
- const LocationContext *LCtx,
+ProgramStateRef createContainerEnd(CheckerContext &C, ProgramStateRef State,
+ const MemRegion *Cont, const Expr *E,
+ QualType T, const LocationContext *LCtx,
unsigned BlockCount);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
const ContainerData &CData);
@@ -260,8 +260,9 @@ void ContainerModeling::handleBegin(CheckerContext &C, const Expr *CE,
auto State = C.getState();
auto BeginSym = getContainerBegin(State, ContReg);
if (!BeginSym) {
- State = createContainerBegin(State, ContReg, CE, C.getASTContext().LongTy,
- C.getLocationContext(), C.blockCount());
+ State =
+ createContainerBegin(C, State, ContReg, CE, C.getASTContext().LongTy,
+ C.getLocationContext(), C.blockCount());
BeginSym = getContainerBegin(State, ContReg);
}
State = setIteratorPosition(State, RetVal,
@@ -282,7 +283,7 @@ void ContainerModeling::handleEnd(CheckerContext &C, const Expr *CE,
auto State = C.getState();
auto EndSym = getContainerEnd(State, ContReg);
if (!EndSym) {
- State = createContainerEnd(State, ContReg, CE, C.getASTContext().LongTy,
+ State = createContainerEnd(C, State, ContReg, CE, C.getASTContext().LongTy,
C.getLocationContext(), C.blockCount());
EndSym = getContainerEnd(State, ContReg);
}
@@ -326,7 +327,7 @@ void ContainerModeling::handleAssignment(CheckerContext &C, SVal Cont,
auto &SVB = C.getSValBuilder();
// Then generate and assign a new "end" symbol for the new container.
auto NewEndSym =
- SymMgr.conjureSymbol(CE, C.getLocationContext(),
+ SymMgr.conjureSymbol(C.getCFGElementRef(), C.getLocationContext(),
C.getASTContext().LongTy, C.blockCount());
State = assumeNoOverflow(State, NewEndSym, 4);
if (CData) {
@@ -844,7 +845,7 @@ SymbolRef getContainerEnd(ProgramStateRef State, const MemRegion *Cont) {
return CDataPtr->getEnd();
}
-ProgramStateRef createContainerBegin(ProgramStateRef State,
+ProgramStateRef createContainerBegin(CheckerContext &C, Pr...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/128251
More information about the cfe-commits
mailing list