[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr
vrnithinkumar created this revision. Herald added subscribers: cfe-commits, steakhal, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project: clang. vrnithinkumar requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D86293 Files: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.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 @@ -215,8 +215,7 @@ std::unique_ptr P; std::unique_ptr Q; Q = std::move(P); -// TODO: Fix test with expecting warning after '=' operator overloading modeling. -Q->foo(); // no-warning +Q->foo(); // expected-warning {{Dereference of null smart pointer 'Q' [alpha.cplusplus.SmartPtr]}} } } @@ -252,3 +251,36 @@ P->foo(); // No warning. PValid->foo(); // No warning. } + +void drefOnMovedFromValidPtr() { + std::unique_ptr PToMove(new A()); + std::unique_ptr P; + P = std::move(PToMove); + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} +} + +void drefOnMovedToNullPtr() { + std::unique_ptr PToMove(new A()); + std::unique_ptr P; + P = std::move(PToMove); // No note. + P->foo(); // No warning. +} + +void derefOnNullPtrGotMovedFromValidPtr() { + std::unique_ptr P(new A()); + std::unique_ptr PToMove; + P = std::move(PToMove); + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void drefOnMovedFromUnknownPtr(std::unique_ptr PToMove) { + std::unique_ptr P; + P = std::move(PToMove); + P->foo(); // No warning. +} + +void drefOnMovedUnknownPtr(std::unique_ptr PToMove) { + std::unique_ptr P; + P = std::move(PToMove); + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} +} 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 @@ -109,10 +109,34 @@ // expected-note@-1{{Dereference of null smart pointer 'P'}} } -void noNoteTagsForNonMatchingBugType() { - std::unique_ptr P; // No note. - std::unique_ptr P1; // No note. - P1 = std::move(P); // expected-note {{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}} - P->foo(); // expected-warning {{Dereference of null smart pointer 'P' of type 'std::unique_ptr' [cplusplus.Move]}} - // expected-note@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}} +void drefOnMovedFromValidPtr() { + std::unique_ptr PToMove(new A()); // expected-note {{Smart pointer 'PToMove' is constructed}} + // FIXME: above note should go away once we fix marking region not interested. + std::unique_ptr P; + P = std::move(PToMove); // expected-note {{Smart pointer 'PToMove' is null after moved to 'P'}} + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1 {{Dereference of null smart pointer 'PToMove'}} +} + +void drefOnMovedToNullPtr() { + std::unique_ptr PToMove(new A()); + std::unique_ptr P; + P = std::move(PToMove); // No note. + P->foo(); // No warning. +} + +void derefOnNullPtrGotMovedFromValidPtr() { + std::unique_ptr P(new A()); // expected-note {{Smart pointer 'P' is constructed}} + // FIXME: above note should go away once we fix marking region not interested. + std::unique_ptr PToMove; // expected-note {{Default constructed smart pointer 'PToMove' is null}} + P = std::move(PToMove); // expected-note {{Smart pointer 'P' is null after a null value moved from 'PToMove'}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1 {{Dereference of null smart pointer 'P'}} +} + +void drefOnMovedUnknownPtr(std::unique_ptr PToMove) { + std::unique_ptr P; + P = std::move(PToMove); + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1 {{Dereference of null smart pointer 'PToMove'}} } Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp === --- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp +++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp @@ -56,6 +56,7 @@ void handleReset(const CallEvent &Call, CheckerContext &C) const; void handleRelease(const CallEvent &Call, CheckerContext &C) const; void handleSwap(const CallEvent &Call, CheckerContext &C) const; + bool handleEqOp(const CallEvent &Call, CheckerContext &C) const; usin
[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr
vrnithinkumar updated this revision to Diff 286868. vrnithinkumar added a comment. - Add assignment to nullptr case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86293/new/ https://reviews.llvm.org/D86293 Files: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp clang/test/Analysis/Inputs/system-header-simulator-cxx.h 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 @@ -215,8 +215,7 @@ std::unique_ptr P; std::unique_ptr Q; Q = std::move(P); -// TODO: Fix test with expecting warning after '=' operator overloading modeling. -Q->foo(); // no-warning +Q->foo(); // expected-warning {{Dereference of null smart pointer 'Q' [alpha.cplusplus.SmartPtr]}} } } @@ -252,3 +251,61 @@ P->foo(); // No warning. PValid->foo(); // No warning. } + +void derefOnMovedFromValidPtr() { + std::unique_ptr PToMove(new A()); + std::unique_ptr P; + P = std::move(PToMove); + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnMovedToNullPtr() { + std::unique_ptr PToMove(new A()); + std::unique_ptr P; + P = std::move(PToMove); // No note. + P->foo(); // No warning. +} + +void derefOnNullPtrGotMovedFromValidPtr() { + std::unique_ptr P(new A()); + std::unique_ptr PToMove; + P = std::move(PToMove); + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnMovedFromUnknownPtr(std::unique_ptr PToMove) { + std::unique_ptr P; + P = std::move(PToMove); + P->foo(); // No warning. +} + +void derefOnMovedUnknownPtr(std::unique_ptr PToMove) { + std::unique_ptr P; + P = std::move(PToMove); + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnAssignedNullPtrToNullSmartPtr() { + std::unique_ptr P; + P = nullptr; + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnAssignedZeroToNullSmartPtr() { + std::unique_ptr P(new A()); + P = 0; + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnAssignedNullToUnknowSmartPtr(std::unique_ptr P) { + P = nullptr; + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +std::unique_ptr &&returnRValRefOfUniquePtr(); + +void drefOnAssignedNullFromMethodPtrValidSmartPtr() { + std::unique_ptr P(new A()); + P = returnRValRefOfUniquePtr(); + P->foo(); // No warning. +} 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 @@ -80,7 +80,7 @@ void derefOnStdSwappedNullPtr() { std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}} std::unique_ptr 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'}} + std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}} // expected-note@-1 {{Calling 'swap'}} // expected-note@-2 {{Returning from 'swap'}} P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} @@ -109,10 +109,49 @@ // expected-note@-1{{Dereference of null smart pointer 'P'}} } -void noNoteTagsForNonMatchingBugType() { - std::unique_ptr P; // No note. - std::unique_ptr P1; // No note. - P1 = std::move(P); // expected-note {{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}} - P->foo(); // expected-warning {{Dereference of null smart pointer 'P' of type 'std::unique_ptr' [cplusplus.Move]}} - // expected-note@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}} +void derefOnMovedFromValidPtr() { + std::unique_ptr PToMove(new A()); // expected-note {{Smart pointer 'PToMove' is constructed}} + // FIXME: above note should go away once we fix marking region not interested. + std::unique_ptr P; + P = std::move(PToMove); // expected-note {{Smart pointer 'PToMove' is null after moved to 'P'}} + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1 {{Dereference of null smart pointer 'PToMove'}} +} + +void derefOnMovedToNullPtr() { + std::unique_ptr PToMove(new A()); + std::unique_ptr P; + P = std::move(PToMove); // No note. + P->foo(); // No warning. +} +
[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr
NoQ added a comment. This looks outright correct to me. I have random suggestions on note text here and there. Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:132 + std::unique_ptr PToMove; // expected-note {{Default constructed smart pointer 'PToMove' is null}} + P = std::move(PToMove); // expected-note {{Smart pointer 'P' is null after a null value moved from 'PToMove'}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} I suggest: `Null pointer value move-assigned to 'P'`. Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:139 + std::unique_ptr P; + P = std::move(PToMove); // expected-note {{Smart pointer 'PToMove' is null after moved and assigned to 'P'}} + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} I suggest: `Smart pointer 'PToMove' is null; previous value moved to 'P'`. Or maybe instead keep the note that the move-checker currently emits? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86293/new/ https://reviews.llvm.org/D86293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr
xazax.hun added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:351 +bool SmartPtrModeling::handleEqOp(const CallEvent &Call, + CheckerContext &C) const { I'd prefer to call this AssignOp to avoid confusion with `==`. While your naming is correct, I always found this nomenclature confusing. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:384 + + const auto *ArgRegionVal = State->get(ArgRegion); + if (ArgRegionVal) { I also find the names of the variables confusing. Instead of `ArgRegion` what about `OtherSmartPtrRegion`? Instead of `ArgRegionVal` what about `OtherInnerPtr`? Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:391 + +C.addTransition(State, C.getNoteTag([ThisRegion, ArgRegion, IsArgValNull]( +PathSensitiveBugReport &BR, Adding return after every `addTransition` is a good practive that we should follow. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:419 +llvm::raw_ostream &OS) { + if (&BR.getBugType() != smartptr::getNullDereferenceBugType() || + !BR.isInteresting(ArgRegion)) Isn't this the same as the beginning of the note tag above? I wonder if there is a way to deduplicate this code. Not a huge priority though as I do not have an immediate idea for doing this in a clean way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86293/new/ https://reviews.llvm.org/D86293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr
vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:351 +bool SmartPtrModeling::handleEqOp(const CallEvent &Call, + CheckerContext &C) const { xazax.hun wrote: > I'd prefer to call this AssignOp to avoid confusion with `==`. While your > naming is correct, I always found this nomenclature confusing. IMO it's not a question of preference with this one, `EqOp` is misleading. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:399 +ArgRegion->printPretty(OS); +OS << " is null after moved to "; +ThisRegion->printPretty(OS); The grammar seems off in this one. I think it should be smith like "after being moved to" or "after it was moved to". Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:132 + std::unique_ptr PToMove; // expected-note {{Default constructed smart pointer 'PToMove' is null}} + P = std::move(PToMove); // expected-note {{Smart pointer 'P' is null after a null value moved from 'PToMove'}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} NoQ wrote: > I suggest: `Null pointer value move-assigned to 'P'`. +1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86293/new/ https://reviews.llvm.org/D86293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr
vrnithinkumar added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:351 +bool SmartPtrModeling::handleEqOp(const CallEvent &Call, + CheckerContext &C) const { vsavchenko wrote: > xazax.hun wrote: > > I'd prefer to call this AssignOp to avoid confusion with `==`. While your > > naming is correct, I always found this nomenclature confusing. > IMO it's not a question of preference with this one, `EqOp` is misleading. Changed to AssignOp Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:384 + + const auto *ArgRegionVal = State->get(ArgRegion); + if (ArgRegionVal) { xazax.hun wrote: > I also find the names of the variables confusing. > > Instead of `ArgRegion` what about `OtherSmartPtrRegion`? > Instead of `ArgRegionVal` what about `OtherInnerPtr`? Changed to `OtherSmartPtrRegion ` and `OtherInnerPtr` Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:391 + +C.addTransition(State, C.getNoteTag([ThisRegion, ArgRegion, IsArgValNull]( +PathSensitiveBugReport &BR, xazax.hun wrote: > Adding return after every `addTransition` is a good practive that we should > follow. Added return Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:399 +ArgRegion->printPretty(OS); +OS << " is null after moved to "; +ThisRegion->printPretty(OS); vsavchenko wrote: > The grammar seems off in this one. I think it should be smith like "after > being moved to" or "after it was moved to". Changed to "after being moved to" Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:419 +llvm::raw_ostream &OS) { + if (&BR.getBugType() != smartptr::getNullDereferenceBugType() || + !BR.isInteresting(ArgRegion)) xazax.hun wrote: > Isn't this the same as the beginning of the note tag above? > > I wonder if there is a way to deduplicate this code. Not a huge priority > though as I do not have an immediate idea for doing this in a clean way. Yes. The check for bug type is duplicated across all the note tags. Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:132 + std::unique_ptr PToMove; // expected-note {{Default constructed smart pointer 'PToMove' is null}} + P = std::move(PToMove); // expected-note {{Smart pointer 'P' is null after a null value moved from 'PToMove'}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} vsavchenko wrote: > NoQ wrote: > > I suggest: `Null pointer value move-assigned to 'P'`. > +1 Updated to `Null pointer value move-assigned to 'P'` Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:139 + std::unique_ptr P; + P = std::move(PToMove); // expected-note {{Smart pointer 'PToMove' is null after moved and assigned to 'P'}} + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} NoQ wrote: > I suggest: `Smart pointer 'PToMove' is null; previous value moved to 'P'`. Or > maybe instead keep the note that the move-checker currently emits? Going with first option "Smart pointer 'PToMove' is null; previous value moved to 'P'" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86293/new/ https://reviews.llvm.org/D86293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr
vrnithinkumar updated this revision to Diff 287612. vrnithinkumar marked 9 inline comments as done. vrnithinkumar edited the summary of this revision. vrnithinkumar added a comment. - Addressing review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86293/new/ https://reviews.llvm.org/D86293 Files: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp clang/test/Analysis/Inputs/system-header-simulator-cxx.h 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 @@ -216,8 +216,7 @@ std::unique_ptr P; std::unique_ptr Q; Q = std::move(P); -// TODO: Fix test with expecting warning after '=' operator overloading modeling. -Q->foo(); // no-warning +Q->foo(); // expected-warning {{Dereference of null smart pointer 'Q' [alpha.cplusplus.SmartPtr]}} } } @@ -276,3 +275,61 @@ Y->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}} } } + +void derefOnMovedFromValidPtr() { + std::unique_ptr PToMove(new A()); + std::unique_ptr P; + P = std::move(PToMove); + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnMovedToNullPtr() { + std::unique_ptr PToMove(new A()); + std::unique_ptr P; + P = std::move(PToMove); // No note. + P->foo(); // No warning. +} + +void derefOnNullPtrGotMovedFromValidPtr() { + std::unique_ptr P(new A()); + std::unique_ptr PToMove; + P = std::move(PToMove); + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnMovedFromUnknownPtr(std::unique_ptr PToMove) { + std::unique_ptr P; + P = std::move(PToMove); + P->foo(); // No warning. +} + +void derefOnMovedUnknownPtr(std::unique_ptr PToMove) { + std::unique_ptr P; + P = std::move(PToMove); + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnAssignedNullPtrToNullSmartPtr() { + std::unique_ptr P; + P = nullptr; + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnAssignedZeroToNullSmartPtr() { + std::unique_ptr P(new A()); + P = 0; + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnAssignedNullToUnknowSmartPtr(std::unique_ptr P) { + P = nullptr; + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +std::unique_ptr &&returnRValRefOfUniquePtr(); + +void drefOnAssignedNullFromMethodPtrValidSmartPtr() { + std::unique_ptr P(new A()); + P = returnRValRefOfUniquePtr(); + P->foo(); // No warning. +} 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 @@ -80,7 +80,7 @@ void derefOnStdSwappedNullPtr() { std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}} std::unique_ptr 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'}} + std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}} // expected-note@-1 {{Calling 'swap'}} // expected-note@-2 {{Returning from 'swap'}} P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} @@ -109,14 +109,6 @@ // expected-note@-1{{Dereference of null smart pointer 'P'}} } -void noNoteTagsForNonMatchingBugType() { - std::unique_ptr P; // No note. - std::unique_ptr P1; // No note. - P1 = std::move(P); // expected-note {{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}} - P->foo(); // expected-warning {{Dereference of null smart pointer 'P' of type 'std::unique_ptr' [cplusplus.Move]}} - // expected-note@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}} -} - void derefOnRawPtrFromGetOnNullPtr() { std::unique_ptr P; // FIXME: add note "Default constructed smart pointer 'P' is null" P.get()->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}} @@ -131,3 +123,50 @@ void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr P) { P.get()->foo(); // No warning. } + +void derefOnMovedFromValidPtr() { + std::unique_ptr PToMove(new A()); // expected-note {{Smart pointer 'PToMove' is constructed}} + // FIXME: above note should go away once we fix marking region not i
[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Looks amazing now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86293/new/ https://reviews.llvm.org/D86293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr
This revision was automatically updated to reflect the committed changes. Closed by commit rG20676cab1168: [analyzer] Add modeling of assignment operator in smart ptr (authored by vrnithinkumar). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86293/new/ https://reviews.llvm.org/D86293 Files: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp clang/test/Analysis/Inputs/system-header-simulator-cxx.h 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 @@ -216,8 +216,7 @@ std::unique_ptr P; std::unique_ptr Q; Q = std::move(P); -// TODO: Fix test with expecting warning after '=' operator overloading modeling. -Q->foo(); // no-warning +Q->foo(); // expected-warning {{Dereference of null smart pointer 'Q' [alpha.cplusplus.SmartPtr]}} } } @@ -276,3 +275,61 @@ Y->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}} } } + +void derefOnMovedFromValidPtr() { + std::unique_ptr PToMove(new A()); + std::unique_ptr P; + P = std::move(PToMove); + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnMovedToNullPtr() { + std::unique_ptr PToMove(new A()); + std::unique_ptr P; + P = std::move(PToMove); // No note. + P->foo(); // No warning. +} + +void derefOnNullPtrGotMovedFromValidPtr() { + std::unique_ptr P(new A()); + std::unique_ptr PToMove; + P = std::move(PToMove); + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnMovedFromUnknownPtr(std::unique_ptr PToMove) { + std::unique_ptr P; + P = std::move(PToMove); + P->foo(); // No warning. +} + +void derefOnMovedUnknownPtr(std::unique_ptr PToMove) { + std::unique_ptr P; + P = std::move(PToMove); + PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnAssignedNullPtrToNullSmartPtr() { + std::unique_ptr P; + P = nullptr; + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnAssignedZeroToNullSmartPtr() { + std::unique_ptr P(new A()); + P = 0; + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void derefOnAssignedNullToUnknowSmartPtr(std::unique_ptr P) { + P = nullptr; + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +std::unique_ptr &&returnRValRefOfUniquePtr(); + +void drefOnAssignedNullFromMethodPtrValidSmartPtr() { + std::unique_ptr P(new A()); + P = returnRValRefOfUniquePtr(); + P->foo(); // No warning. +} 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 @@ -80,7 +80,7 @@ void derefOnStdSwappedNullPtr() { std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}} std::unique_ptr 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'}} + std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}} // expected-note@-1 {{Calling 'swap'}} // expected-note@-2 {{Returning from 'swap'}} P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} @@ -109,14 +109,6 @@ // expected-note@-1{{Dereference of null smart pointer 'P'}} } -void noNoteTagsForNonMatchingBugType() { - std::unique_ptr P; // No note. - std::unique_ptr P1; // No note. - P1 = std::move(P); // expected-note {{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}} - P->foo(); // expected-warning {{Dereference of null smart pointer 'P' of type 'std::unique_ptr' [cplusplus.Move]}} - // expected-note@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}} -} - void derefOnRawPtrFromGetOnNullPtr() { std::unique_ptr P; // FIXME: add note "Default constructed smart pointer 'P' is null" P.get()->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}} @@ -131,3 +123,50 @@ void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr P) { P.get()->foo(); // No warning. } + +void derefOnMovedFromValidPtr() { + std::unique_ptr PToMove(new A()); // expected-note {{Smart pointer 'PToMove' is constructed}} + // FIXME: above note should go away once we fix marking region not interested. + s