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 @@ -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 { + : public Checker { - 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 ExplicitRegions, ArrayRef 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(Call.getDecl()); + if (!MethodDecl || !MethodDecl->getParent()) + return {}; + + const auto *RecordDecl = MethodDecl->getParent(); + if (!RecordDecl || !RecordDecl->isInStdNamespace()) + return {}; + + const auto *TSD = dyn_cast(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(&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(); + + 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 ExplicitRegions, @@ -243,6 +307,18 @@ return State->set(RegionMap); } +void SmartPtrModeling::checkLiveSymbols(ProgramStateRef State, + SymbolReaper &SR) const { + // Marking tracked symbols alive + TrackedRegionMapTy TrackedRegions = State->get(); + 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,96 @@ 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(&Call)->getCXXThisVal().getAsRegion(); + + SVal InnerPointerVal; + if (const auto *InnerValPtr = State->get(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"; + }, + /*IsPrunable=*/true)); + 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"; + }, + /*IsPrunable=*/true)); + 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()); + + auto NullVal = C.getSValBuilder().makeNull(); + // Explicitly tracking the region as null. + NullState = NullState->set(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"; + }, + /*IsPrunable=*/true)); + 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"; + }, + /*IsPrunable=*/true)); + return; + } +} + 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 @@ -131,3 +131,112 @@ void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr P) { P.get()->foo(); // No warning. } + +void derefConditionOnNullPtrFalseBranch() { + std::unique_ptr 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 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 P(new A()); + std::unique_ptr 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 P(new A()); + std::unique_ptr 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 P(new A()); + std::unique_ptr PNull; + if (!P) + PNull->foo(); // No warning. +} + +void derefConditionOnUnKnownPtrAssumeNull(std::unique_ptr P) { + std::unique_ptr 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 P) { + std::unique_ptr 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 P) { + P.reset(new A()); + if (!P) + P->foo(); // No warning. + else + P->foo(); // No warning. +} + +struct S { + std::unique_ptr P; + + void foo() { + if (!P) { // No-note because foo() is pruned + return; + } + } + + int callingFooWithNullPointer() { + foo(); // No note on Calling 'S::foo' + P.reset(new int(0)); // expected-note {{Assigning 0}} + return 1 / *(P.get()); // expected-warning {{Division by zero [core.DivideZero]}} + // expected-note@-1 {{Division by zero}} + } + + int callingFooWithValidPointer() { + P.reset(new int(0)); // expected-note {{Assigning 0}} + foo(); // No note on Calling 'S::foo' + return 1 / *(P.get()); // expected-warning {{Division by zero [core.DivideZero]}} + // expected-note@-1 {{Division by zero}} + } + + int callingFooWithUnknownPointer(std::unique_ptr PUnknown) { + P.swap(PUnknown); + foo(); // No note on Calling 'S::foo' + P.reset(new int(0)); // expected-note {{Assigning 0}} + return 1 / *(P.get()); // expected-warning {{Division by zero [core.DivideZero]}} + // expected-note@-1 {{Division by zero}} + } +}; 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 @@ -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 P) { std::unique_ptr 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 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 P; + if (!P) + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +} + +void derefConditionOnValidPtr() { + std::unique_ptr P(new A()); + std::unique_ptr PNull; + if (P) + PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}} +} + +void derefConditionOnNotValidPtr() { + std::unique_ptr P(new A()); + std::unique_ptr PNull; + if (!P) + PNull->foo(); // No warning. +} + +void derefConditionOnUnKnownPtr(std::unique_ptr 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 P) { + P.reset(new A()); + if (!P) + P->foo(); // No warning. + else + P->foo(); // No warning. +} + +void innerPointerSymbolLiveness() { + std::unique_ptr P(new int()); + clang_analyzer_warnOnDeadSymbol(P.get()); + int *RP = P.release(); +} // expected-warning{{SYMBOL DEAD}} + +void boolOpCreatedConjuredSymbolLiveness(std::unique_ptr P) { + if (P) { + int *X = P.get(); + clang_analyzer_warnOnDeadSymbol(X); + } +} // expected-warning{{SYMBOL DEAD}} + +void getCreatedConjuredSymbolLiveness(std::unique_ptr 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}} + } +}