vrnithinkumar created this revision.
Herald added subscribers: cfe-commits, martong.
Herald added a project: clang.
vrnithinkumar abandoned this revision.
vrnithinkumar added a comment.
It was a mistake
I was supposed to update an existing review.
first time use of arc
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D81734
Files:
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/test/Analysis/Inputs/system-header-simulator-cxx.h
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
@@ -10,7 +10,7 @@
std::unique_ptr<int> Q = std::move(P);
if (Q)
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
- *Q.get() = 1; // no-warning
+ *Q.get() = 1; // no-warning
if (P)
clang_analyzer_warnIfReached(); // no-warning
// TODO: Report a null dereference (instead).
@@ -26,3 +26,64 @@
(s->*func)(); // no-crash
}
} // namespace testUnknownCallee
+
+class A {
+public:
+ A(){};
+ void foo();
+};
+
+A *return_null() {
+ return nullptr;
+}
+
+void derefAfterValidCtr() {
+ std::unique_ptr<A> P(new A());
+ P->foo(); // No warning.
+}
+
+void derefAfterDefaultCtr() {
+ std::unique_ptr<A> P;
+ P->foo(); // expected-warning {{Dereference of null smart pointer [cplusplus.SmartPtr]}}
+}
+
+void derefAfterCtrWithNull() {
+ std::unique_ptr<A> P(nullptr);
+ *P; // expected-warning {{Dereference of null smart pointer [cplusplus.SmartPtr]}}
+}
+
+void derefAfterCtrWithNullReturnMethod() {
+ std::unique_ptr<A> P(return_null());
+ P->foo(); // expected-warning {{Dereference of null smart pointer [cplusplus.SmartPtr]}}
+}
+
+void derefAfterRelease() {
+ std::unique_ptr<A> P(new A());
+ P.release();
+ P->foo(); // expected-warning {{Dereference of null smart pointer [cplusplus.SmartPtr]}}
+}
+
+void derefAfterReset() {
+ std::unique_ptr<A> P(new A());
+ P.reset();
+ P->foo(); // expected-warning {{Dereference of null smart pointer [cplusplus.SmartPtr]}}
+}
+
+void derefAfterResetWithNull() {
+ std::unique_ptr<A> P(new A());
+ P.reset(nullptr);
+ P->foo(); // expected-warning {{Dereference of null smart pointer [cplusplus.SmartPtr]}}
+}
+
+void derefAfterResetWithNonNull() {
+ std::unique_ptr<A> P;
+ P.reset(new A());
+ P->foo(); // No warning.
+}
+
+void derefAfterReleaseAndResetWithNonNull() {
+ std::unique_ptr<A> P(new A());
+ P.release();
+ P.reset(new A());
+ P->foo(); // No warning.
+}
\ No newline at end of file
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===================================================================
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -946,10 +946,15 @@
template <typename T> // TODO: Implement the stub for deleter.
class unique_ptr {
public:
+ unique_ptr() {}
+ unique_ptr(T *) {}
unique_ptr(const unique_ptr &) = delete;
unique_ptr(unique_ptr &&);
T *get() const;
+ T *release() const;
+ void reset(T *p = nullptr) const;
+ void swap(unique_ptr<T> &p) const;
typename std::add_lvalue_reference<T>::type operator*() const;
T *operator->() const;
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -13,26 +13,60 @@
#include "Move.h"
+#include "clang/AST/DeclCXX.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/AST/Type.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
using namespace clang;
using namespace ento;
namespace {
-class SmartPtrModeling : public Checker<eval::Call> {
+class SmartPtrModeling
+ : public Checker<eval::Call, check::PostCall, check::DeadSymbols> {
bool isNullAfterMoveMethod(const CallEvent &Call) const;
+ BugType NullDereferenceBugType{this, "Null-smartPtr-deref",
+ "C++ smart pointer"};
public:
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
+ void checkPostCall(const CallEvent &MC, CheckerContext &C) const;
+ void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
+
+private:
+ void reportBug(CheckerContext &C, const CallEvent &Call) const;
+ bool isStdSmartPointerClass(const CXXRecordDecl *RD) const;
+ void updateTrackedRegion(const CallEvent &Call, CheckerContext &C,
+ const MemRegion *Region) const;
+ void handleReset(const CallEvent &Call, CheckerContext &C) const;
+ void handleSwap(const CallEvent &Call, CheckerContext &C) const;
+ void checkDeferenceOps(const CallEvent &Call, CheckerContext &C) const;
+
+ // Set of STL smart pointer class which we are trying to model.
+ const llvm::StringSet<> StdSmartPtrs = {
+ "shared_ptr",
+ "unique_ptr",
+ "weak_ptr",
+ };
+
+ using SmartPtrMethodHandlerFn =
+ void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
+ CallDescriptionMap<SmartPtrMethodHandlerFn> SmartPtrMethodHandlers{
+ {{"reset"}, &SmartPtrModeling::handleReset},
+ {{"release"}, &SmartPtrModeling::handleReset},
+ {{"swap", 1}, &SmartPtrModeling::handleSwap}};
};
} // end of anonymous namespace
+REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, SVal)
+
bool SmartPtrModeling::isNullAfterMoveMethod(const CallEvent &Call) const {
// TODO: Update CallDescription to support anonymous calls?
// TODO: Handle other methods, such as .get() or .release().
@@ -44,23 +78,146 @@
bool SmartPtrModeling::evalCall(const CallEvent &Call,
CheckerContext &C) const {
- if (!isNullAfterMoveMethod(Call))
+ if (isNullAfterMoveMethod(Call)) {
+ ProgramStateRef State = C.getState();
+ const MemRegion *ThisR =
+ cast<CXXInstanceCall>(&Call)->getCXXThisVal().getAsRegion();
+
+ if (!move::isMovedFrom(State, ThisR)) {
+ // TODO: Model this case as well. At least, avoid invalidation of globals.
+ return false;
+ }
+
+ // TODO: Add a note to bug reports describing this decision.
+ C.addTransition(
+ State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
+ C.getSValBuilder().makeZeroVal(Call.getResultType())));
+ return true;
+ }
+
+ const auto *MethodDecl = dyn_cast<CXXMethodDecl>(Call.getDecl());
+ if (!MethodDecl || !isStdSmartPointerClass(MethodDecl->getParent()))
return false;
+ checkDeferenceOps(Call, C);
+
+ const SmartPtrMethodHandlerFn *Handler = SmartPtrMethodHandlers.lookup(Call);
+ if (!Handler)
+ return false;
+ (this->**Handler)(Call, C);
+
+ return C.isDifferent();
+}
+
+void SmartPtrModeling::checkPostCall(const CallEvent &Call,
+ CheckerContext &C) const {
+ const auto *MethodDecl = dyn_cast<CXXMethodDecl>(Call.getDecl());
+ if (!MethodDecl || !isStdSmartPointerClass(MethodDecl->getParent()))
+ return;
+
+ const auto *CC = dyn_cast<CXXConstructorCall>(&Call);
+ if (!CC)
+ return;
+ const MemRegion *ThisValRegion = CC->getCXXThisVal().getAsRegion();
+ if (!ThisValRegion)
+ return;
+ updateTrackedRegion(Call, C, ThisValRegion);
+}
+
+void SmartPtrModeling::checkDeadSymbols(SymbolReaper &SymReaper,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ // Clean up dead regions from the region map.
+ TrackedRegionMapTy TrackedRegions = State->get<TrackedRegionMap>();
+ for (auto E : TrackedRegions) {
+ const MemRegion *Region = E.first;
+ bool IsRegDead = !SymReaper.isLiveRegion(Region);
+
+ if (IsRegDead) {
+ State = State->remove<TrackedRegionMap>(Region);
+ }
+ }
+ C.addTransition(State);
+}
+
+void SmartPtrModeling::checkDeferenceOps(const CallEvent &Call,
+ CheckerContext &C) const {
ProgramStateRef State = C.getState();
- const MemRegion *ThisR =
- cast<CXXInstanceCall>(&Call)->getCXXThisVal().getAsRegion();
+ const auto *OC = dyn_cast<CXXMemberOperatorCall>(&Call);
+ if (!OC)
+ return;
+ const MemRegion *ThisRegion = OC->getCXXThisVal().getAsRegion();
+ if (!ThisRegion)
+ return;
+
+ const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(OC->getDecl());
+
+ if (MethodDecl && MethodDecl->isOverloadedOperator()) {
+ OverloadedOperatorKind OOK = MethodDecl->getOverloadedOperator();
+ if (OOK == OO_Star || OOK == OO_Arrow) {
+ const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisRegion);
+ if (InnerPointVal && InnerPointVal->isZeroConstant()) {
+ reportBug(C, Call);
+ }
+ }
+ }
+}
+
+void SmartPtrModeling::handleReset(const CallEvent &Call,
+ CheckerContext &C) const {
+ const auto *IC = dyn_cast<CXXInstanceCall>(&Call);
+ if (!IC)
+ return;
- if (!move::isMovedFrom(State, ThisR)) {
- // TODO: Model this case as well. At least, avoid invalidation of globals.
+ const MemRegion *ThisValRegion = IC->getCXXThisVal().getAsRegion();
+ if (!ThisValRegion)
+ return;
+ updateTrackedRegion(Call, C, ThisValRegion);
+}
+
+void SmartPtrModeling::handleSwap(const CallEvent &Call,
+ CheckerContext &C) const {
+ // TODO: Add support to handle swap method.
+}
+
+bool SmartPtrModeling::isStdSmartPointerClass(
+ const CXXRecordDecl *RecordDec) const {
+ if (!RecordDec || !RecordDec->getDeclContext()->isStdNamespace())
return false;
+
+ if (RecordDec->getDeclName().isIdentifier()) {
+ return StdSmartPtrs.count(RecordDec->getName().lower());
+ }
+ return false;
+}
+
+void SmartPtrModeling::reportBug(CheckerContext &C,
+ const CallEvent &Call) const {
+ ExplodedNode *ErrNode = C.generateErrorNode();
+ if (!ErrNode)
+ return;
+
+ auto R = std::make_unique<PathSensitiveBugReport>(
+ NullDereferenceBugType, "Dereference of null smart pointer", ErrNode);
+ R->addRange(Call.getSourceRange());
+ C.emitReport(std::move(R));
+}
+
+void SmartPtrModeling::updateTrackedRegion(
+ const CallEvent &Call, CheckerContext &C,
+ const MemRegion *ThisValRegion) const {
+ auto State = C.getState();
+ auto NumArgs = Call.getNumArgs();
+
+ if (NumArgs == 0) {
+ auto NullSVal = C.getSValBuilder().makeNull();
+ State = State->set<TrackedRegionMap>(ThisValRegion, NullSVal);
+ } else if (NumArgs == 1) {
+ auto ArgVal = Call.getArgSVal(0);
+ State = State->set<TrackedRegionMap>(ThisValRegion, ArgVal);
}
- // TODO: Add a note to bug reports describing this decision.
- C.addTransition(
- State->BindExpr(Call.getOriginExpr(), C.getLocationContext(),
- C.getSValBuilder().makeZeroVal(Call.getResultType())));
- return true;
+ C.addTransition(State);
}
void ento::registerSmartPtrModeling(CheckerManager &Mgr) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
