diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h b/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h --- a/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h +++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtr.h @@ -22,12 +22,23 @@ /// Returns true if the event call is on smart pointer. bool isStdSmartPtrCall(const CallEvent &Call); +bool isStdSmartPtr(const CXXRecordDecl *Rec); /// Returns whether the smart pointer is null or not. bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion); const BugType *getNullDereferenceBugType(); +// TODO: Get rid after D97183 +// Returns whether E is of the form a.get(), where the MemRegion corresponding +// to the smart-ptr a is ThisRegion. +bool isExprObtainedFromGet(const ProgramStateRef State, + const MemRegion *ThisRegion, const Expr *E); + +/// Returns the SVal of a MemRegion from the TrackedRegionMap +const SVal *getInnerPointer(const ProgramStateRef State, + const MemRegion *ThisRegion); + } // namespace smartptr } // namespace ento } // namespace clang diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp @@ -53,6 +53,186 @@ const BugType *getNullDereferenceBugType() { return NullDereferenceBugTypePtr; } +class GetNoteEmitter : public BugReporterVisitor { +private: + const MemRegion *ThisRegion; + llvm::SmallPtrSet PtrsTrackedAndConstrained; + llvm::SmallPtrSet StmtsCovered; + +public: + GetNoteEmitter(const MemRegion *ThisRegion) + : ThisRegion{ThisRegion}, PtrsTrackedAndConstrained{}, StmtsCovered{} {} + PathDiagnosticPieceRef VisitNode(const ExplodedNode *Node, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; + + PathDiagnosticPieceRef emitNote(const ExplodedNode *Node, + BugReporterContext &BRC, const Expr *E); + + bool visitIfStmt(const ExplodedNode *Node, const ProgramStateRef State, + BugReporterContext &BRC, const Stmt *S, + const SVal InnerPtrVal); + + PathDiagnosticPieceRef emitNoteForAssignment(const ExplodedNode *Node, + const ProgramStateRef State, + BugReporterContext &BRC, + const Stmt *S, + const SVal InnerPtrVal); + + PathDiagnosticPieceRef emitNoteForInitialization(const ExplodedNode *Node, + const ProgramStateRef State, + BugReporterContext &BRC, + const Stmt *S, + const SVal InnerPtrVal); + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.Add(ThisRegion); + } +}; + +PathDiagnosticPieceRef GetNoteEmitter::VisitNode(const ExplodedNode *Node, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) { + + ProgramStateRef State = Node->getState(); + const SVal *InnerPtr = getInnerPointer(State, ThisRegion); + if (InnerPtr) { + SVal InnerPtrVal = *InnerPtr; + ProgramStateRef NullState, NonNullState; + std::tie(NullState, NonNullState) = + State->assume(InnerPtrVal.castAs()); + + // Check whether we have a constraint on ThisRegion + if (NullState && NonNullState) { + if (const Stmt *S = Node->getStmtForDiagnostics()) { + // Skip if we already have covered this statement + auto ItInsertedPair = StmtsCovered.insert(S); + if (!ItInsertedPair.second) + return nullptr; + + // If statement on raw pointer + visitIfStmt(Node, State, BRC, S, InnerPtrVal); + + // Assignment operator + if (auto Report = + emitNoteForAssignment(Node, State, BRC, S, InnerPtrVal)) + return Report; + + // Variable declaration + if (auto Report = + emitNoteForInitialization(Node, State, BRC, S, InnerPtrVal)) + return Report; + } + } + } + return nullptr; +} + +PathDiagnosticPieceRef GetNoteEmitter::emitNote(const ExplodedNode *Node, + BugReporterContext &BRC, + const Expr *E) { + if (const auto *MCE = llvm::dyn_cast(E)) { + const auto *Method = MCE->getMethodDecl(); + const auto *Record = MCE->getRecordDecl(); + if (Method->getName() == "get" && isStdSmartPtr(Record)) { + llvm::SmallString<128> Str; + llvm::raw_svector_ostream OS(Str); + if (ThisRegion->canPrintPretty()) { + OS << "Obtained null inner pointer from "; + ThisRegion->printPretty(OS); + } else { + OS << "Obtained null inner pointer here"; + } + const Stmt *S = Node->getStmtForDiagnostics(); + PathDiagnosticLocation Pos(S, BRC.getSourceManager(), + Node->getLocationContext()); + return std::make_shared(Pos, OS.str(), true); + } + } + return {}; +} + +bool GetNoteEmitter::visitIfStmt(const ExplodedNode *Node, + const ProgramStateRef State, + BugReporterContext &BRC, const Stmt *S, + const SVal InnerPtrVal) { + const auto *E = llvm::dyn_cast(S); + if (!E || E->getCastKind() != CastKind::CK_PointerToBoolean) + return false; + // Check if we are tracking the expression being casted + const Expr *Sub = E->getSubExpr(); + SVal ExprVal = State->getSVal(Sub, Node->getLocationContext()); + if (ExprVal != InnerPtrVal) + return false; + // So we have a pointer being used in an if statement + // And that pointer is being tracked to the same SVal as + // ThisRegion Also it is at this if statement that the + // constraining starts So we know that this pointer points to + // P.get() + if (const auto *Ptr = llvm::dyn_cast(Sub->IgnoreParenCasts())) { + PtrsTrackedAndConstrained.insert(Ptr->getDecl()); + return true; + } + return false; +} + +PathDiagnosticPieceRef GetNoteEmitter::emitNoteForAssignment( + const ExplodedNode *Node, const ProgramStateRef State, + BugReporterContext &BRC, const Stmt *S, const SVal InnerPtrVal) { + const auto *BO = llvm::dyn_cast(S); + if (!BO || BO->getOpcode() != BO_Assign) + return nullptr; + const Expr *LHS = BO->getLHS(); + const auto *DeclRef = llvm::dyn_cast(LHS); + if (!DeclRef || !PtrsTrackedAndConstrained.contains(DeclRef->getDecl())) + return nullptr; + const Expr *RHS = BO->getRHS(); + // So now we have an assignment of the form a = b, + // where a is known to be tracked and constrained + + // If b is just a pointer, then we should add it to the set + const Expr *Sub = RHS->IgnoreParenCasts(); + if (const auto *Ptr = llvm::dyn_cast(Sub)) { + PtrsTrackedAndConstrained.insert(Ptr->getDecl()); + llvm::SmallString<128> Str; + llvm::raw_svector_ostream OS(Str); + OS << "Obtained null value here"; + PathDiagnosticLocation Pos(S, BRC.getSourceManager(), + Node->getLocationContext()); + return std::make_shared(Pos, OS.str(), true); + } + + // If b is a get() expression, then we can return a note + if (auto Report = emitNote(Node, BRC, RHS)) + return Report; + + return nullptr; +} + +PathDiagnosticPieceRef GetNoteEmitter::emitNoteForInitialization( + const ExplodedNode *Node, const ProgramStateRef State, + BugReporterContext &BRC, const Stmt *S, const SVal InnerPtrVal) { + const auto *DS = llvm::dyn_cast(S); + if (!DS) + return nullptr; + const Decl *D = DS->getSingleDecl(); + assert(D && "DeclStmt should have at least one Decl"); + const auto *VD = llvm::dyn_cast(D); + if (!VD) + return nullptr; + for (const auto *I : PtrsTrackedAndConstrained) { + if (I->getName() == VD->getName()) { + const Expr *Init = VD->getAnyInitializer(); + if (!Init) + continue; + if (auto Report = emitNote(Node, BRC, Init)) + return Report; + break; + } + } + return nullptr; +} + } // namespace smartptr } // namespace ento } // namespace clang @@ -86,6 +266,7 @@ explainDereference(OS, DerefRegion, Call); auto R = std::make_unique(NullDereferenceBugType, OS.str(), ErrNode); + R->addVisitor(std::make_unique(DerefRegion)); R->markInteresting(DerefRegion); C.emitReport(std::move(R)); } 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 @@ -76,11 +76,18 @@ {{"release"}, &SmartPtrModeling::handleRelease}, {{"swap", 1}, &SmartPtrModeling::handleSwap}, {{"get"}, &SmartPtrModeling::handleGet}}; + + // TODO: Get rid after D97183 + mutable llvm::ImmutableSet::Factory StmtSetFactory; }; } // end of anonymous namespace REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, SVal) +// TODO: Get rid of this onece D97183 is settled +REGISTER_MAP_WITH_PROGRAMSTATE(ExprsFromGet, const MemRegion *, + llvm::ImmutableSet) + // Define the inter-checker API. namespace clang { namespace ento { @@ -101,11 +108,34 @@ return false; } +bool isStdSmartPtr(const CXXRecordDecl *Rec) { + if (!Rec || !Rec->getDeclContext()->isStdNamespace()) + return false; + if (Rec->getDeclName().isIdentifier()) { + StringRef Name = Rec->getName(); + return Name == "shared_ptr" || Name == "unique_ptr" || Name == "weak_ptr"; + } + return false; +} + bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) { const auto *InnerPointVal = State->get(ThisRegion); return InnerPointVal && !State->assume(InnerPointVal->castAs(), true); } + +// TODO: Get rid after D97183 +bool isExprObtainedFromGet(const ProgramStateRef State, + const MemRegion *ThisRegion, const Expr *E) { + const auto *ExprSet = State->get(ThisRegion); + return ExprSet && ExprSet->contains(E); +} + +const SVal *getInnerPointer(const ProgramStateRef State, + const MemRegion *ThisRegion) { + return State->get(ThisRegion); +} + } // namespace smartptr } // namespace ento } // namespace clang @@ -446,10 +476,10 @@ return; SVal InnerPointerVal; + const auto *CallExpr = Call.getOriginExpr(); if (const auto *InnerValPtr = State->get(ThisRegion)) { InnerPointerVal = *InnerValPtr; } else { - const auto *CallExpr = Call.getOriginExpr(); InnerPointerVal = C.getSValBuilder().conjureSymbolVal( CallExpr, C.getLocationContext(), Call.getResultType(), C.blockCount()); State = State->set(ThisRegion, InnerPointerVal); @@ -457,8 +487,16 @@ State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), InnerPointerVal); - // TODO: Add NoteTag, for how the raw pointer got using 'get' method. - C.addTransition(State); + + // TODO: Get rid of this onece D97183 is settled + // Store the CallExpr as being obtained through get. It will be necessary in + // isExprObtainedFromGet(). + const auto *ExistingSet = State->get(ThisRegion); + auto BaseSet = ExistingSet ? *ExistingSet : StmtSetFactory.getEmptySet(); + auto NewStmtSet = StmtSetFactory.add(BaseSet, CallExpr); + State = State->set(ThisRegion, NewStmtSet); + + C.addTransition(State); // The note is added later by a visitor. } bool SmartPtrModeling::handleAssignOp(const CallEvent &Call, 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 @@ -306,10 +306,81 @@ }; void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) { - A *RP = P.get(); + A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}} if (!RP) { // expected-note {{Assuming 'RP' is null}} // expected-note@-1 {{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 multpleDeclsWithGet(std::unique_ptr P) { + A *dummy1 = nullptr, *RP = P.get(), *dummy2; // expected-note {{Obtained null inner pointer from 'P'}} + if (!RP) { // expected-note {{Assuming 'RP' is null}} + // expected-note@-1 {{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 multipleGetsShouldNotAllHaveNotes(std::unique_ptr P) { + A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}} + A *dummy1 = P.get(); + A *dummy2 = P.get(); + if (!RP) { // expected-note {{Assuming 'RP' is null}} + // expected-note@-1 {{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 getShouldNotAlwaysLeaveANote() { + std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}} + A *a = P.get(); + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1{{Dereference of null smart pointer 'P'}} +} + +void getShouldNotLeaveANoteAfterReset(std::unique_ptr P) { + A *a = P.get(); + P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1{{Dereference of null smart pointer 'P'}} +} + +void getShouldNotLeaveNoteWhenPtrNotUsed(std::unique_ptr P) { + A *a = P.get(); + if (!P) { // expected-note {{Taking true branch}} + // expected-note@-1 {{Assuming smart pointer 'P' is null}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1{{Dereference of null smart pointer 'P'}} + } +} + +void getShouldLeaveANoteWithWhileLoop(std::unique_ptr P) { + A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}} + while (!RP) { // expected-note {{Assuming 'RP' is null}} + // expected-note@-1 {{Loop condition is true. Entering loop body}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1{{Dereference of null smart pointer 'P'}} + } +} + +void getShouldLeaveANoteWithForLoop(std::unique_ptr P) { + for (A *RP = P.get(); !RP;) { // expected-note {{Assuming 'RP' is null}} + // expected-note@-1 {{Loop condition is true. Entering loop body}} + // expected-note@-2 {{Obtained null inner pointer from 'P'}} + P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}} + // expected-note@-1{{Dereference of null smart pointer 'P'}} + } +} + +void getShouldLeaveNoteOnChaining(std::unique_ptr P) { + A *praw = P.get(), *other; // expected-note {{Obtained null inner pointer from 'P'}} + other = praw; // expected-note {{Obtained null value here}} + if (!other) { // expected-note {{Assuming 'other' is null}} + // expected-note@-1 {{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'}} + } +} \ No newline at end of file