[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
[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
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
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
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
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
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
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
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
