This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb34b1e38381f: [Analysis] Bug fix for exploded graph
branching in evalCall for constructor (authored by vrnithinkumar).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85796/new/
https://reviews.llvm.org/D85796
Files:
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/test/Analysis/smart-ptr-text-output.cpp
clang/test/Analysis/smart-ptr.cpp
Index: clang/test/Analysis/smart-ptr.cpp
===================================================================
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -41,6 +41,7 @@
void derefAfterValidCtr() {
std::unique_ptr<A> P(new A());
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
P->foo(); // No warning.
}
@@ -50,17 +51,20 @@
void derefAfterDefaultCtr() {
std::unique_ptr<A> P;
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
}
void derefAfterCtrWithNull() {
std::unique_ptr<A> P(nullptr);
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
*P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
}
void derefAfterCtrWithNullVariable() {
A *InnerPtr = nullptr;
std::unique_ptr<A> P(InnerPtr);
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
}
@@ -87,6 +91,7 @@
void derefAfterResetWithNonNull() {
std::unique_ptr<A> P;
P.reset(new A());
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
P->foo(); // No warning.
}
@@ -116,37 +121,40 @@
void pass_smart_ptr_by_ptr(std::unique_ptr<A> *a);
void pass_smart_ptr_by_const_ptr(const std::unique_ptr<A> *a);
-void regioninvalidationTest() {
- {
- std::unique_ptr<A> P;
- pass_smart_ptr_by_ref(P);
- P->foo(); // no-warning
- }
- {
- std::unique_ptr<A> P;
- pass_smart_ptr_by_const_ref(P);
- P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
- }
- {
- std::unique_ptr<A> P;
- pass_smart_ptr_by_rvalue_ref(std::move(P));
- P->foo(); // no-warning
- }
- {
- std::unique_ptr<A> P;
- pass_smart_ptr_by_const_rvalue_ref(std::move(P));
- P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
- }
- {
- std::unique_ptr<A> P;
- pass_smart_ptr_by_ptr(&P);
- P->foo();
- }
- {
- std::unique_ptr<A> P;
- pass_smart_ptr_by_const_ptr(&P);
- P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
- }
+void regioninvalidationWithPassByRef() {
+ std::unique_ptr<A> P;
+ pass_smart_ptr_by_ref(P);
+ P->foo(); // no-warning
+}
+
+void regioninvalidationWithPassByCostRef() {
+ std::unique_ptr<A> P;
+ pass_smart_ptr_by_const_ref(P);
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationWithPassByRValueRef() {
+ std::unique_ptr<A> P;
+ pass_smart_ptr_by_rvalue_ref(std::move(P));
+ P->foo(); // no-warning
+}
+
+void regioninvalidationWithPassByConstRValueRef() {
+ std::unique_ptr<A> P;
+ pass_smart_ptr_by_const_rvalue_ref(std::move(P));
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationWithPassByPtr() {
+ std::unique_ptr<A> P;
+ pass_smart_ptr_by_ptr(&P);
+ P->foo();
+}
+
+void regioninvalidationWithPassByConstPtr() {
+ std::unique_ptr<A> P;
+ pass_smart_ptr_by_const_ptr(&P);
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
}
struct StructWithSmartPtr {
@@ -160,37 +168,40 @@
void pass_struct_with_smart_ptr_by_ptr(StructWithSmartPtr *a);
void pass_struct_with_smart_ptr_by_const_ptr(const StructWithSmartPtr *a);
-void regioninvalidationTestWithinStruct() {
- {
- StructWithSmartPtr S;
- pass_struct_with_smart_ptr_by_ref(S);
- S.P->foo(); // no-warning
- }
- {
- StructWithSmartPtr S;
- pass_struct_with_smart_ptr_by_const_ref(S);
- S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
- }
- {
- StructWithSmartPtr S;
- pass_struct_with_smart_ptr_by_rvalue_ref(std::move(S));
- S.P->foo(); // no-warning
- }
- {
- StructWithSmartPtr S;
- pass_struct_with_smart_ptr_by_const_rvalue_ref(std::move(S));
- S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
- }
- {
- StructWithSmartPtr S;
- pass_struct_with_smart_ptr_by_ptr(&S);
- S.P->foo();
- }
- {
- StructWithSmartPtr S;
- pass_struct_with_smart_ptr_by_const_ptr(&S);
- S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
- }
+void regioninvalidationWithinStructPassByRef() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_ref(S);
+ S.P->foo(); // no-warning
+}
+
+void regioninvalidationWithinStructPassByConstRef() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_const_ref(S);
+ S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationWithinStructPassByRValueRef() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_rvalue_ref(std::move(S));
+ S.P->foo(); // no-warning
+}
+
+void regioninvalidationWithinStructPassByConstRValueRef() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_const_rvalue_ref(std::move(S));
+ S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void regioninvalidationWithinStructPassByPtr() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_ptr(&S);
+ S.P->foo(); // no-warning
+}
+
+void regioninvalidationWithinStructPassByConstPtr() {
+ StructWithSmartPtr S;
+ pass_struct_with_smart_ptr_by_const_ptr(&S);
+ S.P->foo(); // expected-warning {{Dereference of null smart pointer 'S.P' [alpha.cplusplus.SmartPtr]}}
}
void derefAfterAssignment() {
@@ -217,14 +228,20 @@
(*P).foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
}
-void derefOnStdSwappedNullPtr() {
+void derefOnFirstStdSwappedNullPtr() {
std::unique_ptr<A> P;
std::unique_ptr<A> PNull;
std::swap(P, PNull);
- PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
}
+void derefOnSecondStdSwappedNullPtr() {
+ std::unique_ptr<A> P;
+ std::unique_ptr<A> PNull;
+ std::swap(P, PNull);
+ PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
void derefOnSwappedValidPtr() {
std::unique_ptr<A> P(new A());
std::unique_ptr<A> PValid(new A());
Index: clang/test/Analysis/smart-ptr-text-output.cpp
===================================================================
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -36,14 +36,15 @@
}
void derefAfterRelease() {
- std::unique_ptr<A> P(new A());
+ std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
+ // FIXME: should mark region as uninteresting after release, so above note will not be there
P.release(); // expected-note {{Smart pointer 'P' is released and set to null}}
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
// expected-note@-1{{Dereference of null smart pointer 'P'}}
}
void derefAfterReset() {
- std::unique_ptr<A> P(new A());
+ std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
// expected-note@-1{{Dereference of null smart pointer 'P'}}
@@ -51,7 +52,7 @@
void derefAfterResetWithNull() {
A *NullInnerPtr = nullptr; // expected-note {{'NullInnerPtr' initialized to a null pointer value}}
- std::unique_ptr<A> P(new A());
+ std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
P.reset(NullInnerPtr); // expected-note {{Smart pointer 'P' reset using a null value}}
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
// expected-note@-1{{Dereference of null smart pointer 'P'}}
@@ -67,7 +68,7 @@
}
void derefOnSwappedNullPtr() {
- std::unique_ptr<A> P(new A());
+ std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
P.swap(PNull); // expected-note {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
PNull->foo(); // No warning.
@@ -77,13 +78,11 @@
// FIXME: Fix this test when "std::swap" is modeled seperately.
void derefOnStdSwappedNullPtr() {
- std::unique_ptr<A> P;
+ std::unique_ptr<A> P; // expected-note {{Default constructed smart pointer 'P' is null}}
std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:978 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
// expected-note@-1 {{Calling 'swap<A>'}}
// expected-note@-2 {{Returning from 'swap<A>'}}
- PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
- // expected-note@-1{{Dereference of null smart pointer 'PNull'}}
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
// expected-note@-1{{Dereference of null smart pointer 'P'}}
}
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -602,11 +602,11 @@
*Call, *this);
ExplodedNodeSet DstEvaluated;
- StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
if (CE && CE->getConstructor()->isTrivial() &&
CE->getConstructor()->isCopyOrMoveConstructor() &&
!CallOpts.IsArrayCtorOrDtor) {
+ StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
// FIXME: Handle other kinds of trivial constructors as well.
for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
I != E; ++I)
@@ -626,6 +626,8 @@
// in the CFG, would be called at the end of the full expression or
// later (for life-time extended temporaries) -- but avoids infeasible
// paths when no-return temporary destructors are used for assertions.
+ ExplodedNodeSet DstEvaluatedPostProcessed;
+ StmtNodeBuilder Bldr(DstEvaluated, DstEvaluatedPostProcessed, *currBldrCtx);
const AnalysisDeclContext *ADC = LCtx->getAnalysisDeclContext();
if (!ADC->getCFGBuildOptions().AddTemporaryDtors) {
if (llvm::isa_and_nonnull<CXXTempObjectRegion>(TargetRegion) &&
@@ -655,7 +657,7 @@
}
ExplodedNodeSet DstPostArgumentCleanup;
- for (ExplodedNode *I : DstEvaluated)
+ for (ExplodedNode *I : DstEvaluatedPostProcessed)
finishArgumentConstruction(DstPostArgumentCleanup, I, *Call);
// If there were other constructors called for object-type arguments
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
