diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp index 391d038c8766a2..f1ca28ba339d51 100644 --- 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 @@ using namespace ento; namespace { class SmartPtrModeling - : public Checker { + : public Checker { - bool isAssignOpMethod(const CallEvent &Call) const; + bool isBoolConversionMethod(const CallEvent &Call) const; public: // Whether the checker should model for null dereferences of smart pointers. @@ -51,6 +53,9 @@ class SmartPtrModeling 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; @@ -62,6 +67,7 @@ class SmartPtrModeling const MemRegion *ThisRegion) const; bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion, const MemRegion *OtherSmartPtrRegion) const; + void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const; using SmartPtrMethodHandlerFn = void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const; @@ -128,7 +134,38 @@ static ProgramStateRef updateSwappedRegion(ProgramStateRef State, return State; } -bool SmartPtrModeling::isAssignOpMethod(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 @@ -143,21 +180,31 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call, if (!smartptr::isStdSmartPtrCall(Call)) return false; - if (isAssignOpMethod(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) @@ -184,8 +231,8 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call, 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 { @@ -202,8 +249,8 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call, !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 @@ -239,6 +286,23 @@ void SmartPtrModeling::checkDeadSymbols(SymbolReaper &SymReaper, 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, @@ -253,6 +317,18 @@ ProgramStateRef SmartPtrModeling::checkRegionChanges( 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(); @@ -275,8 +351,8 @@ void SmartPtrModeling::handleReset(const CallEvent &Call, !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 @@ -310,8 +386,8 @@ void SmartPtrModeling::handleRelease(const CallEvent &Call, !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 @@ -350,10 +426,10 @@ void SmartPtrModeling::handleSwap(const CallEvent &Call, !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); })); } @@ -410,8 +486,8 @@ bool SmartPtrModeling::handleAssignOp(const CallEvent &Call, if (&BR.getBugType() != smartptr::getNullDereferenceBugType() || !BR.isInteresting(ThisRegion)) return; - OS << "Smart pointer "; - ThisRegion->printPretty(OS); + OS << "Smart pointer"; + checkAndPrettyPrintRegion(OS, ThisRegion); OS << " is assigned to null"; })); return true; @@ -447,14 +523,14 @@ bool SmartPtrModeling::updateMovedSmartPointers( if (&BR.getBugType() != smartptr::getNullDereferenceBugType()) return; if (BR.isInteresting(OtherSmartPtrRegion)) { - OS << "Smart pointer "; - OtherSmartPtrRegion->printPretty(OS); - OS << " is null after being moved to "; - ThisRegion->printPretty(OS); + OS << "Smart pointer"; + checkAndPrettyPrintRegion(OS, OtherSmartPtrRegion); + OS << " is null after being moved to"; + checkAndPrettyPrintRegion(OS, ThisRegion); } if (BR.isInteresting(ThisRegion) && IsArgValNull) { - OS << "A null pointer value is moved to "; - ThisRegion->printPretty(OS); + OS << "A null pointer value is moved to"; + checkAndPrettyPrintRegion(OS, ThisRegion); BR.markInteresting(OtherSmartPtrRegion); } })); @@ -471,16 +547,92 @@ bool SmartPtrModeling::updateMovedSmartPointers( if (&BR.getBugType() != smartptr::getNullDereferenceBugType() || !BR.isInteresting(OtherSmartPtrRegion)) return; - OS << "Smart pointer "; - OtherSmartPtrRegion->printPretty(OS); - OS << " is null after; previous value moved to "; - ThisRegion->printPretty(OS); + OS << "Smart pointer"; + checkAndPrettyPrintRegion(OS, OtherSmartPtrRegion); + OS << " is null after; previous value moved to"; + checkAndPrettyPrintRegion(OS, ThisRegion); })); return true; } return false; } +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()); + State = State->set(ThisRegion, InnerPointerVal); + } + + if (State->isNull(InnerPointerVal).isConstrainedTrue()) { + State = State->BindExpr(CallExpr, C.getLocationContext(), + C.getSValBuilder().makeTruthVal(false)); + + C.addTransition(State); + return; + } else if (State->isNonNull(InnerPointerVal).isConstrainedTrue()) { + State = State->BindExpr(CallExpr, C.getLocationContext(), + C.getSValBuilder().makeTruthVal(true)); + + C.addTransition(State); + 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 index 602a5e94c23a79..16c1bddc55e198 100644 --- a/clang/test/Analysis/smart-ptr-text-output.cpp +++ b/clang/test/Analysis/smart-ptr-text-output.cpp @@ -199,3 +199,108 @@ void derefUnknownPtrMovedToConstruct(std::unique_ptr PToMove) { PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}} // expected-note@-1{{Dereference of null smart pointer 'PToMove'}} } + +void derefConditionOnNullPtrFalseBranch() { + std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}} + if (P) { // expected-note {{Taking false branch}} + 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}} + 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}} + 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}} + 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 index b169f9c5c2f235..8e8156011eb5fc 100644 --- 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). @@ -375,3 +376,77 @@ void derefMoveConstructedWithRValueRefReturn() { std::unique_ptr P(functionReturnsRValueRef()); P->foo(); // No warning. } + +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}} + } +} + +int derefConditionOnUnKnownPtr(int *q) { + std::unique_ptr P(q); + if (P) + return *P; // No warning. + else + return *P; // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} +}