Author: Nithin Vadukkumchery Rajendrakumar Date: 2020-08-19T00:03:31+02:00 New Revision: b34b1e38381fa4d1b1d9751a6b5233b68e734cfe
URL: https://github.com/llvm/llvm-project/commit/b34b1e38381fa4d1b1d9751a6b5233b68e734cfe DIFF: https://github.com/llvm/llvm-project/commit/b34b1e38381fa4d1b1d9751a6b5233b68e734cfe.diff LOG: [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 Added: Modified: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp clang/test/Analysis/smart-ptr-text-output.cpp clang/test/Analysis/smart-ptr.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 38a680eb04c00..802bc934cfb06 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -602,11 +602,11 @@ void ExprEngine::handleConstructor(const Expr *E, *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 @@ void ExprEngine::handleConstructor(const Expr *E, // 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 @@ void ExprEngine::handleConstructor(const Expr *E, } ExplodedNodeSet DstPostArgumentCleanup; - for (ExplodedNode *I : DstEvaluated) + for (ExplodedNode *I : DstEvaluatedPostProcessed) finishArgumentConstruction(DstPostArgumentCleanup, I, *Call); // If there were other constructors called for object-type arguments diff --git a/clang/test/Analysis/smart-ptr-text-output.cpp b/clang/test/Analysis/smart-ptr-text-output.cpp index 5280d0021884d..1132a37fa6679 100644 --- a/clang/test/Analysis/smart-ptr-text-output.cpp +++ b/clang/test/Analysis/smart-ptr-text-output.cpp @@ -36,14 +36,15 @@ void derefAfterCtrWithNullVariable() { } 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 derefAfterReset() { 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 derefOnReleasedNullRawPtr() { } 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 @@ void derefOnSwappedNullPtr() { // 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'}} } diff --git a/clang/test/Analysis/smart-ptr.cpp b/clang/test/Analysis/smart-ptr.cpp index f72a918aee203..bcf1e569d690a 100644 --- a/clang/test/Analysis/smart-ptr.cpp +++ b/clang/test/Analysis/smart-ptr.cpp @@ -41,6 +41,7 @@ A *return_null() { void derefAfterValidCtr() { std::unique_ptr<A> P(new A()); + clang_analyzer_numTimesReached(); // expected-warning {{1}} P->foo(); // No warning. } @@ -50,17 +51,20 @@ void derefOfUnknown(std::unique_ptr<A> P) { 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 derefAfterResetWithNull() { 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_const_rvalue_ref(const std::unique_ptr<A> &&a); 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_const_rvalue_ref(const StructWithSmartPtr &&a 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 @@ void derefOnSwappedNullPtr() { (*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()); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits