Index: lib/StaticAnalyzer/Checkers/AllocationState.h =================================================================== --- lib/StaticAnalyzer/Checkers/AllocationState.h +++ lib/StaticAnalyzer/Checkers/AllocationState.h @@ -10,6 +10,7 @@ #ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ALLOCATIONSTATE_H #define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ALLOCATIONSTATE_H +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" namespace clang { @@ -20,6 +21,11 @@ ProgramStateRef markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin); +/// This function provides an additional visitor that augments the bug report +/// with information relevant to memory errors caused by the misuse of +/// AF_InternalBuffer symbols. +std::unique_ptr getDanglingBufferBRVisitor(SymbolRef Sym); + } // end namespace allocation_state } // end namespace ento Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp +++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp @@ -7,30 +7,67 @@ // //===----------------------------------------------------------------------===// // -// This file defines a check that marks a raw pointer to a C++ standard library -// container's inner buffer released when the object is destroyed. This -// information can be used by MallocChecker to detect use-after-free problems. +// This file defines a check that marks a raw pointer to a C++ container's +// inner buffer released when the object is destroyed. This information can +// be used by MallocChecker to detect use-after-free problems. // //===----------------------------------------------------------------------===// +#include "AllocationState.h" #include "ClangSACheckers.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h" #include "clang/StaticAnalyzer/Core/Checker.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" -#include "AllocationState.h" using namespace clang; using namespace ento; +// FIXME: c_str() may be called on a string object many times, so it should +// have a list of symbols associated with it. +REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, SymbolRef) + namespace { -class DanglingInternalBufferChecker : public Checker { +class DanglingInternalBufferChecker + : public Checker { CallDescription CStrFn; public: + class DanglingBufferBRVisitor + : public BugReporterVisitorImpl { + SymbolRef PtrToBuf; + + public: + DanglingBufferBRVisitor(SymbolRef Sym) : PtrToBuf(Sym) {} + + static void *getTag() { + static int Tag = 0; + return &Tag; + } + + void Profile(llvm::FoldingSetNodeID &ID) const override { + ID.AddPointer(getTag()); + } + + std::shared_ptr VisitNode(const ExplodedNode *N, + const ExplodedNode *PrevN, + BugReporterContext &BRC, + BugReport &BR) override; + + // FIXME: Scan the map once in the visitor's constructor and do a direct + // lookup by region. + bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) { + RawPtrMapTy Map = State->get(); + for (const auto Entry : Map) { + if (Entry.second == Sym) + return true; + } + return false; + } + }; + DanglingInternalBufferChecker() : CStrFn("c_str") {} /// Record the connection between the symbol returned by c_str() and the @@ -44,10 +81,6 @@ } // end anonymous namespace -// FIXME: c_str() may be called on a string object many times, so it should -// have a list of symbols associated with it. -REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, SymbolRef) - void DanglingInternalBufferChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { const auto *ICall = dyn_cast(&Call); @@ -103,6 +136,41 @@ C.addTransition(State); } +std::shared_ptr +DanglingInternalBufferChecker::DanglingBufferBRVisitor::VisitNode( + const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC, + BugReport &BR) { + + if (!isSymbolTracked(N->getState(), PtrToBuf) || + isSymbolTracked(PrevN->getState(), PtrToBuf)) + return nullptr; + + const Stmt *S = PathDiagnosticLocation::getStmt(N); + if (!S) + return nullptr; + + SmallString<256> Buf; + llvm::raw_svector_ostream OS(Buf); + OS << "Pointer to dangling buffer was obtained here"; + PathDiagnosticLocation Pos(S, BRC.getSourceManager(), + N->getLocationContext()); + return std::make_shared(Pos, OS.str(), true, + nullptr); +} + +namespace clang { +namespace ento { +namespace allocation_state { + +std::unique_ptr getDanglingBufferBRVisitor(SymbolRef Sym) { + return llvm::make_unique< + DanglingInternalBufferChecker::DanglingBufferBRVisitor>(Sym); +} + +} // end namespace allocation_state +} // end namespace ento +} // end namespace clang + void ento::registerDanglingInternalBufferChecker(CheckerManager &Mgr) { registerNewDeleteChecker(Mgr); Mgr.registerChecker(); Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1994,6 +1994,11 @@ R->markInteresting(Sym); R->addRange(Range); R->addVisitor(llvm::make_unique(Sym)); + + const RefState *RS = C.getState()->get(Sym); + if (RS->getAllocationFamily() == AF_InternalBuffer) + R->addVisitor(allocation_state::getDanglingBufferBRVisitor(Sym)); + C.emitReport(std::move(R)); } } Index: test/Analysis/dangling-internal-buffer.cpp =================================================================== --- test/Analysis/dangling-internal-buffer.cpp +++ test/Analysis/dangling-internal-buffer.cpp @@ -6,7 +6,7 @@ class basic_string { public: ~basic_string(); - const CharT *c_str(); + const CharT *c_str() const; }; typedef basic_string string; @@ -25,8 +25,20 @@ const char *c; { std::string s; - c = s.c_str(); + c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}} + } // expected-note {{Internal buffer is released because the object was destroyed}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_scope_char2() { + const char *c; + { + std::string s; + c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}} } // expected-note {{Internal buffer is released because the object was destroyed}} + std::string s; + const char *c2 = s.c_str(); consume(c); // expected-warning {{Use of memory after it is freed}} // expected-note@-1 {{Use of memory after it is freed}} } @@ -35,7 +47,7 @@ const wchar_t *w; { std::wstring ws; - w = ws.c_str(); + w = ws.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}} } // expected-note {{Internal buffer is released because the object was destroyed}} consume(w); // expected-warning {{Use of memory after it is freed}} // expected-note@-1 {{Use of memory after it is freed}} @@ -45,7 +57,7 @@ const char16_t *c16; { std::u16string s16; - c16 = s16.c_str(); + c16 = s16.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}} } // expected-note {{Internal buffer is released because the object was destroyed}} consume(c16); // expected-warning {{Use of memory after it is freed}} // expected-note@-1 {{Use of memory after it is freed}} @@ -55,7 +67,7 @@ const char32_t *c32; { std::u32string s32; - c32 = s32.c_str(); + c32 = s32.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}} } // expected-note {{Internal buffer is released because the object was destroyed}} consume(c32); // expected-warning {{Use of memory after it is freed}} // expected-note@-1 {{Use of memory after it is freed}}