diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp --- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp @@ -37,7 +37,7 @@ class SmartPtrModeling : public Checker { - 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 @@ 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 @@ 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::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(&Call)->getCXXThisVal().getAsRegion(); @@ -206,6 +206,9 @@ return true; } + if (handleAssignOp(Call, C)) + return true; + const SmartPtrMethodHandlerFn *Handler = SmartPtrMethodHandlers.lookup(Call); if (!Handler) return false; @@ -374,6 +377,87 @@ C.addTransition(State); } +bool SmartPtrModeling::handleAssignOp(const CallEvent &Call, + CheckerContext &C) const { + ProgramStateRef State = C.getState(); + const auto *OC = dyn_cast(&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(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(OtherSmartPtrRegion); + if (OtherInnerPtr) { + State = State->set(ThisRegion, *OtherInnerPtr); + auto NullVal = C.getSValBuilder().makeNull(); + State = State->set(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(ThisRegion); + State = State->set(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(); Checker->ModelSmartPtrDereference = diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h --- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -970,6 +970,7 @@ T *operator->() const noexcept; operator bool() const noexcept; unique_ptr &operator=(unique_ptr &&p) noexcept; + unique_ptr &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 --- a/clang/test/Analysis/smart-ptr-text-output.cpp +++ b/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. + std::unique_ptr 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@-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. +} + +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 {{Null pointer value move-assigned to 'P'}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1 {{Dereference of null smart pointer 'P'}} +} + +void derefOnMovedUnknownPtr(std::unique_ptr PToMove) { + std::unique_ptr 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@-1 {{Dereference of null smart pointer 'PToMove'}} +} + +void derefOnAssignedNullPtrToNullSmartPtr() { + std::unique_ptr 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@-1 {{Dereference of null smart pointer 'P'}} +} + +void derefOnAssignedZeroToNullSmartPtr() { + 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. + 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@-1 {{Dereference of null smart pointer 'P'}} +} diff --git a/clang/test/Analysis/smart-ptr.cpp b/clang/test/Analysis/smart-ptr.cpp --- a/clang/test/Analysis/smart-ptr.cpp +++ b/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. +}