Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp +++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp @@ -32,8 +32,8 @@ // This is a trick to gain access to PtrSet's Factory. namespace clang { namespace ento { -template<> struct ProgramStateTrait - : public ProgramStatePartialTrait { +template <> +struct ProgramStateTrait : public ProgramStatePartialTrait { static void *GDMIndex() { static int Index = 0; return &Index; @@ -46,7 +46,10 @@ class DanglingInternalBufferChecker : public Checker { - CallDescription CStrFn, DataFn; + + CallDescription AppendFn, AssignFn, ClearFn, CStrFn, DataFn, EraseFn, + InsertFn, PopBackFn, PushBackFn, ReplaceFn, ReserveFn, ResizeFn, + ShrinkToFitFn, SwapFn; public: class DanglingBufferBRVisitor : public BugReporterVisitor { @@ -81,7 +84,17 @@ } }; - DanglingInternalBufferChecker() : CStrFn("c_str"), DataFn("data") {} + DanglingInternalBufferChecker() + : AppendFn("append"), AssignFn("assign"), ClearFn("clear"), + CStrFn("c_str"), DataFn("data"), EraseFn("erase"), InsertFn("insert"), + PopBackFn("pop_back"), PushBackFn("push_back"), ReplaceFn("replace"), + ReserveFn("reserve"), ResizeFn("resize"), + ShrinkToFitFn("shrink_to_fit"), SwapFn("swap") {} + + /// Check whether the function called on the container object is a + /// member function that potentially invalidates pointers referring + /// to the objects's internal buffer. + bool mayInvalidateBuffer(const CallEvent &Call) const; /// Record the connection between the symbol returned by c_str() and the /// corresponding string object region in the ProgramState. Mark the symbol @@ -94,6 +107,37 @@ } // end anonymous namespace +// [string.require] +// +// "References, pointers, and iterators referring to the elements of a +// basic_string sequence may be invalidated by the following uses of that +// basic_string object: +// +// -- TODO: As an argument to any standard library function taking a reference +// to non-const basic_string as an argument. For example, as an argument to +// non-member functions swap(), operator>>(), and getline(), or as an argument +// to basic_string::swap(). +// +// -- Calling non-const member functions, except operator[], at, front, back, +// begin, rbegin, end, and rend." +// +bool DanglingInternalBufferChecker::mayInvalidateBuffer( + const CallEvent &Call) const { + if (const auto *MemOpCall = dyn_cast(&Call)) { + OverloadedOperatorKind Opc = MemOpCall->getOriginExpr()->getOperator(); + if (Opc == OO_Equal || Opc == OO_PlusEqual) + return true; + return false; + } + return (isa(Call) || Call.isCalled(AppendFn) || + Call.isCalled(AssignFn) || Call.isCalled(ClearFn) || + Call.isCalled(EraseFn) || Call.isCalled(InsertFn) || + Call.isCalled(PopBackFn) || Call.isCalled(PushBackFn) || + Call.isCalled(ReplaceFn) || Call.isCalled(ReserveFn) || + Call.isCalled(ResizeFn) || Call.isCalled(ShrinkToFitFn) || + Call.isCalled(SwapFn)); +} + void DanglingInternalBufferChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { const auto *ICall = dyn_cast(&Call); @@ -127,7 +171,7 @@ return; } - if (isa(ICall)) { + if (mayInvalidateBuffer(Call)) { if (const PtrSet *PS = State->get(ObjRegion)) { // Mark all pointer symbols associated with the deleted object released. const Expr *Origin = Call.getOriginExpr(); @@ -161,8 +205,8 @@ CleanedUpSet = F.remove(CleanedUpSet, Symbol); } State = CleanedUpSet.isEmpty() - ? State->remove(Entry.first) - : State->set(Entry.first, CleanedUpSet); + ? State->remove(Entry.first) + : State->set(Entry.first, CleanedUpSet); } } C.addTransition(State); @@ -183,7 +227,7 @@ SmallString<256> Buf; llvm::raw_svector_ostream OS(Buf); - OS << "Pointer to dangling buffer was obtained here"; + OS << "Dangling inner pointer obtained here"; PathDiagnosticLocation Pos(S, BRC.getSourceManager(), N->getLocationContext()); return std::make_shared(Pos, OS.str(), true, Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -2908,7 +2908,7 @@ "Returned allocated memory"); } else if (isReleased(RS, RSPrev, S)) { const auto Family = RS->getAllocationFamily(); - switch(Family) { + switch (Family) { case AF_Alloca: case AF_Malloc: case AF_CXXNew: @@ -2916,9 +2916,25 @@ case AF_IfNameIndex: Msg = "Memory is released"; break; - case AF_InternalBuffer: - Msg = "Internal buffer is released because the object was destroyed"; + case AF_InternalBuffer: { + SmallString<256> Buf; + llvm::raw_svector_ostream OS(Buf); + OS << "Inner pointer invalidated by call to "; + if (N->getLocation().getKind() == ProgramPoint::PostImplicitCallKind) { + OS << "destructor"; + } else { + OS << "'"; + const Stmt *S = RS->getStmt(); + if (const auto *MemCallE = dyn_cast(S)) { + OS << MemCallE->getMethodDecl()->getNameAsString(); + } else if (const auto *OpCallE = dyn_cast(S)) { + OS << OpCallE->getDirectCallee()->getNameAsString(); + } + OS << "'"; + } + Msg = OS.str().data(); break; + } case AF_None: llvm_unreachable("Unhandled allocation family!"); } Index: test/Analysis/dangling-internal-buffer.cpp =================================================================== --- test/Analysis/dangling-internal-buffer.cpp +++ test/Analysis/dangling-internal-buffer.cpp @@ -2,13 +2,35 @@ namespace std { -template< typename CharT > +typedef int size_type; + +template class basic_string { public: + basic_string(); + basic_string(const CharT *s); + ~basic_string(); + void clear(); + + basic_string &operator=(const basic_string &str); + basic_string &operator+=(const basic_string &str); + const CharT *c_str() const; const CharT *data() const; CharT *data(); + + basic_string &append(size_type count, CharT ch); + basic_string &assign(size_type count, CharT ch); + basic_string &erase(size_type index, size_type count); + basic_string &insert(size_type index, size_type count, CharT ch); + basic_string &replace(size_type pos, size_type count, const basic_string &str); + void pop_back(); + void push_back(CharT ch); + void reserve(size_type new_cap); + void resize(size_type count); + void shrink_to_fit(); + void swap(basic_string &other); }; typedef basic_string string; @@ -23,73 +45,70 @@ void consume(const char16_t *) {} void consume(const char32_t *) {} -void deref_after_scope_char_cstr() { - const char *c; +void deref_after_scope_char(bool cond) { + const char *c, *d; { 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}} + c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}} + d = s.data(); // expected-note {{Dangling inner pointer obtained here}} + } // expected-note {{Inner pointer invalidated by call to destructor}} + // expected-note@-1 {{Inner pointer invalidated by call to destructor}} 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}} -} - -void deref_after_scope_char_data() { - const char *c; - { - std::string s; - c = s.data(); // 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.data(); - consume(c); // expected-warning {{Use of memory after it is freed}} - // expected-note@-1 {{Use of memory after it is freed}} + if (cond) { + // expected-note@-1 {{Assuming 'cond' is not equal to 0}} + // expected-note@-2 {{Taking true branch}} + // expected-note@-3 {{Assuming 'cond' is 0}} + // expected-note@-4 {{Taking false branch}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} + } else { + consume(d); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} + } } void deref_after_scope_char_data_non_const() { char *c; { std::string s; - c = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}} - } // expected-note {{Internal buffer is released because the object was destroyed}} + c = s.data(); // expected-note {{Dangling inner pointer obtained here}} + } // expected-note {{Inner pointer invalidated by call to destructor}} std::string s; char *c2 = s.data(); 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_wchar_t_cstr() { - const wchar_t *w; +void deref_after_scope_wchar_t(bool cond) { + const wchar_t *c, *d; { - std::wstring ws; - w = ws.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}} - } // expected-note {{Internal buffer is released because the object was destroyed}} - std::wstring ws; - const wchar_t *w2 = ws.c_str(); - consume(w); // expected-warning {{Use of memory after it is freed}} - // expected-note@-1 {{Use of memory after it is freed}} -} - -void deref_after_scope_wchar_t_data() { - const wchar_t *w; - { - std::wstring ws; - w = ws.data(); // expected-note {{Pointer to dangling buffer was obtained here}} - } // expected-note {{Internal buffer is released because the object was destroyed}} - std::wstring ws; - const wchar_t *w2 = ws.data(); - consume(w); // expected-warning {{Use of memory after it is freed}} - // expected-note@-1 {{Use of memory after it is freed}} + std::wstring s; + c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}} + d = s.data(); // expected-note {{Dangling inner pointer obtained here}} + } // expected-note {{Inner pointer invalidated by call to destructor}} + // expected-note@-1 {{Inner pointer invalidated by call to destructor}} + std::wstring s; + const wchar_t *c2 = s.c_str(); + if (cond) { + // expected-note@-1 {{Assuming 'cond' is not equal to 0}} + // expected-note@-2 {{Taking true branch}} + // expected-note@-3 {{Assuming 'cond' is 0}} + // expected-note@-4 {{Taking false branch}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} + } else { + consume(d); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} + } } void deref_after_scope_char16_t_cstr() { const char16_t *c16; { std::u16string s16; - c16 = s16.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}} - } // expected-note {{Internal buffer is released because the object was destroyed}} + c16 = s16.c_str(); // expected-note {{Dangling inner pointer obtained here}} + } // expected-note {{Inner pointer invalidated by call to destructor}} std::u16string s16; const char16_t *c16_2 = s16.c_str(); consume(c16); // expected-warning {{Use of memory after it is freed}} @@ -100,8 +119,8 @@ const char32_t *c32; { std::u32string s32; - c32 = s32.data(); // expected-note {{Pointer to dangling buffer was obtained here}} - } // expected-note {{Internal buffer is released because the object was destroyed}} + c32 = s32.data(); // expected-note {{Dangling inner pointer obtained here}} + } // expected-note {{Inner pointer invalidated by call to destructor}} std::u32string s32; const char32_t *c32_2 = s32.data(); consume(c32); // expected-warning {{Use of memory after it is freed}} @@ -112,12 +131,12 @@ const char *c1, *d1; { std::string s1; - c1 = s1.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}} - d1 = s1.data(); // expected-note {{Pointer to dangling buffer was obtained here}} + c1 = s1.c_str(); // expected-note {{Dangling inner pointer obtained here}} + d1 = s1.data(); // expected-note {{Dangling inner pointer obtained here}} const char *local = s1.c_str(); consume(local); // no-warning - } // expected-note {{Internal buffer is released because the object was destroyed}} - // expected-note@-1 {{Internal buffer is released because the object was destroyed}} + } // expected-note {{Inner pointer invalidated by call to destructor}} + // expected-note@-1 {{Inner pointer invalidated by call to destructor}} std::string s2; const char *c2 = s2.c_str(); if (cond) { @@ -129,23 +148,144 @@ // expected-note@-1 {{Use of memory after it is freed}} } else { consume(d1); // expected-warning {{Use of memory after it is freed}} - } // expected-note@-1 {{Use of memory after it is freed}} + } // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_equals() { + const char *c; + std::string s = "hello"; + c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}} + s = "world"; // expected-note {{Inner pointer invalidated by call to 'operator='}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_plus_equals() { + const char *c; + std::string s = "hello"; + c = s.data(); // expected-note {{Dangling inner pointer obtained here}} + s += " world"; // expected-note {{Inner pointer invalidated by call to 'operator+='}} + 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_cstr_ok() { +void deref_after_clear() { const char *c; std::string s; - { - c = s.c_str(); - } - consume(c); // no-warning + c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}} + s.clear(); // expected-note {{Inner pointer invalidated by call to 'clear'}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_append() { + const char *c; + std::string s = "hello"; + c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}} + s.append(2, 'x'); // expected-note {{Inner pointer invalidated by call to 'append'}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_assign() { + const char *c; + std::string s; + c = s.data(); // expected-note {{Dangling inner pointer obtained here}} + s.assign(4, 'a'); // expected-note {{Inner pointer invalidated by call to 'assign'}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_erase() { + const char *c; + std::string s = "hello"; + c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}} + s.erase(0, 2); // expected-note {{Inner pointer invalidated by call to 'erase'}} + 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_data_ok() { +void deref_after_insert() { + const char *c; + std::string s = "ello"; + c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}} + s.insert(0, 1, 'h'); // expected-note {{Inner pointer invalidated by call to 'insert'}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_replace() { + const char *c; + std::string s = "hello world"; + c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}} + s.replace(6, 5, "string"); // expected-note {{Inner pointer invalidated by call to 'replace'}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_pop_back() { const char *c; std::string s; + c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}} + s.pop_back(); // expected-note {{Inner pointer invalidated by call to 'pop_back'}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_push_back() { + const char *c; + std::string s; + c = s.data(); // expected-note {{Dangling inner pointer obtained here}} + s.push_back('c'); // expected-note {{Inner pointer invalidated by call to 'push_back'}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_reserve() { + const char *c; + std::string s; + c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}} + s.reserve(5); // expected-note {{Inner pointer invalidated by call to 'reserve'}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_resize() { + const char *c; + std::string s; + c = s.data(); // expected-note {{Dangling inner pointer obtained here}} + s.resize(5); // expected-note {{Inner pointer invalidated by call to 'resize'}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_shrink_to_fit() { + const char *c; + std::string s; + c = s.data(); // expected-note {{Dangling inner pointer obtained here}} + s.shrink_to_fit(); // expected-note {{Inner pointer invalidated by call to 'shrink_to_fit'}} + consume(c); // expected-warning {{Use of memory after it is freed}} + // expected-note@-1 {{Use of memory after it is freed}} +} + +void deref_after_swap() { + const char *c; + std::string s1, s2; + c = s1.data(); // expected-note {{Dangling inner pointer obtained here}} + s1.swap(s2); // expected-note {{Inner pointer invalidated by call to 'swap'}} + 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_ok(bool cond) { + const char *c, *d; + std::string s; { - c = s.data(); + c = s.c_str(); + d = s.data(); } - consume(c); // no-warning + if (cond) + consume(c); // no-warning + else + consume(d); // no-warning }