[clang] [NFCI][analyzer] Make CallEvent::getState protected (PR #162673)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 18 09:28:13 PDT 2025
https://github.com/NagyDonat updated https://github.com/llvm/llvm-project/pull/162673
>From 769a30920e1202605cd3bd23f110211a13255ea3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 1 Oct 2025 17:02:36 +0200
Subject: [PATCH 1/5] [NFCI][analyzer] Make CallEvent::getState protected
`CallEvent` instances have a reference to a state object instead having
separate data members for storing the arguments (as `SVal` instances), the
return value (as `SVal`, if available), the dynamic type information and
similar things.
Previously this state was publicly available, which meant that many
checker callbacks had two ways to access the state: either through the
`CallEvent` or through the `CheckerContext`. This redundancy is
inelegant and bugprone (e.g. the recent commit
https://github.com/llvm/llvm-project/pull/160707 fixed a situation where
the state attached to the `CallEvent` could be obsolete in `EvalCall` and
`PointerEscape` callbacks), so this commit limits access to the state
attached to a `CallEvent` and turns it into a protected implementation
detail.
In the future it may be a good idea to completely remove the state
instance from the `CallEvent` and explicitly store the few parts of the
state which are relevant for the call.
In theory this commit should be a non-functional change (because AFAIK
the `CallEvent` and `CheckerContext` provide the same state after the
recent fix), but there is a small chance that it fixes some bugs that I
do not know about.
---
.../Core/PathSensitive/CallEvent.h | 20 +++++++++++++++++--
.../BlockInCriticalSectionChecker.cpp | 2 +-
.../Checkers/CheckObjCDealloc.cpp | 2 +-
.../Checkers/ObjCSuperDeallocChecker.cpp | 2 +-
.../Checkers/StdVariantChecker.cpp | 4 ++--
.../Checkers/TaggedUnionModeling.h | 2 +-
6 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 5dcf03f7a4648..2b082d07da71a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -211,6 +211,16 @@ class CallEvent {
getExtraInvalidatedValues(ValueList &Values,
RegionAndSymbolInvalidationTraits *ETraits) const {}
+ /// The state in which the call is being evaluated.
+ /// CallEvent instances have a reference to a state object instead of storing
+ /// the argument `SVal`s, the return value (if available), the dynamic type
+ /// information and similar things as separate data members. However, the
+ /// existence of this state object is an implementation detail (and perhaps
+ /// it should be eventually eliminated), so use of this method should be
+ /// avoided if possible. (The checker callbacks can and should access the
+ /// canonical state through the CheckerContext.)
+ const ProgramStateRef &getState() const { return State; }
+
public:
CallEvent &operator=(const CallEvent &) = delete;
virtual ~CallEvent() = default;
@@ -231,8 +241,14 @@ class CallEvent {
}
void setForeign(bool B) const { Foreign = B; }
- /// The state in which the call is being evaluated.
- const ProgramStateRef &getState() const { return State; }
+ /// Returns the ASTContext which is (indirectly) associated with this call
+ /// event. This method is exposed for the convenience of a few checkers that
+ /// need the AST context in functions that take a CallEvent as argument. For
+ /// the sake of clarity prefer to get the ASTContext from more "natural"
+ /// sources (e.g. the CheckerContext) instead of using this method!
+ ASTContext &getASTContext() const {
+ return getState()->getStateManager().getContext();
+ }
/// The context in which the call is being evaluated.
const LocationContext *getLocationContext() const { return LCtx; }
diff --git a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
index bf35bee70870b..3ddd6590fcbb0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -104,7 +104,7 @@ class RAIIMutexDescriptor {
// this function is called instead of early returning it. To avoid this, a
// bool variable (IdentifierInfoInitialized) is used and the function will
// be run only once.
- const auto &ASTCtx = Call.getState()->getStateManager().getContext();
+ const auto &ASTCtx = Call.getASTContext();
Guard = &ASTCtx.Idents.get(GuardName);
}
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
index 9d3aeff465ca1..242084876a3c5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -929,7 +929,7 @@ ObjCDeallocChecker::getValueReleasedByNillingOut(const ObjCMethodCall &M,
SVal Arg = M.getArgSVal(0);
ProgramStateRef notNilState, nilState;
std::tie(notNilState, nilState) =
- M.getState()->assume(Arg.castAs<DefinedOrUnknownSVal>());
+ C.getState()->assume(Arg.castAs<DefinedOrUnknownSVal>());
if (!(nilState && !notNilState))
return nullptr;
diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
index f984caf59afb8..c1550f28f6f27 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
@@ -230,7 +230,7 @@ ObjCSuperDeallocChecker::isSuperDeallocMessage(const ObjCMethodCall &M) const {
if (M.getOriginExpr()->getReceiverKind() != ObjCMessageExpr::SuperInstance)
return false;
- ASTContext &Ctx = M.getState()->getStateManager().getContext();
+ ASTContext &Ctx = M.getASTContext();
initIdentifierInfoAndSelectors(Ctx);
return M.getSelector() == SELdealloc;
diff --git a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp
index 4fc1c576a9687..db8bbee8761d5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp
@@ -211,13 +211,13 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> {
if (!DefaultType)
return;
- ProgramStateRef State = ConstructorCall->getState();
+ ProgramStateRef State = C.getState();
State = State->set<VariantHeldTypeMap>(ThisMemRegion, *DefaultType);
C.addTransition(State);
}
bool handleStdGetCall(const CallEvent &Call, CheckerContext &C) const {
- ProgramStateRef State = Call.getState();
+ ProgramStateRef State = C.getState();
const auto &ArgType = Call.getArgSVal(0)
.getType(C.getASTContext())
diff --git a/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h b/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h
index dec461296fed5..b8fb57213fd65 100644
--- a/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h
@@ -52,7 +52,7 @@ removeInformationStoredForDeadInstances(const CallEvent &Call,
template <class TypeMap>
void handleConstructorAndAssignment(const CallEvent &Call, CheckerContext &C,
SVal ThisSVal) {
- ProgramStateRef State = Call.getState();
+ ProgramStateRef State = C.getState();
if (!State)
return;
>From e0d10bac7e9fe15da1e6b35be089369ca990902f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 13 Oct 2025 14:57:58 +0200
Subject: [PATCH 2/5] Shorten and clarify comments
---
.../Core/PathSensitive/CallEvent.h | 24 ++++++++-----------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 2b082d07da71a..f07506394e779 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -211,15 +211,14 @@ class CallEvent {
getExtraInvalidatedValues(ValueList &Values,
RegionAndSymbolInvalidationTraits *ETraits) const {}
- /// The state in which the call is being evaluated.
- /// CallEvent instances have a reference to a state object instead of storing
- /// the argument `SVal`s, the return value (if available), the dynamic type
- /// information and similar things as separate data members. However, the
- /// existence of this state object is an implementation detail (and perhaps
- /// it should be eventually eliminated), so use of this method should be
- /// avoided if possible. (The checker callbacks can and should access the
- /// canonical state through the CheckerContext.)
- const ProgramStateRef &getState() const { return State; }
+ /// A state for looking up relevant Environment entries (arguments, return
+ /// value), dynamic type information and similar "stable" things.
+ /// WARNING: During the evaluation of a function call, several state
+ /// transitions happen, so this state can become partially obsolete!
+ ///
+ /// TODO: Instead of storing a complete state object in the CallEvent, only
+ /// store the relevant parts (argument/return SVals, dynamic type etc.).
+ ProgramStateRef getState() const { return State; }
public:
CallEvent &operator=(const CallEvent &) = delete;
@@ -241,11 +240,8 @@ class CallEvent {
}
void setForeign(bool B) const { Foreign = B; }
- /// Returns the ASTContext which is (indirectly) associated with this call
- /// event. This method is exposed for the convenience of a few checkers that
- /// need the AST context in functions that take a CallEvent as argument. For
- /// the sake of clarity prefer to get the ASTContext from more "natural"
- /// sources (e.g. the CheckerContext) instead of using this method!
+ /// NOTE: There are plans for refactoring that would eliminate this method.
+ /// Prefer to use CheckerContext::getASTContext if possible!
ASTContext &getASTContext() const {
return getState()->getStateManager().getContext();
}
>From ca654161610f53bfad36c48dd0ae765c86c77b99 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 13 Oct 2025 15:39:15 +0200
Subject: [PATCH 3/5] Make the returned ASTContext const
---
.../clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h | 2 +-
.../lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index f07506394e779..ef245227df999 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -242,7 +242,7 @@ class CallEvent {
/// NOTE: There are plans for refactoring that would eliminate this method.
/// Prefer to use CheckerContext::getASTContext if possible!
- ASTContext &getASTContext() const {
+ const ASTContext &getASTContext() const {
return getState()->getStateManager().getContext();
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
index c1550f28f6f27..3f9600b0e9b70 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
@@ -34,7 +34,7 @@ class ObjCSuperDeallocChecker
this, "[super dealloc] should not be called more than once",
categories::CoreFoundationObjectiveC};
- void initIdentifierInfoAndSelectors(ASTContext &Ctx) const;
+ void initIdentifierInfoAndSelectors(const ASTContext &Ctx) const;
bool isSuperDeallocMessage(const ObjCMethodCall &M) const;
@@ -215,7 +215,7 @@ void ObjCSuperDeallocChecker::diagnoseCallArguments(const CallEvent &CE,
}
void
-ObjCSuperDeallocChecker::initIdentifierInfoAndSelectors(ASTContext &Ctx) const {
+ObjCSuperDeallocChecker::initIdentifierInfoAndSelectors(const ASTContext &Ctx) const {
if (IIdealloc)
return;
@@ -230,7 +230,7 @@ ObjCSuperDeallocChecker::isSuperDeallocMessage(const ObjCMethodCall &M) const {
if (M.getOriginExpr()->getReceiverKind() != ObjCMessageExpr::SuperInstance)
return false;
- ASTContext &Ctx = M.getASTContext();
+ const ASTContext &Ctx = M.getASTContext();
initIdentifierInfoAndSelectors(Ctx);
return M.getSelector() == SELdealloc;
>From 1d76d59df9ba7e5ee99826c358ddc5eac06f7b42 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Mon, 13 Oct 2025 15:50:06 +0200
Subject: [PATCH 4/5] Satisfy git-clang-format
---
clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
index 3f9600b0e9b70..227cbfac770d2 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
@@ -214,8 +214,8 @@ void ObjCSuperDeallocChecker::diagnoseCallArguments(const CallEvent &CE,
}
}
-void
-ObjCSuperDeallocChecker::initIdentifierInfoAndSelectors(const ASTContext &Ctx) const {
+void ObjCSuperDeallocChecker::initIdentifierInfoAndSelectors(
+ const ASTContext &Ctx) const {
if (IIdealloc)
return;
>From dd4a5c7caf38ba3aea8ad89159c067d6d79e0e78 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.nagy at ericsson.com>
Date: Wed, 15 Oct 2025 11:35:23 +0200
Subject: [PATCH 5/5] Clarify TODO commet
Co-authored-by: Artem Dergachev <noqnoqneo at gmail.com>
---
.../clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index ef245227df999..3f26b30459a19 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -217,7 +217,8 @@ class CallEvent {
/// transitions happen, so this state can become partially obsolete!
///
/// TODO: Instead of storing a complete state object in the CallEvent, only
- /// store the relevant parts (argument/return SVals, dynamic type etc.).
+ /// store the relevant parts (such as argument/return SVals etc.) that aren't
+ /// allowed to become obsolete until the end of the call evaluation.
ProgramStateRef getState() const { return State; }
public:
More information about the cfe-commits
mailing list