[clang] 20676ca - [analyzer] Add modeling of assignment operator in smart ptr
Nithin Vadukkumchery Rajendrakumar via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 26 02:23:15 PDT 2020
Author: Nithin Vadukkumchery Rajendrakumar
Date: 2020-08-26T11:22:55+02:00
New Revision: 20676cab1168c2c60982af85f42725955cbcd7b5
URL: https://github.com/llvm/llvm-project/commit/20676cab1168c2c60982af85f42725955cbcd7b5
DIFF: https://github.com/llvm/llvm-project/commit/20676cab1168c2c60982af85f42725955cbcd7b5.diff
LOG: [analyzer] Add modeling of assignment operator in smart ptr
Summary: Support for 'std::unique_ptr>::operator=' in SmartPtrModeling
Reviewers: NoQ, Szelethus, vsavchenko, xazax.hun
Reviewed By: NoQ, vsavchenko, xazax.hun
Subscribers: martong, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D86293
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index 0b084accbfbe..c405ef12433a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -37,7 +37,7 @@ namespace {
class SmartPtrModeling
: public Checker<eval::Call, check::DeadSymbols, check::RegionChanges> {
- bool isNullAfterMoveMethod(const CallEvent &Call) const;
+ bool isAssignOpMethod(const CallEvent &Call) const;
public:
// Whether the checker should model for null dereferences of smart pointers.
@@ -57,6 +57,7 @@ class SmartPtrModeling
void handleRelease(const CallEvent &Call, CheckerContext &C) const;
void handleSwap(const CallEvent &Call, CheckerContext &C) const;
void handleGet(const CallEvent &Call, CheckerContext &C) const;
+ bool handleAssignOp(const CallEvent &Call, CheckerContext &C) const;
using SmartPtrMethodHandlerFn =
void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
@@ -123,7 +124,7 @@ static ProgramStateRef updateSwappedRegion(ProgramStateRef State,
return State;
}
-bool SmartPtrModeling::isNullAfterMoveMethod(const CallEvent &Call) const {
+bool SmartPtrModeling::isAssignOpMethod(const CallEvent &Call) const {
// TODO: Update CallDescription to support anonymous calls?
// TODO: Handle other methods, such as .get() or .release().
// But once we do, we'd need a visitor to explain null dereferences
@@ -134,12 +135,11 @@ bool SmartPtrModeling::isNullAfterMoveMethod(const CallEvent &Call) const {
bool SmartPtrModeling::evalCall(const CallEvent &Call,
CheckerContext &C) const {
-
ProgramStateRef State = C.getState();
if (!smartptr::isStdSmartPtrCall(Call))
return false;
- if (isNullAfterMoveMethod(Call)) {
+ if (isAssignOpMethod(Call)) {
const MemRegion *ThisR =
cast<CXXInstanceCall>(&Call)->getCXXThisVal().getAsRegion();
@@ -206,6 +206,9 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call,
return true;
}
+ if (handleAssignOp(Call, C))
+ return true;
+
const SmartPtrMethodHandlerFn *Handler = SmartPtrMethodHandlers.lookup(Call);
if (!Handler)
return false;
@@ -374,6 +377,87 @@ void SmartPtrModeling::handleGet(const CallEvent &Call,
C.addTransition(State);
}
+bool SmartPtrModeling::handleAssignOp(const CallEvent &Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ const auto *OC = dyn_cast<CXXMemberOperatorCall>(&Call);
+ if (!OC)
+ return false;
+ OverloadedOperatorKind OOK = OC->getOverloadedOperator();
+ if (OOK != OO_Equal)
+ return false;
+ const MemRegion *ThisRegion = OC->getCXXThisVal().getAsRegion();
+ if (!ThisRegion)
+ return false;
+
+ const MemRegion *OtherSmartPtrRegion = OC->getArgSVal(0).getAsRegion();
+ // In case of 'nullptr' or '0' assigned
+ if (!OtherSmartPtrRegion) {
+ bool AssignedNull = Call.getArgSVal(0).isZeroConstant();
+ if (!AssignedNull)
+ return false;
+ auto NullVal = C.getSValBuilder().makeNull();
+ State = State->set<TrackedRegionMap>(ThisRegion, NullVal);
+ C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) {
+ if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+ !BR.isInteresting(ThisRegion))
+ return;
+ OS << "Smart pointer ";
+ ThisRegion->printPretty(OS);
+ OS << " is assigned to null";
+ }));
+ return true;
+ }
+
+ const auto *OtherInnerPtr = State->get<TrackedRegionMap>(OtherSmartPtrRegion);
+ if (OtherInnerPtr) {
+ State = State->set<TrackedRegionMap>(ThisRegion, *OtherInnerPtr);
+ auto NullVal = C.getSValBuilder().makeNull();
+ State = State->set<TrackedRegionMap>(OtherSmartPtrRegion, NullVal);
+ bool IsArgValNull = OtherInnerPtr->isZeroConstant();
+
+ C.addTransition(
+ State,
+ C.getNoteTag([ThisRegion, OtherSmartPtrRegion, IsArgValNull](
+ PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+ if (&BR.getBugType() != smartptr::getNullDereferenceBugType())
+ return;
+ if (BR.isInteresting(OtherSmartPtrRegion)) {
+ OS << "Smart pointer ";
+ OtherSmartPtrRegion->printPretty(OS);
+ OS << " is null after being moved to ";
+ ThisRegion->printPretty(OS);
+ }
+ if (BR.isInteresting(ThisRegion) && IsArgValNull) {
+ OS << "Null pointer value move-assigned to ";
+ ThisRegion->printPretty(OS);
+ BR.markInteresting(OtherSmartPtrRegion);
+ }
+ }));
+ return true;
+ } else {
+ // In case we dont know anything about value we are moving from
+ // remove the entry from map for which smart pointer got moved to.
+ auto NullVal = C.getSValBuilder().makeNull();
+ State = State->remove<TrackedRegionMap>(ThisRegion);
+ State = State->set<TrackedRegionMap>(OtherSmartPtrRegion, NullVal);
+ C.addTransition(State, C.getNoteTag([OtherSmartPtrRegion,
+ ThisRegion](PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) {
+ if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+ !BR.isInteresting(OtherSmartPtrRegion))
+ return;
+ OS << "Smart pointer ";
+ OtherSmartPtrRegion->printPretty(OS);
+ OS << " is null after; previous value moved to ";
+ ThisRegion->printPretty(OS);
+ }));
+ return true;
+ }
+ return false;
+}
+
void ento::registerSmartPtrModeling(CheckerManager &Mgr) {
auto *Checker = Mgr.registerChecker<SmartPtrModeling>();
Checker->ModelSmartPtrDereference =
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
index a0759479bfeb..f2b148cbc692 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -970,6 +970,7 @@ class unique_ptr {
T *operator->() const noexcept;
operator bool() const noexcept;
unique_ptr<T> &operator=(unique_ptr<T> &&p) noexcept;
+ unique_ptr<T> &operator=(nullptr_t) noexcept;
};
// TODO :: Once the deleter parameter is added update with additional template parameter.
diff --git a/clang/test/Analysis/smart-ptr-text-output.cpp b/clang/test/Analysis/smart-ptr-text-output.cpp
index 9af6f251e01d..d63cd9b805f8 100644
--- a/clang/test/Analysis/smart-ptr-text-output.cpp
+++ b/clang/test/Analysis/smart-ptr-text-output.cpp
@@ -80,7 +80,7 @@ void derefOnSwappedNullPtr() {
void derefOnStdSwappedNullPtr() {
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 at Inputs/system-header-simulator-cxx.h:978 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
+ std::swap(P, PNull); // expected-note at Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
// expected-note at -1 {{Calling 'swap<A>'}}
// expected-note at -2 {{Returning from 'swap<A>'}}
P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
@@ -109,14 +109,6 @@ void noNoteTagsForNonInterestingRegion() {
// expected-note at -1{{Dereference of null smart pointer 'P'}}
}
-void noNoteTagsForNonMatchingBugType() {
- std::unique_ptr<A> P; // No note.
- std::unique_ptr<A> 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 at -1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
-}
-
void derefOnRawPtrFromGetOnNullPtr() {
std::unique_ptr<A> 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 derefOnRawPtrFromGetOnValidPtr() {
void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr<A> P) {
P.get()->foo(); // No warning.
}
+
+void derefOnMovedFromValidPtr() {
+ std::unique_ptr<A> 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<A> P;
+ P = std::move(PToMove); // expected-note {{Smart pointer 'PToMove' is null after being moved to 'P'}}
+ PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+ // expected-note at -1 {{Dereference of null smart pointer 'PToMove'}}
+}
+
+void derefOnMovedToNullPtr() {
+ std::unique_ptr<A> PToMove(new A());
+ std::unique_ptr<A> P;
+ P = std::move(PToMove); // No note.
+ P->foo(); // No warning.
+}
+
+void derefOnNullPtrGotMovedFromValidPtr() {
+ std::unique_ptr<A> 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<A> PToMove; // expected-note {{Default constructed smart pointer 'PToMove' is null}}
+ P = std::move(PToMove); // expected-note {{Null pointer value move-assigned to 'P'}}
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+ // expected-note at -1 {{Dereference of null smart pointer 'P'}}
+}
+
+void derefOnMovedUnknownPtr(std::unique_ptr<A> PToMove) {
+ std::unique_ptr<A> P;
+ P = std::move(PToMove); // expected-note {{Smart pointer 'PToMove' is null after; previous value moved to 'P'}}
+ PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+ // expected-note at -1 {{Dereference of null smart pointer 'PToMove'}}
+}
+
+void derefOnAssignedNullPtrToNullSmartPtr() {
+ std::unique_ptr<A> P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ P = nullptr; // expected-note {{Smart pointer 'P' is assigned to null}}
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+ // expected-note at -1 {{Dereference of null smart pointer 'P'}}
+}
+
+void derefOnAssignedZeroToNullSmartPtr() {
+ std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
+ // FIXME: above note should go away once we fix marking region not interested.
+ P = 0; // expected-note {{Smart pointer 'P' is assigned to null}}
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+ // expected-note at -1 {{Dereference of null smart pointer 'P'}}
+}
diff --git a/clang/test/Analysis/smart-ptr.cpp b/clang/test/Analysis/smart-ptr.cpp
index 17f6718c6605..1403cd6492b2 100644
--- a/clang/test/Analysis/smart-ptr.cpp
+++ b/clang/test/Analysis/smart-ptr.cpp
@@ -216,8 +216,7 @@ void derefAfterAssignment() {
std::unique_ptr<A> P;
std::unique_ptr<A> 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 @@ void derefOnRawPtrFromMultipleGetOnUnknownPtr(std::unique_ptr<A> P) {
Y->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
}
}
+
+void derefOnMovedFromValidPtr() {
+ std::unique_ptr<A> PToMove(new A());
+ std::unique_ptr<A> P;
+ P = std::move(PToMove);
+ PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnMovedToNullPtr() {
+ std::unique_ptr<A> PToMove(new A());
+ std::unique_ptr<A> P;
+ P = std::move(PToMove); // No note.
+ P->foo(); // No warning.
+}
+
+void derefOnNullPtrGotMovedFromValidPtr() {
+ std::unique_ptr<A> P(new A());
+ std::unique_ptr<A> PToMove;
+ P = std::move(PToMove);
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnMovedFromUnknownPtr(std::unique_ptr<A> PToMove) {
+ std::unique_ptr<A> P;
+ P = std::move(PToMove);
+ P->foo(); // No warning.
+}
+
+void derefOnMovedUnknownPtr(std::unique_ptr<A> PToMove) {
+ std::unique_ptr<A> P;
+ P = std::move(PToMove);
+ PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnAssignedNullPtrToNullSmartPtr() {
+ std::unique_ptr<A> P;
+ P = nullptr;
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnAssignedZeroToNullSmartPtr() {
+ std::unique_ptr<A> P(new A());
+ P = 0;
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnAssignedNullToUnknowSmartPtr(std::unique_ptr<A> P) {
+ P = nullptr;
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+std::unique_ptr<A> &&returnRValRefOfUniquePtr();
+
+void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
+ std::unique_ptr<A> P(new A());
+ P = returnRValRefOfUniquePtr();
+ P->foo(); // No warning.
+}
More information about the cfe-commits
mailing list