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<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]}}
}
}
@@ -252,3 +251,36 @@
P->foo(); // No warning.
PValid->foo(); // No warning.
}
+
+void drefOnMovedFromValidPtr() {
+ 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 drefOnMovedToNullPtr() {
+ 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 drefOnMovedFromUnknownPtr(std::unique_ptr<A> PToMove) {
+ std::unique_ptr<A> P;
+ P = std::move(PToMove);
+ P->foo(); // No warning.
+}
+
+void drefOnMovedUnknownPtr(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]}}
+}
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<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@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+void drefOnMovedFromValidPtr() {
+ 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 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<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 {{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<A> PToMove) {
+ std::unique_ptr<A> 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;
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<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 *ArgRegion = OC->getArgSVal(0).getAsRegion();
+ if (!ArgRegion)
+ return false;
+
+ const auto *ArgRegionVal = State->get<TrackedRegionMap>(ArgRegion);
+ if (ArgRegionVal) {
+ State = State->set<TrackedRegionMap>(ThisRegion, *ArgRegionVal);
+ auto NullVal = C.getSValBuilder().makeNull();
+ State = State->set<TrackedRegionMap>(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<TrackedRegionMap>(ThisRegion);
+ State = State->set<TrackedRegionMap>(ArgRegion, NullVal);
+ C.addTransition(State);
+ }
+ return true;
+}
+
void ento::registerSmartPtrModeling(CheckerManager &Mgr) {
auto *Checker = Mgr.registerChecker<SmartPtrModeling>();
Checker->ModelSmartPtrDereference =
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
