Skip to content

Commit 5882e6f

Browse files
Xazax-hunXazax-hun
committed
[analyzer] Escape symbols conjured into specific regions during a conservative EvalCall
This patch introduced additional PointerEscape callbacks after conservative calls for output parameters. This should not really affect the current checkers but the upcoming FuchsiaHandleChecker relies on this heavily. Differential Revision: https://reviews.llvm.org/D71224
1 parent 19e83a9 commit 5882e6f

File tree

11 files changed

+155
-50
lines changed

11 files changed

+155
-50
lines changed

‎clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,6 +1266,12 @@ def AnalysisOrderChecker : Checker<"AnalysisOrder">,
12661266
"false",
12671267
Released,
12681268
Hide>,
1269+
CmdLineOption<Boolean,
1270+
"PointerEscape",
1271+
"",
1272+
"false",
1273+
Released,
1274+
Hide>,
12691275
CmdLineOption<Boolean,
12701276
"*",
12711277
"Enables all callbacks.",

‎clang/include/clang/StaticAnalyzer/Core/CheckerManager.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ enum PointerEscapeKind {
8585
/// argument to a function.
8686
PSK_IndirectEscapeOnCall,
8787

88+
89+
/// Escape for a new symbol that was generated into a region
90+
/// that the analyzer cannot follow during a conservative call.
91+
PSK_EscapeOutParameters,
92+
8893
/// The reason for pointer escape is unknown. For example,
8994
/// a region containing this pointer is invalidated.
9095
PSK_EscapeOther

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -613,10 +613,16 @@ class ExprEngine : public SubEngine {
613613
const ProgramPoint *PP = nullptr);
614614

615615
/// Call PointerEscape callback when a value escapes as a result of bind.
616-
ProgramStateRef processPointerEscapedOnBind(ProgramStateRef State,
617-
SVal Loc,
618-
SVal Val,
619-
const LocationContext *LCtx) override;
616+
ProgramStateRef processPointerEscapedOnBind(
617+
ProgramStateRef State, ArrayRef<std::pair<SVal, SVal>> LocAndVals,
618+
const LocationContext *LCtx, PointerEscapeKind Kind,
619+
const CallEvent *Call) override;
620+
621+
ProgramStateRef
622+
processPointerEscapedOnBind(ProgramStateRef State,
623+
SVal Loc, SVal Val,
624+
const LocationContext *LCtx);
625+
620626
/// Call PointerEscape callback when a value escapes as a result of
621627
/// region invalidation.
622628
/// \param[in] ITraits Specifies invalidation traits for regions/symbols.
@@ -628,9 +634,10 @@ class ExprEngine : public SubEngine {
628634
RegionAndSymbolInvalidationTraits &ITraits) override;
629635

630636
/// A simple wrapper when you only need to notify checkers of pointer-escape
631-
/// of a single value.
632-
ProgramStateRef escapeValue(ProgramStateRef State, SVal V,
633-
PointerEscapeKind K) const;
637+
/// of some values.
638+
ProgramStateRef escapeValues(ProgramStateRef State, ArrayRef<SVal> Vs,
639+
PointerEscapeKind K,
640+
const CallEvent *Call = nullptr) const;
634641

635642
public:
636643
// FIXME: 'tag' should be removed, and a LocationContext should be used

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "clang/Analysis/ProgramPoint.h"
1616
#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
1717
#include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
18+
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
1819

1920
namespace clang {
2021

@@ -148,8 +149,10 @@ class SubEngine {
148149
return processRegionChanges(state, nullptr, MR, MR, LCtx, nullptr);
149150
}
150151

151-
virtual ProgramStateRef
152-
processPointerEscapedOnBind(ProgramStateRef State, SVal Loc, SVal Val, const LocationContext *LCtx) = 0;
152+
virtual ProgramStateRef processPointerEscapedOnBind(
153+
ProgramStateRef State, ArrayRef<std::pair<SVal, SVal>> LocAndVals,
154+
const LocationContext *LCtx, PointerEscapeKind Kind,
155+
const CallEvent *Call) = 0;
153156

154157
virtual ProgramStateRef
155158
notifyCheckersOfPointerEscape(ProgramStateRef State,

‎clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class AnalysisOrderChecker
4040
check::EndFunction,
4141
check::NewAllocator,
4242
check::Bind,
43+
check::PointerEscape,
4344
check::RegionChanges,
4445
check::LiveSymbols> {
4546

@@ -165,6 +166,15 @@ class AnalysisOrderChecker
165166
llvm::errs() << "RegionChanges\n";
166167
return State;
167168
}
169+
170+
ProgramStateRef checkPointerEscape(ProgramStateRef State,
171+
const InvalidatedSymbols &Escaped,
172+
const CallEvent *Call,
173+
PointerEscapeKind Kind) const {
174+
if (isCallbackEnabled(State, "PointerEscape"))
175+
llvm::errs() << "PointerEscape\n";
176+
return State;
177+
}
168178
};
169179
} // end anonymous namespace
170180

‎clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

Lines changed: 56 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,13 +1172,16 @@ void ExprEngine::VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *BTE,
11721172
}
11731173
}
11741174

1175-
ProgramStateRef ExprEngine::escapeValue(ProgramStateRef State, SVal V,
1176-
PointerEscapeKind K) const {
1175+
ProgramStateRef ExprEngine::escapeValues(ProgramStateRef State,
1176+
ArrayRef<SVal> Vs,
1177+
PointerEscapeKind K,
1178+
const CallEvent *Call) const {
11771179
class CollectReachableSymbolsCallback final : public SymbolVisitor {
1178-
InvalidatedSymbols Symbols;
1180+
InvalidatedSymbols &Symbols;
11791181

11801182
public:
1181-
explicit CollectReachableSymbolsCallback(ProgramStateRef) {}
1183+
explicit CollectReachableSymbolsCallback(InvalidatedSymbols &Symbols)
1184+
: Symbols(Symbols) {}
11821185

11831186
const InvalidatedSymbols &getSymbols() const { return Symbols; }
11841187

@@ -1187,11 +1190,13 @@ ProgramStateRef ExprEngine::escapeValue(ProgramStateRef State, SVal V,
11871190
return true;
11881191
}
11891192
};
1193+
InvalidatedSymbols Symbols;
1194+
CollectReachableSymbolsCallback CallBack(Symbols);
1195+
for (SVal V : Vs)
1196+
State->scanReachableSymbols(V, CallBack);
11901197

1191-
const CollectReachableSymbolsCallback &Scanner =
1192-
State->scanReachableSymbols<CollectReachableSymbolsCallback>(V);
11931198
return getCheckerManager().runCheckersForPointerEscape(
1194-
State, Scanner.getSymbols(), /*CallEvent*/ nullptr, K, nullptr);
1199+
State, CallBack.getSymbols(), Call, K, nullptr);
11951200
}
11961201

11971202
void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
@@ -1484,7 +1489,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
14841489
for (auto Child : Ex->children()) {
14851490
assert(Child);
14861491
SVal Val = State->getSVal(Child, LCtx);
1487-
State = escapeValue(State, Val, PSK_EscapeOther);
1492+
State = escapeValues(State, Val, PSK_EscapeOther);
14881493
}
14891494

14901495
Bldr2.generateNode(S, N, State);
@@ -2685,33 +2690,52 @@ void ExprEngine::VisitAtomicExpr(const AtomicExpr *AE, ExplodedNode *Pred,
26852690
// destructor. We won't see the destructor during analysis, but it's there.
26862691
// (4) We are binding to a MemRegion with stack storage that the store
26872692
// does not understand.
2693+
ProgramStateRef ExprEngine::processPointerEscapedOnBind(
2694+
ProgramStateRef State, ArrayRef<std::pair<SVal, SVal>> LocAndVals,
2695+
const LocationContext *LCtx, PointerEscapeKind Kind,
2696+
const CallEvent *Call) {
2697+
SmallVector<SVal, 8> Escaped;
2698+
for (const std::pair<SVal, SVal> &LocAndVal : LocAndVals) {
2699+
// Cases (1) and (2).
2700+
const MemRegion *MR = LocAndVal.first.getAsRegion();
2701+
if (!MR || !MR->hasStackStorage()) {
2702+
Escaped.push_back(LocAndVal.second);
2703+
continue;
2704+
}
2705+
2706+
// Case (3).
2707+
if (const auto *VR = dyn_cast<VarRegion>(MR->getBaseRegion()))
2708+
if (VR->hasStackParametersStorage() && VR->getStackFrame()->inTopFrame())
2709+
if (const auto *RD = VR->getValueType()->getAsCXXRecordDecl())
2710+
if (!RD->hasTrivialDestructor()) {
2711+
Escaped.push_back(LocAndVal.second);
2712+
continue;
2713+
}
2714+
2715+
// Case (4): in order to test that, generate a new state with the binding
2716+
// added. If it is the same state, then it escapes (since the store cannot
2717+
// represent the binding).
2718+
// Do this only if we know that the store is not supposed to generate the
2719+
// same state.
2720+
SVal StoredVal = State->getSVal(MR);
2721+
if (StoredVal != LocAndVal.second)
2722+
if (State ==
2723+
(State->bindLoc(loc::MemRegionVal(MR), LocAndVal.second, LCtx)))
2724+
Escaped.push_back(LocAndVal.second);
2725+
}
2726+
2727+
if (Escaped.empty())
2728+
return State;
2729+
2730+
return escapeValues(State, Escaped, Kind, Call);
2731+
}
2732+
26882733
ProgramStateRef
26892734
ExprEngine::processPointerEscapedOnBind(ProgramStateRef State, SVal Loc,
26902735
SVal Val, const LocationContext *LCtx) {
2691-
2692-
// Cases (1) and (2).
2693-
const MemRegion *MR = Loc.getAsRegion();
2694-
if (!MR || !MR->hasStackStorage())
2695-
return escapeValue(State, Val, PSK_EscapeOnBind);
2696-
2697-
// Case (3).
2698-
if (const auto *VR = dyn_cast<VarRegion>(MR->getBaseRegion()))
2699-
if (VR->hasStackParametersStorage() && VR->getStackFrame()->inTopFrame())
2700-
if (const auto *RD = VR->getValueType()->getAsCXXRecordDecl())
2701-
if (!RD->hasTrivialDestructor())
2702-
return escapeValue(State, Val, PSK_EscapeOnBind);
2703-
2704-
// Case (4): in order to test that, generate a new state with the binding
2705-
// added. If it is the same state, then it escapes (since the store cannot
2706-
// represent the binding).
2707-
// Do this only if we know that the store is not supposed to generate the
2708-
// same state.
2709-
SVal StoredVal = State->getSVal(MR);
2710-
if (StoredVal != Val)
2711-
if (State == (State->bindLoc(loc::MemRegionVal(MR), Val, LCtx)))
2712-
return escapeValue(State, Val, PSK_EscapeOnBind);
2713-
2714-
return State;
2736+
std::pair<SVal, SVal> LocAndVal(Loc, Val);
2737+
return processPointerEscapedOnBind(State, LocAndVal, LCtx, PSK_EscapeOnBind,
2738+
nullptr);
27152739
}
27162740

27172741
ProgramStateRef

‎clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
102102
state = state->BindExpr(B, LCtx, Result);
103103
} else {
104104
// If we cannot evaluate the operation escape the operands.
105-
state = escapeValue(state, LeftV, PSK_EscapeOther);
106-
state = escapeValue(state, RightV, PSK_EscapeOther);
105+
state = escapeValues(state, LeftV, PSK_EscapeOther);
106+
state = escapeValues(state, RightV, PSK_EscapeOther);
107107
}
108108

109109
Bldr.generateNode(B, *it, state);
@@ -275,7 +275,7 @@ ProgramStateRef ExprEngine::handleLValueBitCast(
275275
V = evalMinus(V);
276276
state = state->BindExpr(CastE, LCtx, V);
277277
if (V.isUnknown() && !OrigV.isUnknown()) {
278-
state = escapeValue(state, OrigV, PSK_EscapeOther);
278+
state = escapeValues(state, OrigV, PSK_EscapeOther);
279279
}
280280
Bldr.generateNode(CastE, Pred, state);
281281

‎clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
#include "clang/AST/Decl.h"
1314
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
1415
#include "PrettyStackTraceLocationContext.h"
1516
#include "clang/AST/CXXInheritance.h"
@@ -592,9 +593,45 @@ void ExprEngine::evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
592593
for (auto I : dstCallEvaluated)
593594
finishArgumentConstruction(dstArgumentCleanup, I, Call);
594595

595-
// Finally, run any post-call checks.
596-
getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
596+
ExplodedNodeSet dstPostCall;
597+
getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup,
597598
Call, *this);
599+
600+
// Escaping symbols conjured during invalidating the regions above.
601+
// Note that, for inlined calls the nodes were put back into the worklist,
602+
// so we can assume that every node belongs to a conservative call at this
603+
// point.
604+
605+
// Run pointerEscape callback with the newly conjured symbols.
606+
SmallVector<std::pair<SVal, SVal>, 8> Escaped;
607+
for (auto I : dstPostCall) {
608+
NodeBuilder B(I, Dst, *currBldrCtx);
609+
ProgramStateRef State = I->getState();
610+
Escaped.clear();
611+
{
612+
unsigned Arg = -1;
613+
for (const ParmVarDecl *PVD : Call.parameters()) {
614+
++Arg;
615+
QualType ParamTy = PVD->getType();
616+
if (ParamTy.isNull() ||
617+
(!ParamTy->isPointerType() && !ParamTy->isReferenceType()))
618+
continue;
619+
QualType Pointee = ParamTy->getPointeeType();
620+
if (Pointee.isConstQualified() || Pointee->isVoidType())
621+
continue;
622+
if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion())
623+
Escaped.emplace_back(loc::MemRegionVal(MR), State->getSVal(MR, Pointee));
624+
}
625+
}
626+
627+
State = processPointerEscapedOnBind(State, Escaped, I->getLocationContext(),
628+
PSK_EscapeOutParameters, &Call);
629+
630+
if (State == I->getState())
631+
Dst.insert(I);
632+
else
633+
B.generateNode(I->getLocation(), State, I);
634+
}
598635
}
599636

600637
ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
@@ -670,8 +707,7 @@ ProgramStateRef ExprEngine::bindReturnValue(const CallEvent &Call,
670707
// Conservatively evaluate call by invalidating regions and binding
671708
// a conjured return value.
672709
void ExprEngine::conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr,
673-
ExplodedNode *Pred,
674-
ProgramStateRef State) {
710+
ExplodedNode *Pred, ProgramStateRef State) {
675711
State = Call.invalidateRegions(currBldrCtx->blockCount(), State);
676712
State = bindReturnValue(Call, Pred->getLocationContext(), State);
677713

‎clang/test/Analysis/analyzer-config.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
// CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
3838
// CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
3939
// CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
40+
// CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false
4041
// CHECK-NEXT: debug.AnalysisOrder:PostCall = false
4142
// CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
4243
// CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
@@ -97,4 +98,4 @@
9798
// CHECK-NEXT: unroll-loops = false
9899
// CHECK-NEXT: widen-loops = false
99100
// CHECK-NEXT: [stats]
100-
// CHECK-NEXT: num-entries = 94
101+
// CHECK-NEXT: num-entries = 95

‎clang/test/Analysis/expr-inspection.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,5 @@ struct S {
5050

5151
void test_field_dumps(struct S s, struct S *p) {
5252
clang_analyzer_dump_pointer(&s.x); // expected-warning{{&s.x}}
53-
clang_analyzer_dump_pointer(&p->x); // expected-warning{{&SymRegion{reg_$0<struct S * p>}.x}}
53+
clang_analyzer_dump_pointer(&p->x); // expected-warning{{&SymRegion{reg_$1<struct S * p>}.x}}
5454
}

‎clang/test/Analysis/pointer-escape-on-conservative-calls.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: %clang_cc1 -analyze -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:PointerEscape=true -analyzer-config debug.AnalysisOrder:PostCall=true %s 2>&1 | FileCheck %s
2+
3+
4+
void f(int *);
5+
int *getMem();
6+
7+
int main() {
8+
f(getMem());
9+
return 0;
10+
}
11+
12+
// CHECK: PostCall (f)
13+
// CHECK-NEXT: PointerEscape

0 commit comments

Comments
 (0)