Skip to content

Commit 6078f5e

Browse files
fangyi-zhoufangyi-zhou
authored
Reland [Clang][analyzer] replace Stmt* with ConstCFGElement in SymbolConjured (llvm#137355)
Closes llvm#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. There is a caveat at loop widening, where the correct location is the CFG terminator (which is not an element and does not have a ref). In this case, the first element in the block is passed as a location. Previous PR llvm#128251, Reverted at llvm#137304.
1 parent d597452 commit 6078f5e

38 files changed

+476
-389
lines changed

‎clang/include/clang/Analysis/CFG.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ class CFGElement {
122122
return (Kind) x;
123123
}
124124

125-
void dumpToStream(llvm::raw_ostream &OS) const;
125+
void dumpToStream(llvm::raw_ostream &OS,
126+
bool TerminateWithNewLine = true) const;
126127

127128
void dump() const {
128129
dumpToStream(llvm::errs());
@@ -695,6 +696,11 @@ class CFGBlock {
695696
void dump() const {
696697
dumpToStream(llvm::errs());
697698
}
699+
700+
void Profile(llvm::FoldingSetNodeID &ID) const {
701+
ID.AddPointer(Parent);
702+
ID.AddInteger(Index);
703+
}
698704
};
699705

700706
template <bool IsReverse, bool IsConst> class ElementRefIterator {
@@ -1190,6 +1196,8 @@ class CFGBlock {
11901196
}
11911197
};
11921198

1199+
using ConstCFGElementRef = CFGBlock::ConstCFGElementRef;
1200+
11931201
/// CFGCallback defines methods that should be called when a logical
11941202
/// operator error is found when building the CFG.
11951203
class CFGCallback {

‎clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "clang/AST/DeclCXX.h"
2020
#include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
2121
#include "llvm/ADT/StringExtras.h"
22+
#include "llvm/Support/raw_ostream.h"
2223

2324
namespace clang {
2425

@@ -29,6 +30,13 @@ class SValExplainer : public FullSValVisitor<SValExplainer, std::string> {
2930
ASTContext &ACtx;
3031
ProgramStateRef State;
3132

33+
std::string printCFGElementRef(ConstCFGElementRef Elem) {
34+
std::string Str;
35+
llvm::raw_string_ostream OS(Str);
36+
Elem->dumpToStream(OS, /*TerminateWithNewLine=*/false);
37+
return Str;
38+
}
39+
3240
std::string printStmt(const Stmt *S) {
3341
std::string Str;
3442
llvm::raw_string_ostream OS(Str);
@@ -114,7 +122,8 @@ class SValExplainer : public FullSValVisitor<SValExplainer, std::string> {
114122

115123
std::string VisitSymbolConjured(const SymbolConjured *S) {
116124
return "symbol of type '" + S->getType().getAsString() +
117-
"' conjured at statement '" + printStmt(S->getStmt()) + "'";
125+
"' conjured at CFG element '" +
126+
printCFGElementRef(S->getCFGElementRef()) + "'";
118127
}
119128

120129
std::string VisitSymbolDerived(const SymbolDerived *S) {

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ class CheckerContext {
151151
return Pred->getSVal(S);
152152
}
153153

154+
ConstCFGElementRef getCFGElementRef() const { return Eng.getCFGElementRef(); }
155+
154156
/// Returns true if the value of \p E is greater than or equal to \p
155157
/// Val under unsigned comparison.
156158
bool isGreaterOrEqual(const Expr *E, unsigned long long Val);

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ class ExprEngine {
226226
return (*G.roots_begin())->getLocation().getLocationContext();
227227
}
228228

229-
CFGBlock::ConstCFGElementRef getCFGElementRef() const {
229+
ConstCFGElementRef getCFGElementRef() const {
230230
const CFGBlock *blockPtr = currBldrCtx ? currBldrCtx->getBlock() : nullptr;
231231
return {blockPtr, currStmtIdx};
232232
}

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ namespace ento {
2727
/// by the loop body in any iteration.
2828
ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
2929
const LocationContext *LCtx,
30-
unsigned BlockCount, const Stmt *LoopStmt);
30+
unsigned BlockCount,
31+
ConstCFGElementRef Elem);
3132

3233
} // end namespace ento
3334
} // end namespace clang

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ class ProgramState : public llvm::FoldingSetNode {
313313
/// be triggered by this event.
314314
///
315315
/// \param Regions the set of regions to be invalidated.
316-
/// \param E the expression that caused the invalidation.
316+
/// \param Elem The CFG Element that caused the invalidation.
317317
/// \param BlockCount The number of times the current basic block has been
318318
/// visited.
319319
/// \param CausesPointerEscape the flag is set to true when the invalidation
@@ -325,16 +325,17 @@ class ProgramState : public llvm::FoldingSetNode {
325325
/// \param ITraits information about special handling for particular regions
326326
/// or symbols.
327327
[[nodiscard]] ProgramStateRef
328-
invalidateRegions(ArrayRef<const MemRegion *> Regions, const Stmt *S,
329-
unsigned BlockCount, const LocationContext *LCtx,
330-
bool CausesPointerEscape, InvalidatedSymbols *IS = nullptr,
328+
invalidateRegions(ArrayRef<const MemRegion *> Regions,
329+
ConstCFGElementRef Elem, unsigned BlockCount,
330+
const LocationContext *LCtx, bool CausesPointerEscape,
331+
InvalidatedSymbols *IS = nullptr,
331332
const CallEvent *Call = nullptr,
332333
RegionAndSymbolInvalidationTraits *ITraits = nullptr) const;
333334

334335
[[nodiscard]] ProgramStateRef
335-
invalidateRegions(ArrayRef<SVal> Values, const Stmt *S, unsigned BlockCount,
336-
const LocationContext *LCtx, bool CausesPointerEscape,
337-
InvalidatedSymbols *IS = nullptr,
336+
invalidateRegions(ArrayRef<SVal> Values, ConstCFGElementRef Elem,
337+
unsigned BlockCount, const LocationContext *LCtx,
338+
bool CausesPointerEscape, InvalidatedSymbols *IS = nullptr,
338339
const CallEvent *Call = nullptr,
339340
RegionAndSymbolInvalidationTraits *ITraits = nullptr) const;
340341

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h‎

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "clang/AST/Expr.h"
2020
#include "clang/AST/ExprObjC.h"
2121
#include "clang/AST/Type.h"
22+
#include "clang/Analysis/CFG.h"
2223
#include "clang/Basic/LLVM.h"
2324
#include "clang/Basic/LangOptions.h"
2425
#include "clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h"
@@ -171,19 +172,11 @@ class SValBuilder {
171172

172173
// Forwarding methods to SymbolManager.
173174

174-
const SymbolConjured* conjureSymbol(const Stmt *stmt,
175+
const SymbolConjured *conjureSymbol(ConstCFGElementRef Elem,
175176
const LocationContext *LCtx,
176-
QualType type,
177-
unsigned visitCount,
177+
QualType type, unsigned visitCount,
178178
const void *symbolTag = nullptr) {
179-
return SymMgr.conjureSymbol(stmt, LCtx, type, visitCount, symbolTag);
180-
}
181-
182-
const SymbolConjured* conjureSymbol(const Expr *expr,
183-
const LocationContext *LCtx,
184-
unsigned visitCount,
185-
const void *symbolTag = nullptr) {
186-
return SymMgr.conjureSymbol(expr, LCtx, visitCount, symbolTag);
179+
return SymMgr.conjureSymbol(Elem, LCtx, type, visitCount, symbolTag);
187180
}
188181

189182
/// Construct an SVal representing '0' for the specified type.
@@ -199,16 +192,16 @@ class SValBuilder {
199192
/// preserve the relation between related(or even equivalent) expressions, so
200193
/// conjured symbols should be used sparingly.
201194
DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag,
202-
const Expr *expr,
195+
ConstCFGElementRef elem,
203196
const LocationContext *LCtx,
204197
unsigned count);
205-
DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag, const Stmt *S,
198+
DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag,
199+
ConstCFGElementRef elem,
206200
const LocationContext *LCtx,
207201
QualType type, unsigned count);
208-
DefinedOrUnknownSVal conjureSymbolVal(const Stmt *stmt,
202+
DefinedOrUnknownSVal conjureSymbolVal(ConstCFGElementRef elem,
209203
const LocationContext *LCtx,
210-
QualType type,
211-
unsigned visitCount);
204+
QualType type, unsigned visitCount);
212205
DefinedOrUnknownSVal conjureSymbolVal(const CallEvent &call, QualType type,
213206
unsigned visitCount,
214207
const void *symbolTag = nullptr);
@@ -217,17 +210,7 @@ class SValBuilder {
217210
const void *symbolTag = nullptr);
218211

219212
/// Conjure a symbol representing heap allocated memory region.
220-
///
221-
/// Note, the expression should represent a location.
222-
DefinedSVal getConjuredHeapSymbolVal(const Expr *E,
223-
const LocationContext *LCtx,
224-
unsigned Count);
225-
226-
/// Conjure a symbol representing heap allocated memory region.
227-
///
228-
/// Note, now, the expression *doesn't* need to represent a location.
229-
/// But the type need to!
230-
DefinedSVal getConjuredHeapSymbolVal(const Expr *E,
213+
DefinedSVal getConjuredHeapSymbolVal(ConstCFGElementRef elem,
231214
const LocationContext *LCtx,
232215
QualType type, unsigned Count);
233216

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@
1414
#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_STORE_H
1515

1616
#include "clang/AST/Type.h"
17+
#include "clang/Basic/LLVM.h"
1718
#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
1819
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
1920
#include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h"
2021
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
2122
#include "clang/StaticAnalyzer/Core/PathSensitive/StoreRef.h"
2223
#include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
23-
#include "clang/Basic/LLVM.h"
2424
#include "llvm/ADT/ArrayRef.h"
2525
#include "llvm/ADT/DenseSet.h"
2626
#include "llvm/ADT/SmallVector.h"
@@ -223,7 +223,7 @@ class StoreManager {
223223
///
224224
/// \param[in] store The initial store.
225225
/// \param[in] Values The values to invalidate.
226-
/// \param[in] S The current statement being evaluated. Used to conjure
226+
/// \param[in] Elem The current CFG Element being evaluated. Used to conjure
227227
/// symbols to mark the values of invalidated regions.
228228
/// \param[in] Count The current block count. Used to conjure
229229
/// symbols to mark the values of invalidated regions.
@@ -241,8 +241,8 @@ class StoreManager {
241241
/// even if they do not currently have bindings. Pass \c NULL if this
242242
/// information will not be used.
243243
virtual StoreRef invalidateRegions(
244-
Store store, ArrayRef<SVal> Values, const Stmt *S, unsigned Count,
245-
const LocationContext *LCtx, const CallEvent *Call,
244+
Store store, ArrayRef<SVal> Values, ConstCFGElementRef Elem,
245+
unsigned Count, const LocationContext *LCtx, const CallEvent *Call,
246246
InvalidatedSymbols &IS, RegionAndSymbolInvalidationTraits &ITraits,
247247
InvalidatedRegions *TopLevelRegions, InvalidatedRegions *Invalidated) = 0;
248248

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h‎

Lines changed: 49 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -80,29 +80,62 @@ class SymbolRegionValue : public SymbolData {
8080
/// A symbol representing the result of an expression in the case when we do
8181
/// not know anything about what the expression is.
8282
class SymbolConjured : public SymbolData {
83-
const Stmt *S;
83+
ConstCFGElementRef Elem;
8484
QualType T;
8585
unsigned Count;
8686
const LocationContext *LCtx;
8787
const void *SymbolTag;
8888

8989
friend class SymExprAllocator;
90-
SymbolConjured(SymbolID sym, const Stmt *s, const LocationContext *lctx,
91-
QualType t, unsigned count, const void *symbolTag)
92-
: SymbolData(SymbolConjuredKind, sym), S(s), T(t), Count(count),
90+
SymbolConjured(SymbolID sym, ConstCFGElementRef elem,
91+
const LocationContext *lctx, QualType t, unsigned count,
92+
const void *symbolTag)
93+
: SymbolData(SymbolConjuredKind, sym), Elem(elem), T(t), Count(count),
9394
LCtx(lctx), SymbolTag(symbolTag) {
94-
// FIXME: 's' might be a nullptr if we're conducting invalidation
95-
// that was caused by a destructor call on a temporary object,
96-
// which has no statement associated with it.
97-
// Due to this, we might be creating the same invalidation symbol for
98-
// two different invalidation passes (for two different temporaries).
9995
assert(lctx);
10096
assert(isValidTypeForSymbol(t));
10197
}
10298

10399
public:
104-
/// It might return null.
105-
const Stmt *getStmt() const { return S; }
100+
ConstCFGElementRef getCFGElementRef() const { return Elem; }
101+
102+
// It might return null.
103+
const Stmt *getStmt() const {
104+
switch (Elem->getKind()) {
105+
case CFGElement::Initializer:
106+
return Elem->castAs<CFGInitializer>().getInitializer()->getInit();
107+
case CFGElement::ScopeBegin:
108+
return Elem->castAs<CFGScopeBegin>().getTriggerStmt();
109+
case CFGElement::ScopeEnd:
110+
return Elem->castAs<CFGScopeEnd>().getTriggerStmt();
111+
case CFGElement::NewAllocator:
112+
return Elem->castAs<CFGNewAllocator>().getAllocatorExpr();
113+
case CFGElement::LifetimeEnds:
114+
return Elem->castAs<CFGLifetimeEnds>().getTriggerStmt();
115+
case CFGElement::LoopExit:
116+
return Elem->castAs<CFGLoopExit>().getLoopStmt();
117+
case CFGElement::Statement:
118+
return Elem->castAs<CFGStmt>().getStmt();
119+
case CFGElement::Constructor:
120+
return Elem->castAs<CFGConstructor>().getStmt();
121+
case CFGElement::CXXRecordTypedCall:
122+
return Elem->castAs<CFGCXXRecordTypedCall>().getStmt();
123+
case CFGElement::AutomaticObjectDtor:
124+
return Elem->castAs<CFGAutomaticObjDtor>().getTriggerStmt();
125+
case CFGElement::DeleteDtor:
126+
return Elem->castAs<CFGDeleteDtor>().getDeleteExpr();
127+
case CFGElement::BaseDtor:
128+
return nullptr;
129+
case CFGElement::MemberDtor:
130+
return nullptr;
131+
case CFGElement::TemporaryDtor:
132+
return Elem->castAs<CFGTemporaryDtor>().getBindTemporaryExpr();
133+
case CFGElement::CleanupFunction:
134+
return nullptr;
135+
}
136+
return nullptr;
137+
}
138+
106139
unsigned getCount() const { return Count; }
107140
/// It might return null.
108141
const void *getTag() const { return SymbolTag; }
@@ -113,19 +146,19 @@ class SymbolConjured : public SymbolData {
113146

114147
void dumpToStream(raw_ostream &os) const override;
115148

116-
static void Profile(llvm::FoldingSetNodeID &profile, const Stmt *S,
149+
static void Profile(llvm::FoldingSetNodeID &profile, ConstCFGElementRef Elem,
117150
const LocationContext *LCtx, QualType T, unsigned Count,
118151
const void *SymbolTag) {
119152
profile.AddInteger((unsigned)SymbolConjuredKind);
120-
profile.AddPointer(S);
153+
profile.Add(Elem);
121154
profile.AddPointer(LCtx);
122155
profile.Add(T);
123156
profile.AddInteger(Count);
124157
profile.AddPointer(SymbolTag);
125158
}
126159

127160
void Profile(llvm::FoldingSetNodeID& profile) override {
128-
Profile(profile, S, LCtx, T, Count, SymbolTag);
161+
Profile(profile, Elem, LCtx, T, Count, SymbolTag);
129162
}
130163

131164
// Implement isa<T> support.
@@ -533,18 +566,12 @@ class SymbolManager {
533566
template <typename SymExprT, typename... Args>
534567
const SymExprT *acquire(Args &&...args);
535568

536-
const SymbolConjured *conjureSymbol(const Stmt *E,
569+
const SymbolConjured *conjureSymbol(ConstCFGElementRef Elem,
537570
const LocationContext *LCtx, QualType T,
538571
unsigned VisitCount,
539572
const void *SymbolTag = nullptr) {
540-
return acquire<SymbolConjured>(E, LCtx, T, VisitCount, SymbolTag);
541-
}
542573

543-
const SymbolConjured* conjureSymbol(const Expr *E,
544-
const LocationContext *LCtx,
545-
unsigned VisitCount,
546-
const void *SymbolTag = nullptr) {
547-
return conjureSymbol(E, LCtx, E->getType(), VisitCount, SymbolTag);
574+
return acquire<SymbolConjured>(Elem, LCtx, T, VisitCount, SymbolTag);
548575
}
549576

550577
QualType getType(const SymExpr *SE) const {

0 commit comments

Comments
 (0)