Skip to content

Commit b34b1e3

Browse files
vrnithinkumarvrnithinkumar
committed
[Analysis] Bug fix for exploded graph branching in evalCall for constructor
Summary: Make exactly single NodeBuilder exists at any given time Reviewers: NoQ, Szelethus, vsavchenko, xazax.hun Reviewed By: NoQ Subscribers: martong, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D85796
1 parent 78bd423 commit b34b1e3

File tree

3 files changed

+91
-73
lines changed

3 files changed

+91
-73
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -602,11 +602,11 @@ void ExprEngine::handleConstructor(const Expr *E,
602602
*Call, *this);
603603

604604
ExplodedNodeSet DstEvaluated;
605-
StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
606605

607606
if (CE && CE->getConstructor()->isTrivial() &&
608607
CE->getConstructor()->isCopyOrMoveConstructor() &&
609608
!CallOpts.IsArrayCtorOrDtor) {
609+
StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
610610
// FIXME: Handle other kinds of trivial constructors as well.
611611
for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
612612
I != E; ++I)
@@ -626,6 +626,8 @@ void ExprEngine::handleConstructor(const Expr *E,
626626
// in the CFG, would be called at the end of the full expression or
627627
// later (for life-time extended temporaries) -- but avoids infeasible
628628
// paths when no-return temporary destructors are used for assertions.
629+
ExplodedNodeSet DstEvaluatedPostProcessed;
630+
StmtNodeBuilder Bldr(DstEvaluated, DstEvaluatedPostProcessed, *currBldrCtx);
629631
const AnalysisDeclContext *ADC = LCtx->getAnalysisDeclContext();
630632
if (!ADC->getCFGBuildOptions().AddTemporaryDtors) {
631633
if (llvm::isa_and_nonnull<CXXTempObjectRegion>(TargetRegion) &&
@@ -655,7 +657,7 @@ void ExprEngine::handleConstructor(const Expr *E,
655657
}
656658

657659
ExplodedNodeSet DstPostArgumentCleanup;
658-
for (ExplodedNode *I : DstEvaluated)
660+
for (ExplodedNode *I : DstEvaluatedPostProcessed)
659661
finishArgumentConstruction(DstPostArgumentCleanup, I, *Call);
660662

661663
// If there were other constructors called for object-type arguments

‎clang/test/Analysis/smart-ptr-text-output.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,23 @@ void derefAfterCtrWithNullVariable() {
3636
}
3737

3838
void derefAfterRelease() {
39-
std::unique_ptr<A> P(new A());
39+
std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
40+
// FIXME: should mark region as uninteresting after release, so above note will not be there
4041
P.release(); // expected-note {{Smart pointer 'P' is released and set to null}}
4142
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
4243
// expected-note@-1{{Dereference of null smart pointer 'P'}}
4344
}
4445

4546
void derefAfterReset() {
46-
std::unique_ptr<A> P(new A());
47+
std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
4748
P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
4849
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
4950
// expected-note@-1{{Dereference of null smart pointer 'P'}}
5051
}
5152

5253
void derefAfterResetWithNull() {
5354
A *NullInnerPtr = nullptr; // expected-note {{'NullInnerPtr' initialized to a null pointer value}}
54-
std::unique_ptr<A> P(new A());
55+
std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
5556
P.reset(NullInnerPtr); // expected-note {{Smart pointer 'P' reset using a null value}}
5657
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
5758
// expected-note@-1{{Dereference of null smart pointer 'P'}}
@@ -67,7 +68,7 @@ void derefOnReleasedNullRawPtr() {
6768
}
6869

6970
void derefOnSwappedNullPtr() {
70-
std::unique_ptr<A> P(new A());
71+
std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
7172
std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
7273
P.swap(PNull); // expected-note {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
7374
PNull->foo(); // No warning.
@@ -77,13 +78,11 @@ void derefOnSwappedNullPtr() {
7778

7879
// FIXME: Fix this test when "std::swap" is modeled seperately.
7980
void derefOnStdSwappedNullPtr() {
80-
std::unique_ptr<A> P;
81+
std::unique_ptr<A> P; // expected-note {{Default constructed smart pointer 'P' is null}}
8182
std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
8283
std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:978 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
8384
// expected-note@-1 {{Calling 'swap<A>'}}
8485
// expected-note@-2 {{Returning from 'swap<A>'}}
85-
PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
86-
// expected-note@-1{{Dereference of null smart pointer 'PNull'}}
8786
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
8887
// expected-note@-1{{Dereference of null smart pointer 'P'}}
8988
}

‎clang/test/Analysis/smart-ptr.cpp

Lines changed: 81 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ A *return_null() {
4141

4242
void derefAfterValidCtr() {
4343
std::unique_ptr<A> P(new A());
44+
clang_analyzer_numTimesReached(); // expected-warning {{1}}
4445
P->foo(); // No warning.
4546
}
4647

@@ -50,17 +51,20 @@ void derefOfUnknown(std::unique_ptr<A> P) {
5051

5152
void derefAfterDefaultCtr() {
5253
std::unique_ptr<A> P;
54+
clang_analyzer_numTimesReached(); // expected-warning {{1}}
5355
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
5456
}
5557

5658
void derefAfterCtrWithNull() {
5759
std::unique_ptr<A> P(nullptr);
60+
clang_analyzer_numTimesReached(); // expected-warning {{1}}
5861
*P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
5962
}
6063

6164
void derefAfterCtrWithNullVariable() {
6265
A *InnerPtr = nullptr;
6366
std::unique_ptr<A> P(InnerPtr);
67+
clang_analyzer_numTimesReached(); // expected-warning {{1}}
6468
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
6569
}
6670

@@ -87,6 +91,7 @@ void derefAfterResetWithNull() {
8791
void derefAfterResetWithNonNull() {
8892
std::unique_ptr<A> P;
8993
P.reset(new A());
94+
clang_analyzer_numTimesReached(); // expected-warning {{1}}
9095
P->foo(); // No warning.
9196
}
9297

@@ -116,37 +121,40 @@ void pass_smart_ptr_by_const_rvalue_ref(const std::unique_ptr<A> &&a);
116121
void pass_smart_ptr_by_ptr(std::unique_ptr<A> *a);
117122
void pass_smart_ptr_by_const_ptr(const std::unique_ptr<A> *a);
118123

119-
void regioninvalidationTest() {
120-
{
121-
std::unique_ptr<A> P;
122-
pass_smart_ptr_by_ref(P);
123-
P->foo(); // no-warning
124-
}
125-
{
126-
std::unique_ptr<A> P;
127-
pass_smart_ptr_by_const_ref(P);
128-
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
129-
}
130-
{
131-
std::unique_ptr<A> P;
132-
pass_smart_ptr_by_rvalue_ref(std::move(P));
133-
P->foo(); // no-warning
134-
}
135-
{
136-
std::unique_ptr<A> P;
137-
pass_smart_ptr_by_const_rvalue_ref(std::move(P));
138-
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
139-
}
140-
{
141-
std::unique_ptr<A> P;
142-
pass_smart_ptr_by_ptr(&P);
143-
P->foo();
144-
}
145-
{
146-
std::unique_ptr<A> P;
147-
pass_smart_ptr_by_const_ptr(&P);
148-
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
149-
}
124+
void regioninvalidationWithPassByRef() {
125+
std::unique_ptr<A> P;
126+
pass_smart_ptr_by_ref(P);
127+
P->foo(); // no-warning
128+
}
129+
130+
void regioninvalidationWithPassByCostRef() {
131+
std::unique_ptr<A> P;
132+
pass_smart_ptr_by_const_ref(P);
133+
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
134+
}
135+
136+
void regioninvalidationWithPassByRValueRef() {
137+
std::unique_ptr<A> P;
138+
pass_smart_ptr_by_rvalue_ref(std::move(P));
139+
P->foo(); // no-warning
140+
}
141+
142+
void regioninvalidationWithPassByConstRValueRef() {
143+
std::unique_ptr<A> P;
144+
pass_smart_ptr_by_const_rvalue_ref(std::move(P));
145+
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
146+
}
147+
148+
void regioninvalidationWithPassByPtr() {
149+
std::unique_ptr<A> P;
150+
pass_smart_ptr_by_ptr(&P);
151+
P->foo();
152+
}
153+
154+
void regioninvalidationWithPassByConstPtr() {
155+
std::unique_ptr<A> P;
156+
pass_smart_ptr_by_const_ptr(&P);
157+
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
150158
}
151159

152160
struct StructWithSmartPtr {
@@ -160,37 +168,40 @@ void pass_struct_with_smart_ptr_by_const_rvalue_ref(const StructWithSmartPtr &&a
160168
void pass_struct_with_smart_ptr_by_ptr(StructWithSmartPtr *a);
161169
void pass_struct_with_smart_ptr_by_const_ptr(const StructWithSmartPtr *a);
162170

163-
void regioninvalidationTestWithinStruct() {
164-
{
165-
StructWithSmartPtr S;
166-
pass_struct_with_smart_ptr_by_ref(S);
167-
S.P->foo(); // no-warning
168-
}
169-
{
170-
StructWithSmartPtr S;
171-
pass_struct_with_smart_ptr_by_const_ref(S);
172-
S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
173-
}
174-
{
175-
StructWithSmartPtr S;
176-
pass_struct_with_smart_ptr_by_rvalue_ref(std::move(S));
177-
S.P->foo(); // no-warning
178-
}
179-
{
180-
StructWithSmartPtr S;
181-
pass_struct_with_smart_ptr_by_const_rvalue_ref(std::move(S));
182-
S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
183-
}
184-
{
185-
StructWithSmartPtr S;
186-
pass_struct_with_smart_ptr_by_ptr(&S);
187-
S.P->foo();
188-
}
189-
{
190-
StructWithSmartPtr S;
191-
pass_struct_with_smart_ptr_by_const_ptr(&S);
192-
S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
193-
}
171+
void regioninvalidationWithinStructPassByRef() {
172+
StructWithSmartPtr S;
173+
pass_struct_with_smart_ptr_by_ref(S);
174+
S.P->foo(); // no-warning
175+
}
176+
177+
void regioninvalidationWithinStructPassByConstRef() {
178+
StructWithSmartPtr S;
179+
pass_struct_with_smart_ptr_by_const_ref(S);
180+
S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
181+
}
182+
183+
void regioninvalidationWithinStructPassByRValueRef() {
184+
StructWithSmartPtr S;
185+
pass_struct_with_smart_ptr_by_rvalue_ref(std::move(S));
186+
S.P->foo(); // no-warning
187+
}
188+
189+
void regioninvalidationWithinStructPassByConstRValueRef() {
190+
StructWithSmartPtr S;
191+
pass_struct_with_smart_ptr_by_const_rvalue_ref(std::move(S));
192+
S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
193+
}
194+
195+
void regioninvalidationWithinStructPassByPtr() {
196+
StructWithSmartPtr S;
197+
pass_struct_with_smart_ptr_by_ptr(&S);
198+
S.P->foo(); // no-warning
199+
}
200+
201+
void regioninvalidationWithinStructPassByConstPtr() {
202+
StructWithSmartPtr S;
203+
pass_struct_with_smart_ptr_by_const_ptr(&S);
204+
S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
194205
}
195206

196207
void derefAfterAssignment() {
@@ -217,14 +228,20 @@ void derefOnSwappedNullPtr() {
217228
(*P).foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
218229
}
219230

220-
void derefOnStdSwappedNullPtr() {
231+
void derefOnFirstStdSwappedNullPtr() {
221232
std::unique_ptr<A> P;
222233
std::unique_ptr<A> PNull;
223234
std::swap(P, PNull);
224-
PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
225235
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
226236
}
227237

238+
void derefOnSecondStdSwappedNullPtr() {
239+
std::unique_ptr<A> P;
240+
std::unique_ptr<A> PNull;
241+
std::swap(P, PNull);
242+
PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
243+
}
244+
228245
void derefOnSwappedValidPtr() {
229246
std::unique_ptr<A> P(new A());
230247
std::unique_ptr<A> PValid(new A());

0 commit comments

Comments
 (0)