Skip to content

Commit 769a309

Browse files
committed
[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 llvm#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.
1 parent d62505f commit 769a309

File tree

6 files changed

+24
-8
lines changed

6 files changed

+24
-8
lines changed

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,16 @@ class CallEvent {
211211
getExtraInvalidatedValues(ValueList &Values,
212212
RegionAndSymbolInvalidationTraits *ETraits) const {}
213213

214+
/// The state in which the call is being evaluated.
215+
/// CallEvent instances have a reference to a state object instead of storing
216+
/// the argument `SVal`s, the return value (if available), the dynamic type
217+
/// information and similar things as separate data members. However, the
218+
/// existence of this state object is an implementation detail (and perhaps
219+
/// it should be eventually eliminated), so use of this method should be
220+
/// avoided if possible. (The checker callbacks can and should access the
221+
/// canonical state through the CheckerContext.)
222+
const ProgramStateRef &getState() const { return State; }
223+
214224
public:
215225
CallEvent &operator=(const CallEvent &) = delete;
216226
virtual ~CallEvent() = default;
@@ -231,8 +241,14 @@ class CallEvent {
231241
}
232242
void setForeign(bool B) const { Foreign = B; }
233243

234-
/// The state in which the call is being evaluated.
235-
const ProgramStateRef &getState() const { return State; }
244+
/// Returns the ASTContext which is (indirectly) associated with this call
245+
/// event. This method is exposed for the convenience of a few checkers that
246+
/// need the AST context in functions that take a CallEvent as argument. For
247+
/// the sake of clarity prefer to get the ASTContext from more "natural"
248+
/// sources (e.g. the CheckerContext) instead of using this method!
249+
ASTContext &getASTContext() const {
250+
return getState()->getStateManager().getContext();
251+
}
236252

237253
/// The context in which the call is being evaluated.
238254
const LocationContext *getLocationContext() const { return LCtx; }

clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ class RAIIMutexDescriptor {
104104
// this function is called instead of early returning it. To avoid this, a
105105
// bool variable (IdentifierInfoInitialized) is used and the function will
106106
// be run only once.
107-
const auto &ASTCtx = Call.getState()->getStateManager().getContext();
107+
const auto &ASTCtx = Call.getASTContext();
108108
Guard = &ASTCtx.Idents.get(GuardName);
109109
}
110110
}

clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -929,7 +929,7 @@ ObjCDeallocChecker::getValueReleasedByNillingOut(const ObjCMethodCall &M,
929929
SVal Arg = M.getArgSVal(0);
930930
ProgramStateRef notNilState, nilState;
931931
std::tie(notNilState, nilState) =
932-
M.getState()->assume(Arg.castAs<DefinedOrUnknownSVal>());
932+
C.getState()->assume(Arg.castAs<DefinedOrUnknownSVal>());
933933
if (!(nilState && !notNilState))
934934
return nullptr;
935935

clang/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ ObjCSuperDeallocChecker::isSuperDeallocMessage(const ObjCMethodCall &M) const {
230230
if (M.getOriginExpr()->getReceiverKind() != ObjCMessageExpr::SuperInstance)
231231
return false;
232232

233-
ASTContext &Ctx = M.getState()->getStateManager().getContext();
233+
ASTContext &Ctx = M.getASTContext();
234234
initIdentifierInfoAndSelectors(Ctx);
235235

236236
return M.getSelector() == SELdealloc;

clang/lib/StaticAnalyzer/Checkers/StdVariantChecker.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,13 @@ class StdVariantChecker : public Checker<eval::Call, check::RegionChanges> {
211211
if (!DefaultType)
212212
return;
213213

214-
ProgramStateRef State = ConstructorCall->getState();
214+
ProgramStateRef State = C.getState();
215215
State = State->set<VariantHeldTypeMap>(ThisMemRegion, *DefaultType);
216216
C.addTransition(State);
217217
}
218218

219219
bool handleStdGetCall(const CallEvent &Call, CheckerContext &C) const {
220-
ProgramStateRef State = Call.getState();
220+
ProgramStateRef State = C.getState();
221221

222222
const auto &ArgType = Call.getArgSVal(0)
223223
.getType(C.getASTContext())

clang/lib/StaticAnalyzer/Checkers/TaggedUnionModeling.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ removeInformationStoredForDeadInstances(const CallEvent &Call,
5252
template <class TypeMap>
5353
void handleConstructorAndAssignment(const CallEvent &Call, CheckerContext &C,
5454
SVal ThisSVal) {
55-
ProgramStateRef State = Call.getState();
55+
ProgramStateRef State = C.getState();
5656

5757
if (!State)
5858
return;

0 commit comments

Comments
 (0)