vrnithinkumar updated this revision to Diff 287512.
vrnithinkumar marked 11 inline comments as done.
vrnithinkumar added a comment.
- Adding checkLiveSymbols and review comments changes
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
@@ -8,12 +8,13 @@
void clang_analyzer_warnIfReached();
void clang_analyzer_numTimesReached();
void clang_analyzer_eval(bool);
+void clang_analyzer_warnOnDeadSymbol(int *);
void derefAfterMove(std::unique_ptr<int> P) {
std::unique_ptr<int> Q = std::move(P);
if (Q)
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
- *Q.get() = 1; // no-warning
+ *Q.get() = 1; // expected-warning {{Dereference of null pointer [core.NullDereference]}}
if (P)
clang_analyzer_warnIfReached(); // no-warning
// TODO: Report a null dereference (instead).
@@ -276,3 +277,69 @@
Y->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
}
}
+
+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.
+}
+
+void innerPointerSymbolLiveness() {
+ std::unique_ptr<int> P(new int());
+ clang_analyzer_warnOnDeadSymbol(P.get());
+ int *RP = P.release();
+} // expected-warning{{SYMBOL DEAD}}
+
+void boolOpCreatedConjuredSymbolLiveness(std::unique_ptr<int> P) {
+ if (P) {
+ int *X = P.get();
+ clang_analyzer_warnOnDeadSymbol(X);
+ }
+} // expected-warning{{SYMBOL DEAD}}
+
+void getCreatedConjuredSymbolLiveness(std::unique_ptr<int> P) {
+ int *X = P.get();
+ clang_analyzer_warnOnDeadSymbol(X);
+ int Y;
+ if (!P) {
+ Y = *P.get(); // expected-warning {{Dereference of null pointer [core.NullDereference]}}
+ // expected-warning@-1 {{SYMBOL DEAD}}
+ }
+}
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
@@ -131,3 +131,80 @@
void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr<A> P) {
P.get()->foo(); // No warning.
}
+
+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"
@@ -35,9 +36,10 @@
namespace {
class SmartPtrModeling
- : public Checker<eval::Call, check::DeadSymbols, check::RegionChanges> {
+ : public Checker<eval::Call, check::DeadSymbols, check::RegionChanges,
+ check::LiveSymbols> {
- bool isNullAfterMoveMethod(const CallEvent &Call) const;
+ bool isBoolConversionMethod(const CallEvent &Call) const;
public:
// Whether the checker should model for null dereferences of smart pointers.
@@ -51,12 +53,16 @@
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;
+ void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) 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 handleGet(const CallEvent &Call, CheckerContext &C) const;
+ void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
using SmartPtrMethodHandlerFn =
void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
@@ -123,7 +129,38 @@
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) {
+ const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl());
+ if (!MethodDecl || !MethodDecl->getParent())
+ return {};
+
+ const auto *RecordDecl = MethodDecl->getParent();
+ if (!RecordDecl || !RecordDecl->isInStdNamespace())
+ return {};
+
+ const auto *TSD = dyn_cast<ClassTemplateSpecializationDecl>(RecordDecl);
+ if (!TSD)
+ return {};
+
+ auto TemplateArgs = TSD->getTemplateArgs().asArray();
+ if (TemplateArgs.size() == 0)
+ return {};
+ auto InnerValueType = TemplateArgs[0].getAsType();
+ return C.getASTContext().getPointerType(InnerValueType.getCanonicalType());
+}
+
+// Helper method to pretty print region and avoid extra spacing.
+static void checkAndPrettyPrintRegion(llvm::raw_ostream &OS,
+ const MemRegion *Region) {
+ if (Region->canPrintPretty()) {
+ OS << " ";
+ Region->printPretty(OS);
+ }
+}
+
+bool SmartPtrModeling::isBoolConversionMethod(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
@@ -139,21 +176,31 @@
if (!smartptr::isStdSmartPtrCall(Call))
return false;
- if (isNullAfterMoveMethod(Call)) {
+ if (isBoolConversionMethod(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.
+ handleBoolConversion(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)
@@ -177,8 +224,8 @@
if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
!BR.isInteresting(ThisRegion))
return;
- OS << "Default constructed smart pointer ";
- ThisRegion->printPretty(OS);
+ OS << "Default constructed smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
OS << " is null";
}));
} else {
@@ -195,8 +242,8 @@
!BR.isInteresting(ThisRegion))
return;
bugreporter::trackExpressionValue(BR.getErrorNode(), TrackingExpr, BR);
- OS << "Smart pointer ";
- ThisRegion->printPretty(OS);
+ OS << "Smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
if (ArgVal.isZeroConstant())
OS << " is constructed using a null value";
else
@@ -229,6 +276,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,
@@ -243,6 +307,18 @@
return State->set<TrackedRegionMap>(RegionMap);
}
+void SmartPtrModeling::checkLiveSymbols(ProgramStateRef State,
+ SymbolReaper &SR) const {
+ // Marking tracked symbols alive
+ TrackedRegionMapTy TrackedRegions = State->get<TrackedRegionMap>();
+ for (auto I = TrackedRegions.begin(), E = TrackedRegions.end(); I != E; ++I) {
+ SVal Val = I->second;
+ for (auto si = Val.symbol_begin(), se = Val.symbol_end(); si != se; ++si) {
+ SR.markLive(*si);
+ }
+ }
+}
+
void SmartPtrModeling::handleReset(const CallEvent &Call,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
@@ -265,8 +341,8 @@
!BR.isInteresting(ThisRegion))
return;
bugreporter::trackExpressionValue(BR.getErrorNode(), TrackingExpr, BR);
- OS << "Smart pointer ";
- ThisRegion->printPretty(OS);
+ OS << "Smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
OS << " reset using a null value";
}));
// TODO: Make sure to ivalidate the region in the Store if we don't have
@@ -300,8 +376,8 @@
!BR.isInteresting(ThisRegion))
return;
- OS << "Smart pointer ";
- ThisRegion->printPretty(OS);
+ OS << "Smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
OS << " is released and set to null";
}));
// TODO: Add support to enable MallocChecker to start tracking the raw
@@ -340,10 +416,10 @@
!BR.isInteresting(ThisRegion))
return;
BR.markInteresting(ArgRegion);
- OS << "Swapped null smart pointer ";
- ArgRegion->printPretty(OS);
- OS << " with smart pointer ";
- ThisRegion->printPretty(OS);
+ OS << "Swapped null smart pointer";
+ checkAndPrettyPrintRegion(OS, ArgRegion);
+ OS << " with smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
}));
}
@@ -374,6 +450,89 @@
C.addTransition(State);
}
+void SmartPtrModeling::handleBoolConversion(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();
+
+ SVal InnerPointerVal;
+ if (const auto *InnerValPtr = State->get<TrackedRegionMap>(ThisRegion)) {
+ InnerPointerVal = *InnerValPtr;
+ } 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();
+ InnerPointerVal = C.getSValBuilder().conjureSymbolVal(
+ CallExpr, LC, InnerPointerType, C.blockCount());
+ }
+
+ if (State->isNull(InnerPointerVal).isConstrainedTrue()) {
+ State = State->BindExpr(CallExpr, C.getLocationContext(),
+ C.getSValBuilder().makeTruthVal(false));
+
+ C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) {
+ BR.markInteresting(ThisRegion);
+ OS << "Smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
+ OS << " is null";
+ }));
+ return;
+ } else if (State->isNonNull(InnerPointerVal).isConstrainedTrue()) {
+ State = State->BindExpr(CallExpr, C.getLocationContext(),
+ C.getSValBuilder().makeTruthVal(true));
+
+ C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) {
+ OS << "Smart pointer";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
+ OS << " is non-null";
+ }));
+ return;
+ } else if (move::isMovedFrom(State, ThisRegion)) {
+ C.addTransition(
+ State->BindExpr(CallExpr, C.getLocationContext(),
+ C.getSValBuilder().makeZeroVal(Call.getResultType())));
+ return;
+ } else {
+ ProgramStateRef NotNullState, NullState;
+ std::tie(NotNullState, NullState) =
+ State->assume(InnerPointerVal.castAs<DefinedOrUnknownSVal>());
+
+ 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";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
+ OS << " is null";
+ }));
+ 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";
+ checkAndPrettyPrintRegion(OS, ThisRegion);
+ OS << " is non-null";
+ }));
+ return;
+ }
+}
+
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
