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 @@ -15,6 +15,7 @@ #define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_SMARTPTR_H #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h" namespace clang { namespace ento { @@ -28,6 +29,31 @@ const BugType *getNullDereferenceBugType(); +// 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); + +class FindWhereConstrained : public BugReporterVisitor { +private: + const MemRegion *ThisRegion; + SValBuilder &Bldr; + llvm::SmallPtrSet PtrsTrackedAndConstrained; + llvm::SmallPtrSet StmtsCovered; + +public: + FindWhereConstrained(const MemRegion *ThisRegion, SValBuilder &Bldr) + : ThisRegion{ThisRegion}, Bldr{Bldr}, PtrsTrackedAndConstrained{}, + StmtsCovered{} {} + PathDiagnosticPieceRef VisitNode(const ExplodedNode *Node, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) override; + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.Add(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 @@ -24,6 +24,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h" #include "llvm/ADT/StringRef.h" +#include using namespace clang; using namespace ento; @@ -86,6 +87,8 @@ explainDereference(OS, DerefRegion, Call); auto R = std::make_unique(NullDereferenceBugType, OS.str(), ErrNode); + R->addVisitor(std::make_unique( + DerefRegion, C.getSValBuilder())); 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 @@ -16,8 +16,11 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclarationName.h" +#include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" +#include "clang/AST/OperationKinds.h" #include "clang/AST/Type.h" +#include "clang/Analysis/PathDiagnostic.h" #include "clang/Basic/LLVM.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" @@ -25,10 +28,14 @@ #include "clang/StaticAnalyzer/Core/CheckerManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/Environment.h" #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h" #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" +#include "llvm/ADT/ImmutableSet.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/raw_ostream.h" #include using namespace clang; @@ -76,10 +83,14 @@ {{"release"}, &SmartPtrModeling::handleRelease}, {{"swap", 1}, &SmartPtrModeling::handleSwap}, {{"get"}, &SmartPtrModeling::handleGet}}; + mutable llvm::ImmutableSet::Factory StmtSetFactory; }; } // end of anonymous namespace REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, SVal) +// REGISTER_SET_WITH_PROGRAMSTATE(ExprsFromGet, const Stmt *) +REGISTER_MAP_WITH_PROGRAMSTATE(ExprsFromGet, const MemRegion *, + llvm::ImmutableSet) // Define the inter-checker API. namespace clang { @@ -106,6 +117,142 @@ return InnerPointVal && !State->assume(InnerPointVal->castAs(), true); } + +bool isExprObtainedFromGet(const ProgramStateRef State, + const MemRegion *ThisRegion, const Expr *E) { + const auto *ExprSet = State->get(ThisRegion); + return ExprSet && ExprSet->contains(E); +} + +PathDiagnosticPieceRef +FindWhereConstrained::VisitNode(const ExplodedNode *Node, + BugReporterContext &BRC, + PathSensitiveBugReport &BR) { + + ProgramStateRef State = Node->getState(); + const SVal *InnerPtr = State->get(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()) { + if (StmtsCovered.contains(S)) { + return nullptr; + } + StmtsCovered.insert(S); + + auto bugReportOnGet = [&](const Expr *E) -> auto { + if (const auto *MCE = llvm::dyn_cast(E)) { + const auto *Method = MCE->getMethodDecl(); + const auto *Record = MCE->getRecordDecl(); + if (Method->getName() == "get" && + Record->getDeclContext()->isStdNamespace() && + Record->getName() == "unique_ptr") { + 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"; + } + PathDiagnosticLocation Pos(S, BRC.getSourceManager(), + Node->getLocationContext()); + return std::make_shared(Pos, OS.str(), + true); + } + } + return std::shared_ptr(); + }; + + // If statement on raw pointer + if (const auto *E = llvm::dyn_cast(S)) { + if (E->getCastKind() == CastKind::CK_PointerToBoolean) { + // Check if we are tracking the expression being casted + const Expr *Sub = E->getSubExpr(); + const Environment &Env = State->getEnvironment(); + EnvironmentEntry Entry(Sub, Node->getLocationContext()); + const SVal ExprVal = Env.getSVal(Entry, Bldr); + if (ExprVal == InnerPtrVal) { + // 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 *InnerCastExpr = + llvm::dyn_cast(Sub)) { + Sub = InnerCastExpr->getSubExpr(); + if (const auto *Ptr = llvm::dyn_cast(Sub)) { + PtrsTrackedAndConstrained.insert(Ptr->getDecl()); + } + } + } + } + } + // Assignment operator + if (const auto *BO = llvm::dyn_cast(S)) { + if (BO->getOpcode() == BO_Assign) { + const Expr *LHS = BO->getLHS(); + + if (const auto *DeclRef = llvm::dyn_cast(LHS)) { + if (PtrsTrackedAndConstrained.contains(DeclRef->getDecl())) { + 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 + if (const auto *ICE = llvm::dyn_cast(RHS)) { + const Expr *Sub = ICE->getSubExpr(); + 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 + auto report = bugReportOnGet(RHS); + if (report) { + return report; + } + } + } + } + } + + // Variable declaration + if (const auto *DS = llvm::dyn_cast(S)) { + const Decl *D = DS->getSingleDecl(); + if (const auto *VD = llvm::dyn_cast(D)) { + for (const auto *I : PtrsTrackedAndConstrained) { + if (I->getName() == VD->getName()) { + const Expr *Init = VD->getAnyInitializer(); + if (Init) { + auto report = bugReportOnGet(Init); + if (report) { + return report; + } + break; + } + } + } + } + } + } + } + } + return nullptr; +} + } // namespace smartptr } // namespace ento } // namespace clang @@ -446,10 +593,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 +604,19 @@ State = State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), InnerPointerVal); - // TODO: Add NoteTag, for how the raw pointer got using 'get' method. - C.addTransition(State); + + // Store the CallExpr as being obtained through get. It will be necessary in + // isExprObtainedFromGet(). + if (const auto *StmtSet = State->get(ThisRegion)) { + auto NewStmtSet = StmtSetFactory.add(*StmtSet, CallExpr); + State = State->set(ThisRegion, NewStmtSet); + } else { + auto EmptySet = StmtSetFactory.getEmptySet(); + auto NewStmtSet = StmtSetFactory.add(EmptySet, 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,61 @@ }; 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 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