vrnithinkumar updated this revision to Diff 286632.
vrnithinkumar marked 5 inline comments as done.
vrnithinkumar added a comment.
Changes to use conjureSymbolVal if the inner pointer value is missing
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86027/new/
https://reviews.llvm.org/D86027
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
@@ -252,3 +252,46 @@
P->foo(); // No warning.
PValid->foo(); // No warning.
}
+
+void derefConditionOnNullPtr() {
+ std::unique_ptr<A> P;
+ if (P)
+ P->foo(); // No warning.
+ else
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotNullPtr() {
+ std::unique_ptr<A> P;
+ if (!P)
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnValidPtr() {
+ std::unique_ptr<A> P(new A());
+ std::unique_ptr<A> PNull;
+ if (P)
+ PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotValidPtr() {
+ std::unique_ptr<A> P(new A());
+ std::unique_ptr<A> PNull;
+ if (!P)
+ PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtr(std::unique_ptr<A> P) {
+ if (P)
+ P->foo(); // No warning.
+ else
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnValidPtrAfterReset(std::unique_ptr<A> P) {
+ P.reset(new A());
+ if (!P)
+ P->foo(); // No warning.
+ else
+ P->foo(); // No warning.
+}
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
@@ -116,3 +116,80 @@
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 derefConditionOnNullPtrFalseBranch() {
+ std::unique_ptr<A> P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (P) { // expected-note {{Taking false branch}}
+ // expected-note@-1{{Smart pointer 'P' is nul}}
+ P->foo(); // No warning.
+ } else {
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+ // expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnNullPtrTrueBranch() {
+ std::unique_ptr<A> P; // expected-note {{Default constructed smart pointer 'P' is null}}
+ if (!P) { // expected-note {{Taking true branch}}
+ // expected-note@-1{{Smart pointer 'P' is nul}}
+ P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+ // expected-note@-1{{Dereference of null smart pointer 'P'}}
+ }
+}
+
+void derefConditionOnValidPtrTrueBranch() {
+ std::unique_ptr<A> P(new A());
+ std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (P) { // expected-note {{Taking true branch}}
+ // expected-note@-1{{Smart pointer 'P' is non-nul}}
+ PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+ // expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ } else {
+ PNull->foo(); // No warning
+ }
+}
+
+void derefConditionOnValidPtrFalseBranch() {
+ std::unique_ptr<A> P(new A());
+ std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (!P) { // expected-note {{Taking false branch}}
+ // expected-note@-1{{Smart pointer 'P' is non-nul}}
+ PNull->foo(); // No warning
+ } else {
+ PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+ // expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ }
+}
+
+void derefConditionOnNotValidPtr() {
+ std::unique_ptr<A> P(new A());
+ std::unique_ptr<A> PNull;
+ if (!P)
+ PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtrAssumeNull(std::unique_ptr<A> P) {
+ std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (!P) { // expected-note {{Taking true branch}}
+ // expected-note@-1{{Assuming smart pointer 'P' is null}}
+ PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+ // expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ }
+}
+
+void derefConditionOnUnKnownPtrAssumeNonNull(std::unique_ptr<A> P) {
+ std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+ if (P) { // expected-note {{Taking true branch}}
+ // expected-note@-1{{Assuming smart pointer 'P' is non-null}}
+ PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+ // expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+ }
+}
+
+void derefOnValidPtrAfterReset(std::unique_ptr<A> P) {
+ P.reset(new A());
+ if (!P)
+ P->foo(); // No warning.
+ else
+ P->foo(); // No warning.
+}
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -15,6 +15,7 @@
#include "SmartPtr.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclarationName.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/Type.h"
#include "clang/Basic/LLVM.h"
@@ -37,7 +38,7 @@
class SmartPtrModeling
: public Checker<eval::Call, check::DeadSymbols, check::RegionChanges> {
- bool isNullAfterMoveMethod(const CallEvent &Call) const;
+ bool isBooleanOpMethod(const CallEvent &Call) const;
public:
// Whether the checker should model for null dereferences of smart pointers.
@@ -51,11 +52,14 @@
ArrayRef<const MemRegion *> ExplicitRegions,
ArrayRef<const MemRegion *> Regions,
const LocationContext *LCtx, const CallEvent *Call) const;
+ void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
+ const char *Sep) const;
private:
void handleReset(const CallEvent &Call, CheckerContext &C) const;
void handleRelease(const CallEvent &Call, CheckerContext &C) const;
void handleSwap(const CallEvent &Call, CheckerContext &C) const;
+ void handleBoolOperation(const CallEvent &Call, CheckerContext &C) const;
using SmartPtrMethodHandlerFn =
void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
@@ -121,7 +125,32 @@
return State;
}
-bool SmartPtrModeling::isNullAfterMoveMethod(const CallEvent &Call) const {
+// Helper method to get the inner pointer type of specialized smart pointer
+// Returns empty type if not found valid inner pointer type.
+static QualType getInnerPointerType(const CallEvent &Call, CheckerContext &C) {
+ QualType InnerType{};
+ const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl());
+ if (!MethodDecl || !MethodDecl->getParent())
+ return InnerType;
+
+ const auto *RecordDecl = MethodDecl->getParent();
+ if (!RecordDecl || !RecordDecl->getDeclContext()->isStdNamespace())
+ return InnerType;
+
+ const auto *TSD = dyn_cast<ClassTemplateSpecializationDecl>(RecordDecl);
+ if (TSD) {
+ auto TemplateArgs = TSD->getTemplateArgs().asArray();
+ assert(
+ TemplateArgs.size() > 0 &&
+ "Smart pointer should have specialized with atleast one template type");
+ auto InnerValueType = TemplateArgs[0].getAsType();
+ InnerType =
+ C.getASTContext().getPointerType(InnerValueType.getCanonicalType());
+ }
+ return InnerType;
+}
+
+bool SmartPtrModeling::isBooleanOpMethod(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
@@ -137,21 +166,31 @@
if (!smartptr::isStdSmartPtrCall(Call))
return false;
- if (isNullAfterMoveMethod(Call)) {
+ if (isBooleanOpMethod(Call)) {
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;
+ if (ModelSmartPtrDereference) {
+ // The check for the region is moved is duplicated in handleBoolOperation
+ // method.
+ // FIXME: Once we model std::move for smart pointers clean up this and use
+ // that modeling.
+ handleBoolOperation(Call, C);
+ return true;
+ } else {
+ 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;
}
-
- // 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;
}
if (!ModelSmartPtrDereference)
@@ -227,6 +266,23 @@
C.addTransition(State);
}
+void SmartPtrModeling::printState(raw_ostream &Out, ProgramStateRef State,
+ const char *NL, const char *Sep) const {
+ TrackedRegionMapTy RS = State->get<TrackedRegionMap>();
+
+ if (!RS.isEmpty()) {
+ Out << Sep << "Smart ptr regions :" << NL;
+ for (auto I : RS) {
+ I.first->dumpToStream(Out);
+ if (smartptr::isNullSmartPtr(State, I.first))
+ Out << ": Null";
+ else
+ Out << ": Non Null";
+ Out << NL;
+ }
+ }
+}
+
ProgramStateRef SmartPtrModeling::checkRegionChanges(
ProgramStateRef State, const InvalidatedSymbols *Invalidated,
ArrayRef<const MemRegion *> ExplicitRegions,
@@ -345,6 +401,80 @@
}));
}
+void SmartPtrModeling::handleBoolOperation(const CallEvent &Call,
+ CheckerContext &C) const {
+ // To model unique_ptr::operator bool
+ ProgramStateRef State = C.getState();
+ const Expr *CallExpr = Call.getOriginExpr();
+ const MemRegion *ThisRegion =
+ cast<CXXInstanceCall>(&Call)->getCXXThisVal().getAsRegion();
+ const auto *InnerPointVal = State->get<TrackedRegionMap>(ThisRegion);
+ if (InnerPointVal) {
+ bool IsInnerPtrNull = InnerPointVal->isZeroConstant();
+ State = State->BindExpr(CallExpr, C.getLocationContext(),
+ C.getSValBuilder().makeTruthVal(!IsInnerPtrNull));
+
+ C.addTransition(State, C.getNoteTag([ThisRegion, IsInnerPtrNull](
+ PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) {
+ if (IsInnerPtrNull)
+ BR.markInteresting(ThisRegion);
+ OS << "Smart pointer ";
+ ThisRegion->printPretty(OS);
+ OS << (IsInnerPtrNull ? " is null" : " is non-null");
+ }));
+ return;
+ } else if (move::isMovedFrom(State, ThisRegion)) {
+ C.addTransition(
+ State->BindExpr(CallExpr, C.getLocationContext(),
+ C.getSValBuilder().makeZeroVal(Call.getResultType())));
+ return;
+ } else {
+ // In case of inner pointer SVal is not available we create
+ // conjureSymbolVal for inner pointer value.
+ auto InnerPointerType = getInnerPointerType(Call, C);
+ if (InnerPointerType.isNull())
+ return;
+
+ const LocationContext *LC = C.getLocationContext();
+ SVal InnerPointerVal = C.getSValBuilder().conjureSymbolVal(
+ CallExpr, LC, InnerPointerType, C.blockCount());
+
+ State = State->set<TrackedRegionMap>(ThisRegion, InnerPointerVal);
+ ProgramStateRef NotNullState, NullState;
+ std::tie(NotNullState, NullState) =
+ State->assume(InnerPointerVal.castAs<DefinedOrUnknownSVal>());
+
+ if (NullState) {
+ auto NullVal = C.getSValBuilder().makeNull();
+ // Explicitly tracking the region as null.
+ NullState = NullState->set<TrackedRegionMap>(ThisRegion, NullVal);
+
+ NullState = NullState->BindExpr(CallExpr, C.getLocationContext(),
+ C.getSValBuilder().makeTruthVal(false));
+ C.addTransition(NullState,
+ C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) {
+ OS << "Assuming smart pointer ";
+ ThisRegion->printPretty(OS);
+ OS << " is null";
+ }));
+ }
+ if (NotNullState) {
+ NotNullState =
+ NotNullState->BindExpr(CallExpr, C.getLocationContext(),
+ C.getSValBuilder().makeTruthVal(true));
+ C.addTransition(NotNullState,
+ C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) {
+ OS << "Assuming smart pointer ";
+ ThisRegion->printPretty(OS);
+ OS << " is non-null";
+ }));
+ }
+ }
+}
+
void ento::registerSmartPtrModeling(CheckerManager &Mgr) {
auto *Checker = Mgr.registerChecker<SmartPtrModeling>();
Checker->ModelSmartPtrDereference =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
