vrnithinkumar updated this revision to Diff 285514.
vrnithinkumar added a comment.
- Addressing review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85796/new/
https://reviews.llvm.org/D85796
Files:
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.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/ExprEngineCallAndReturn.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -583,8 +583,8 @@
// to see if the can evaluate the function call, and get a callback at
// defaultEvalCall if all of them fail.
ExplodedNodeSet dstCallEvaluated;
- getCheckerManager().runCheckersForEvalCall(dstCallEvaluated, dstPreVisit,
- Call, *this, EvalCallOptions());
+ getCheckerManager().runCheckersForEvalCall(
+ dstCallEvaluated, dstPreVisit, Call, *this, EvalCallOptions(), None);
// If there were other constructors called for object-type arguments
// of this call, clean them up.
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -616,7 +616,7 @@
for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
I != E; ++I)
getCheckerManager().runCheckersForEvalCall(DstEvaluated, *I, *Call, *this,
- CallOpts);
+ CallOpts, Bldr);
}
// If the CFG was constructed without elements for temporary destructors
Index: clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -654,12 +654,14 @@
const ExplodedNodeSet &Src,
const CallEvent &Call,
ExprEngine &Eng,
- const EvalCallOptions &CallOpts) {
+ const EvalCallOptions &CallOpts,
+ llvm::Optional<NodeBuilder> B) {
for (auto *const Pred : Src) {
bool anyEvaluated = false;
ExplodedNodeSet checkDst;
- NodeBuilder B(Pred, checkDst, Eng.getBuilderContext());
+ auto CurrentNodeBldr =
+ B ? B.getValue() : NodeBuilder(Pred, checkDst, Eng.getBuilderContext());
// Check if any of the EvalCall callbacks can evaluate the call.
for (const auto &EvalCallChecker : EvalCallCheckers) {
@@ -672,7 +674,7 @@
{ // CheckerContext generates transitions(populates checkDest) on
// destruction, so introduce the scope to make sure it gets properly
// populated.
- CheckerContext C(B, Eng, Pred, L);
+ CheckerContext C(CurrentNodeBldr, Eng, Pred, L);
evaluated = EvalCallChecker(Call, C);
}
assert(!(evaluated && anyEvaluated)
Index: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -51,6 +51,7 @@
struct EvalCallOptions;
class MemRegion;
struct NodeBuilderContext;
+class NodeBuilder;
class ObjCMethodCall;
class RegionAndSymbolInvalidationTraits;
class SVal;
@@ -439,7 +440,8 @@
/// Warning: Currently, the CallEvent MUST come from a CallExpr!
void runCheckersForEvalCall(ExplodedNodeSet &Dst, const ExplodedNodeSet &Src,
const CallEvent &CE, ExprEngine &Eng,
- const EvalCallOptions &CallOpts);
+ const EvalCallOptions &CallOpts,
+ llvm::Optional<NodeBuilder> B);
/// Run checkers for the entire Translation Unit.
void runCheckersOnEndOfTranslationUnit(const TranslationUnitDecl *TU,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
