Skip to content

Commit cea09a5

Browse files
fangyi-zhoufangyi-zhou
committed
Address most of the reviewer comments
Test are expected to fail, to be fixed in a later commit.
1 parent 87b45cc commit cea09a5

31 files changed

+317
-343
lines changed

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

Lines changed: 4 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());
@@ -1195,6 +1196,8 @@ class CFGBlock {
11951196
}
11961197
};
11971198

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

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +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/ADT/StringRef.h"
2322
#include "llvm/Support/raw_ostream.h"
24-
#include <cctype>
2523

2624
namespace clang {
2725

@@ -32,15 +30,11 @@ class SValExplainer : public FullSValVisitor<SValExplainer, std::string> {
3230
ASTContext &ACtx;
3331
ProgramStateRef State;
3432

35-
std::string printCFGElementRef(CFGBlock::ConstCFGElementRef ElemRef) {
33+
std::string printCFGElementRef(ConstCFGElementRef Elem) {
3634
std::string Str;
3735
llvm::raw_string_ostream OS(Str);
38-
ElemRef->dumpToStream(OS);
39-
// HACK: `CFGBlock::ConstCFGElementRef::dumpToStream` contains a new line
40-
// character in the end of the string, we don't want it so we remove it
41-
// here.
42-
llvm::StringRef StrRef(Str);
43-
return StrRef.rtrim().str();
36+
Elem->dumpToStream(OS, /*TerminateWithNewLine=*/false);
37+
return Str;
4438
}
4539

4640
std::string printStmt(const Stmt *S) {

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,9 +151,7 @@ class CheckerContext {
151151
return Pred->getSVal(S);
152152
}
153153

154-
CFGBlock::ConstCFGElementRef getCFGElementRef() const {
155-
return Eng.getCFGElementRef();
156-
}
154+
ConstCFGElementRef getCFGElementRef() const { return Eng.getCFGElementRef(); }
157155

158156
/// Returns true if the value of \p E is greater than or equal to \p
159157
/// Val under unsigned comparison.

‎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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ namespace ento {
2828
ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
2929
const LocationContext *LCtx,
3030
unsigned BlockCount,
31-
CFGBlock::ConstCFGElementRef ElemRef);
31+
ConstCFGElementRef Elem);
3232

3333
} // end namespace ento
3434
} // end namespace clang

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -313,8 +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 ElemRef \p CFGBlock::ConstCFGElementRef that caused the
317-
/// invalidation.
316+
/// \param Elem The CFG Element that caused the invalidation.
318317
/// \param BlockCount The number of times the current basic block has been
319318
/// visited.
320319
/// \param CausesPointerEscape the flag is set to true when the invalidation
@@ -327,14 +326,14 @@ class ProgramState : public llvm::FoldingSetNode {
327326
/// or symbols.
328327
[[nodiscard]] ProgramStateRef
329328
invalidateRegions(ArrayRef<const MemRegion *> Regions,
330-
CFGBlock::ConstCFGElementRef ElemRef, unsigned BlockCount,
329+
ConstCFGElementRef Elem, unsigned BlockCount,
331330
const LocationContext *LCtx, bool CausesPointerEscape,
332331
InvalidatedSymbols *IS = nullptr,
333332
const CallEvent *Call = nullptr,
334333
RegionAndSymbolInvalidationTraits *ITraits = nullptr) const;
335334

336335
[[nodiscard]] ProgramStateRef
337-
invalidateRegions(ArrayRef<SVal> Values, CFGBlock::ConstCFGElementRef ElemRef,
336+
invalidateRegions(ArrayRef<SVal> Values, ConstCFGElementRef Elem,
338337
unsigned BlockCount, const LocationContext *LCtx,
339338
bool CausesPointerEscape, InvalidatedSymbols *IS = nullptr,
340339
const CallEvent *Call = nullptr,

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,11 @@ class SValBuilder {
172172

173173
// Forwarding methods to SymbolManager.
174174

175-
const SymbolConjured *conjureSymbol(CFGBlock::ConstCFGElementRef ElemRef,
175+
const SymbolConjured *conjureSymbol(ConstCFGElementRef Elem,
176176
const LocationContext *LCtx,
177177
QualType type, unsigned visitCount,
178178
const void *symbolTag = nullptr) {
179-
return SymMgr.conjureSymbol(ElemRef, LCtx, type, visitCount, symbolTag);
179+
return SymMgr.conjureSymbol(Elem, LCtx, type, visitCount, symbolTag);
180180
}
181181

182182
/// Construct an SVal representing '0' for the specified type.
@@ -192,19 +192,19 @@ class SValBuilder {
192192
/// preserve the relation between related(or even equivalent) expressions, so
193193
/// conjured symbols should be used sparingly.
194194
DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag,
195-
CFGBlock::ConstCFGElementRef elemRef,
195+
ConstCFGElementRef elem,
196196
const LocationContext *LCtx,
197197
unsigned count);
198198
DefinedOrUnknownSVal conjureSymbolVal(const void *symbolTag,
199-
CFGBlock::ConstCFGElementRef elemRef,
199+
ConstCFGElementRef elem,
200200
const LocationContext *LCtx,
201201
QualType type, unsigned count);
202-
DefinedOrUnknownSVal conjureSymbolVal(CFGBlock::ConstCFGElementRef elemRef,
202+
DefinedOrUnknownSVal conjureSymbolVal(ConstCFGElementRef elem,
203203
const LocationContext *LCtx,
204204
QualType type, unsigned visitCount);
205205

206206
/// Conjure a symbol representing heap allocated memory region.
207-
DefinedSVal getConjuredHeapSymbolVal(CFGBlock::ConstCFGElementRef elemRef,
207+
DefinedSVal getConjuredHeapSymbolVal(ConstCFGElementRef elem,
208208
const LocationContext *LCtx,
209209
QualType type, unsigned Count);
210210

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,8 @@ class StoreManager {
223223
///
224224
/// \param[in] store The initial store.
225225
/// \param[in] Values The values to invalidate.
226-
/// \param[in] ElemRef The current \p CFGBlock::ConstCFGElementRef being
227-
/// evaluated. Used to conjure symbols to mark the values of invalidated
228-
/// regions.
226+
/// \param[in] Elem The current CFG Element being evaluated. Used to conjure
227+
/// symbols to mark the values of invalidated regions.
229228
/// \param[in] Count The current block count. Used to conjure
230229
/// symbols to mark the values of invalidated regions.
231230
/// \param[in] Call The call expression which will be used to determine which
@@ -242,7 +241,7 @@ class StoreManager {
242241
/// even if they do not currently have bindings. Pass \c NULL if this
243242
/// information will not be used.
244243
virtual StoreRef invalidateRegions(
245-
Store store, ArrayRef<SVal> Values, CFGBlock::ConstCFGElementRef ElemRef,
244+
Store store, ArrayRef<SVal> Values, ConstCFGElementRef Elem,
246245
unsigned Count, const LocationContext *LCtx, const CallEvent *Call,
247246
InvalidatedSymbols &IS, RegionAndSymbolInvalidationTraits &ITraits,
248247
InvalidatedRegions *TopLevelRegions, InvalidatedRegions *Invalidated) = 0;

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

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -80,58 +80,56 @@ 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-
CFGBlock::ConstCFGElementRef ElemRef;
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, CFGBlock::ConstCFGElementRef elemRef,
90+
SymbolConjured(SymbolID sym, ConstCFGElementRef elem,
9191
const LocationContext *lctx, QualType t, unsigned count,
9292
const void *symbolTag)
93-
: SymbolData(SymbolConjuredKind, sym), ElemRef(elemRef), T(t),
94-
Count(count), LCtx(lctx), SymbolTag(symbolTag) {
93+
: SymbolData(SymbolConjuredKind, sym), Elem(elem), T(t), Count(count),
94+
LCtx(lctx), SymbolTag(symbolTag) {
9595
assert(lctx);
9696
assert(isValidTypeForSymbol(t));
9797
}
9898

9999
public:
100-
const CFGBlock::ConstCFGElementRef getCFGElementRef() const {
101-
return ElemRef;
102-
}
100+
ConstCFGElementRef getCFGElementRef() const { return Elem; }
103101

104102
// It might return null.
105103
const Stmt *getStmt() const {
106-
switch (ElemRef->getKind()) {
104+
switch (Elem->getKind()) {
107105
case CFGElement::Initializer:
108-
return ElemRef->castAs<CFGInitializer>().getInitializer()->getInit();
106+
return Elem->castAs<CFGInitializer>().getInitializer()->getInit();
109107
case CFGElement::ScopeBegin:
110-
return ElemRef->castAs<CFGScopeBegin>().getTriggerStmt();
108+
return Elem->castAs<CFGScopeBegin>().getTriggerStmt();
111109
case CFGElement::ScopeEnd:
112-
return ElemRef->castAs<CFGScopeEnd>().getTriggerStmt();
110+
return Elem->castAs<CFGScopeEnd>().getTriggerStmt();
113111
case CFGElement::NewAllocator:
114-
return ElemRef->castAs<CFGNewAllocator>().getAllocatorExpr();
112+
return Elem->castAs<CFGNewAllocator>().getAllocatorExpr();
115113
case CFGElement::LifetimeEnds:
116-
return ElemRef->castAs<CFGLifetimeEnds>().getTriggerStmt();
114+
return Elem->castAs<CFGLifetimeEnds>().getTriggerStmt();
117115
case CFGElement::LoopExit:
118-
return ElemRef->castAs<CFGLoopExit>().getLoopStmt();
116+
return Elem->castAs<CFGLoopExit>().getLoopStmt();
119117
case CFGElement::Statement:
120-
return ElemRef->castAs<CFGStmt>().getStmt();
118+
return Elem->castAs<CFGStmt>().getStmt();
121119
case CFGElement::Constructor:
122-
return ElemRef->castAs<CFGConstructor>().getStmt();
120+
return Elem->castAs<CFGConstructor>().getStmt();
123121
case CFGElement::CXXRecordTypedCall:
124-
return ElemRef->castAs<CFGCXXRecordTypedCall>().getStmt();
122+
return Elem->castAs<CFGCXXRecordTypedCall>().getStmt();
125123
case CFGElement::AutomaticObjectDtor:
126-
return ElemRef->castAs<CFGAutomaticObjDtor>().getTriggerStmt();
124+
return Elem->castAs<CFGAutomaticObjDtor>().getTriggerStmt();
127125
case CFGElement::DeleteDtor:
128-
return ElemRef->castAs<CFGDeleteDtor>().getDeleteExpr();
126+
return Elem->castAs<CFGDeleteDtor>().getDeleteExpr();
129127
case CFGElement::BaseDtor:
130128
return nullptr;
131129
case CFGElement::MemberDtor:
132130
return nullptr;
133131
case CFGElement::TemporaryDtor:
134-
return ElemRef->castAs<CFGTemporaryDtor>().getBindTemporaryExpr();
132+
return Elem->castAs<CFGTemporaryDtor>().getBindTemporaryExpr();
135133
case CFGElement::CleanupFunction:
136134
return nullptr;
137135
}
@@ -148,20 +146,19 @@ class SymbolConjured : public SymbolData {
148146

149147
void dumpToStream(raw_ostream &os) const override;
150148

151-
static void Profile(llvm::FoldingSetNodeID &profile,
152-
CFGBlock::ConstCFGElementRef ElemRef,
149+
static void Profile(llvm::FoldingSetNodeID &profile, ConstCFGElementRef Elem,
153150
const LocationContext *LCtx, QualType T, unsigned Count,
154151
const void *SymbolTag) {
155152
profile.AddInteger((unsigned)SymbolConjuredKind);
156-
profile.Add(ElemRef);
153+
profile.Add(Elem);
157154
profile.AddPointer(LCtx);
158155
profile.Add(T);
159156
profile.AddInteger(Count);
160157
profile.AddPointer(SymbolTag);
161158
}
162159

163160
void Profile(llvm::FoldingSetNodeID& profile) override {
164-
Profile(profile, ElemRef, LCtx, T, Count, SymbolTag);
161+
Profile(profile, Elem, LCtx, T, Count, SymbolTag);
165162
}
166163

167164
// Implement isa<T> support.
@@ -569,12 +566,12 @@ class SymbolManager {
569566
template <typename SymExprT, typename... Args>
570567
const SymExprT *acquire(Args &&...args);
571568

572-
const SymbolConjured *conjureSymbol(CFGBlock::ConstCFGElementRef ElemRef,
569+
const SymbolConjured *conjureSymbol(ConstCFGElementRef Elem,
573570
const LocationContext *LCtx, QualType T,
574571
unsigned VisitCount,
575572
const void *SymbolTag = nullptr) {
576573

577-
return acquire<SymbolConjured>(ElemRef, LCtx, T, VisitCount, SymbolTag);
574+
return acquire<SymbolConjured>(Elem, LCtx, T, VisitCount, SymbolTag);
578575
}
579576

580577
QualType getType(const SymExpr *SE) const {

0 commit comments

Comments
 (0)