Skip to content

Commit 22a5bb3

Browse files
[analyzer] Limit Store by region-store-binding-limit (llvm#127602)
In our test pool, the max entry point RT was improved by this change: 1'181 seconds (~19.7 minutes) -> 94 seconds (1.6 minutes) BTW, the 1.6 minutes is still really bad. But a few orders of magnitude better than it was before. This was the most servere RT edge-case as you can see from the numbers. There are are more known RT bottlenecks, such as: - Large environment sizes, and `removeDead`. See more about the failed attempt on improving it at: https://discourse.llvm.org/t/unsuccessful-attempts-to-fix-a-slow-analysis-case-related-to-removedead-and-environment-size/84650 - Large chunk of time could be spend inside `assume`, to reach a fixed point. This is something we want to look into a bit later if we have time. We have 3'075'607 entry points in our test set. About 393'352 entry points ran longer than 1 second when measured. To give a sense of the distribution, if we ignore the slowest 500 entry points, then the maximum entry point runs for about 14 seconds. These 500 slow entry points are in 332 translation units. By this patch, out of the slowest 500 entry points, 72 entry points were improved by at least 10x after this change. We measured no RT regression on the "usual" entry points. ![slow-entrypoints-before-and-after-bind-limit](https://github.com/user-attachments/assets/44425a76-f1cb-449c-bc3e-f44beb8c5dc7) (The dashed lines represent the maximum of their RT) CPP-6092
1 parent 96c87a1 commit 22a5bb3

File tree

10 files changed

+659
-155
lines changed

10 files changed

+659
-155
lines changed

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,14 @@ ANALYZER_OPTION(
483483
"behavior, set the option to 0.",
484484
5)
485485

486+
ANALYZER_OPTION(
487+
unsigned, RegionStoreMaxBindingFanOut, "region-store-max-binding-fanout",
488+
"This option limits how many sub-bindings a single binding operation can "
489+
"scatter into. For example, binding an array would scatter into binding "
490+
"each individual element. Setting this to zero means unlimited, but then "
491+
"modelling large array initializers may take proportional time to their "
492+
"size.", 128)
493+
486494
//===----------------------------------------------------------------------===//
487495
// String analyzer options.
488496
//===----------------------------------------------------------------------===//

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,13 +659,13 @@ class ExprEngine {
659659
SVal Loc, SVal Val,
660660
const LocationContext *LCtx);
661661

662+
public:
662663
/// A simple wrapper when you only need to notify checkers of pointer-escape
663664
/// of some values.
664665
ProgramStateRef escapeValues(ProgramStateRef State, ArrayRef<SVal> Vs,
665666
PointerEscapeKind K,
666667
const CallEvent *Call = nullptr) const;
667668

668-
public:
669669
// FIXME: 'tag' should be removed, and a LocationContext should be used
670670
// instead.
671671
// FIXME: Comment on the meaning of the arguments, when 'St' may not

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ class ProgramState : public llvm::FoldingSetNode {
126126
/// makeWithStore - Return a ProgramState with the same values as the current
127127
/// state with the exception of using the specified Store.
128128
ProgramStateRef makeWithStore(const StoreRef &store) const;
129+
ProgramStateRef makeWithStore(const BindResult &BindRes) const;
129130

130131
void setStore(const StoreRef &storeRef);
131132

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,14 @@ class SymbolReaper;
5050

5151
using InvalidatedSymbols = llvm::DenseSet<SymbolRef>;
5252

53+
struct BindResult {
54+
StoreRef ResultingStore;
55+
56+
// If during the bind operation we exhaust the allowed binding budget, we set
57+
// this to the beginning of the escaped part of the region.
58+
llvm::SmallVector<SVal, 0> FailedToBindValues;
59+
};
60+
5361
class StoreManager {
5462
protected:
5563
SValBuilder &svalBuilder;
@@ -105,17 +113,17 @@ class StoreManager {
105113
/// \return A StoreRef object that contains the same
106114
/// bindings as \c store with the addition of having the value specified
107115
/// by \c val bound to the location given for \c loc.
108-
virtual StoreRef Bind(Store store, Loc loc, SVal val) = 0;
116+
virtual BindResult Bind(Store store, Loc loc, SVal val) = 0;
109117

110118
/// Return a store with the specified value bound to all sub-regions of the
111119
/// region. The region must not have previous bindings. If you need to
112120
/// invalidate existing bindings, consider invalidateRegions().
113-
virtual StoreRef BindDefaultInitial(Store store, const MemRegion *R,
114-
SVal V) = 0;
121+
virtual BindResult BindDefaultInitial(Store store, const MemRegion *R,
122+
SVal V) = 0;
115123

116124
/// Return a store with in which all values within the given region are
117125
/// reset to zero. This method is allowed to overwrite previous bindings.
118-
virtual StoreRef BindDefaultZero(Store store, const MemRegion *R) = 0;
126+
virtual BindResult BindDefaultZero(Store store, const MemRegion *R) = 0;
119127

120128
/// Create a new store with the specified binding removed.
121129
/// \param ST the original store, that is the basis for the new store.
@@ -240,9 +248,8 @@ class StoreManager {
240248

241249
/// enterStackFrame - Let the StoreManager to do something when execution
242250
/// engine is about to execute into a callee.
243-
StoreRef enterStackFrame(Store store,
244-
const CallEvent &Call,
245-
const StackFrameContext *CalleeCtx);
251+
BindResult enterStackFrame(Store store, const CallEvent &Call,
252+
const StackFrameContext *CalleeCtx);
246253

247254
/// Finds the transitive closure of symbols within the given region.
248255
///

clang/lib/StaticAnalyzer/Core/ProgramState.cpp

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -116,32 +116,33 @@ ProgramStateRef ProgramState::bindLoc(Loc LV,
116116
const LocationContext *LCtx,
117117
bool notifyChanges) const {
118118
ProgramStateManager &Mgr = getStateManager();
119-
ProgramStateRef newState = makeWithStore(Mgr.StoreMgr->Bind(getStore(),
120-
LV, V));
119+
ExprEngine &Eng = Mgr.getOwningEngine();
120+
ProgramStateRef State = makeWithStore(Mgr.StoreMgr->Bind(getStore(), LV, V));
121121
const MemRegion *MR = LV.getAsRegion();
122+
122123
if (MR && notifyChanges)
123-
return Mgr.getOwningEngine().processRegionChange(newState, MR, LCtx);
124+
return Eng.processRegionChange(State, MR, LCtx);
124125

125-
return newState;
126+
return State;
126127
}
127128

128129
ProgramStateRef
129130
ProgramState::bindDefaultInitial(SVal loc, SVal V,
130131
const LocationContext *LCtx) const {
131132
ProgramStateManager &Mgr = getStateManager();
132133
const MemRegion *R = loc.castAs<loc::MemRegionVal>().getRegion();
133-
const StoreRef &newStore = Mgr.StoreMgr->BindDefaultInitial(getStore(), R, V);
134-
ProgramStateRef new_state = makeWithStore(newStore);
135-
return Mgr.getOwningEngine().processRegionChange(new_state, R, LCtx);
134+
BindResult BindRes = Mgr.StoreMgr->BindDefaultInitial(getStore(), R, V);
135+
ProgramStateRef State = makeWithStore(BindRes);
136+
return Mgr.getOwningEngine().processRegionChange(State, R, LCtx);
136137
}
137138

138139
ProgramStateRef
139140
ProgramState::bindDefaultZero(SVal loc, const LocationContext *LCtx) const {
140141
ProgramStateManager &Mgr = getStateManager();
141142
const MemRegion *R = loc.castAs<loc::MemRegionVal>().getRegion();
142-
const StoreRef &newStore = Mgr.StoreMgr->BindDefaultZero(getStore(), R);
143-
ProgramStateRef new_state = makeWithStore(newStore);
144-
return Mgr.getOwningEngine().processRegionChange(new_state, R, LCtx);
143+
BindResult BindRes = Mgr.StoreMgr->BindDefaultZero(getStore(), R);
144+
ProgramStateRef State = makeWithStore(BindRes);
145+
return Mgr.getOwningEngine().processRegionChange(State, R, LCtx);
145146
}
146147

147148
typedef ArrayRef<const MemRegion *> RegionList;
@@ -232,9 +233,8 @@ SVal ProgramState::wrapSymbolicRegion(SVal Val) const {
232233
ProgramStateRef
233234
ProgramState::enterStackFrame(const CallEvent &Call,
234235
const StackFrameContext *CalleeCtx) const {
235-
const StoreRef &NewStore =
236-
getStateManager().StoreMgr->enterStackFrame(getStore(), Call, CalleeCtx);
237-
return makeWithStore(NewStore);
236+
return makeWithStore(
237+
getStateManager().StoreMgr->enterStackFrame(getStore(), Call, CalleeCtx));
238238
}
239239

240240
SVal ProgramState::getSelfSVal(const LocationContext *LCtx) const {
@@ -437,6 +437,18 @@ ProgramStateRef ProgramState::makeWithStore(const StoreRef &store) const {
437437
return getStateManager().getPersistentState(NewSt);
438438
}
439439

440+
ProgramStateRef ProgramState::makeWithStore(const BindResult &BindRes) const {
441+
ExprEngine &Eng = getStateManager().getOwningEngine();
442+
ProgramStateRef State = makeWithStore(BindRes.ResultingStore);
443+
444+
// We must always notify the checkers for failing binds because otherwise they
445+
// may keep stale traits for these symbols.
446+
// Eg., Malloc checker may report leaks if we failed to bind that symbol.
447+
if (BindRes.FailedToBindValues.empty())
448+
return State;
449+
return Eng.escapeValues(State, BindRes.FailedToBindValues, PSK_EscapeOnBind);
450+
}
451+
440452
ProgramStateRef ProgramState::cloneAsPosteriorlyOverconstrained() const {
441453
ProgramState NewSt(*this);
442454
NewSt.PosteriorlyOverconstrained = true;

0 commit comments

Comments
 (0)