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 @@ -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; using SmartPtrMethodHandlerFn = void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const; @@ -132,7 +133,6 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call, CheckerContext &C) const { - ProgramStateRef State = C.getState(); if (!smartptr::isStdSmartPtrCall(Call)) return false; @@ -204,6 +204,9 @@ return true; } + if (handleEqOp(Call, C)) + return true; + const SmartPtrMethodHandlerFn *Handler = SmartPtrMethodHandlers.lookup(Call); if (!Handler) return false; @@ -345,6 +348,59 @@ })); } +bool SmartPtrModeling::handleEqOp(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 *ArgRegion = OC->getArgSVal(0).getAsRegion(); + if (!ArgRegion) + return false; + + const auto *ArgRegionVal = State->get(ArgRegion); + if (ArgRegionVal) { + State = State->set(ThisRegion, *ArgRegionVal); + auto NullVal = C.getSValBuilder().makeNull(); + State = State->set(ArgRegion, NullVal); + bool IsArgValNull = ArgRegionVal->isZeroConstant(); + + C.addTransition(State, C.getNoteTag([ThisRegion, ArgRegion, IsArgValNull]( + PathSensitiveBugReport &BR, + llvm::raw_ostream &OS) { + if (&BR.getBugType() != smartptr::getNullDereferenceBugType()) + return; + if (BR.isInteresting(ArgRegion)) { + OS << "Smart pointer "; + ArgRegion->printPretty(OS); + OS << " is null after moved to "; + ThisRegion->printPretty(OS); + } + if (BR.isInteresting(ThisRegion) && IsArgValNull) { + OS << "Smart pointer "; + ThisRegion->printPretty(OS); + OS << " is null after a null value moved from "; + ArgRegion->printPretty(OS); + BR.markInteresting(ArgRegion); + } + })); + } 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(ArgRegion, NullVal); + C.addTransition(State); + } + return true; +} + void ento::registerSmartPtrModeling(CheckerManager &Mgr) { auto *Checker = Mgr.registerChecker(); Checker->ModelSmartPtrDereference = 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 @@ -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'}} } 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 @@ -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]}} +}