rnkovacs updated this revision to Diff 155770.
rnkovacs marked an inline comment as done.
rnkovacs edited the summary of this revision.
rnkovacs added a comment.
Added standard quote, marking the section about non-member functions that may
also invalidate the buffer as a TODO.
Also changed the note message to that suggested by @NoQ (thanks!). All tests
pass now.
https://reviews.llvm.org/D49360
Files:
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/dangling-internal-buffer.cpp
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 <typename CharT>
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<char> 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}}
+ d = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
+ } // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ // expected-note@-1 {{Method call is allowed to invalidate the internal buffer}}
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}}
+ } // expected-note {{Method call is allowed to invalidate the internal buffer}}
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 {{Pointer to dangling buffer was obtained here}}
+ d = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
+ } // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ // expected-note@-1 {{Method call is allowed to invalidate the internal buffer}}
+ 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}}
+ } // expected-note {{Method call is allowed to invalidate the internal buffer}}
std::u16string s16;
const char16_t *c16_2 = s16.c_str();
consume(c16); // expected-warning {{Use of memory after it is freed}}
@@ -101,7 +120,7 @@
{
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}}
+ } // expected-note {{Method call is allowed to invalidate the internal buffer}}
std::u32string s32;
const char32_t *c32_2 = s32.data();
consume(c32); // expected-warning {{Use of memory after it is freed}}
@@ -113,11 +132,11 @@
{
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}}
+ d1 = s1.data(); // expected-note {{Pointer to dangling buffer was 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 {{Method call is allowed to invalidate the internal buffer}}
+ // expected-note@-1 {{Method call is allowed to invalidate the internal buffer}}
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 {{Pointer to dangling buffer was obtained here}}
+ s = "world"; // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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_plus_equals() {
+ const char *c;
+ std::string s = "hello";
+ c = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
+ s += " world"; // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ consume(c); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_clear() {
const char *c;
std::string s;
- {
- c = s.c_str();
- }
- consume(c); // no-warning
+ c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
+ s.clear(); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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 {{Pointer to dangling buffer was obtained here}}
+ s.append(2, 'x'); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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 {{Pointer to dangling buffer was obtained here}}
+ s.assign(4, 'a'); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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 {{Pointer to dangling buffer was obtained here}}
+ s.erase(0, 2); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ consume(c); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_insert() {
+ const char *c;
+ std::string s = "ello";
+ c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained here}}
+ s.insert(0, 1, 'h'); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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 {{Pointer to dangling buffer was obtained here}}
+ s.replace(6, 5, "string"); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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 {{Pointer to dangling buffer was obtained here}}
+ s.pop_back(); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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 {{Pointer to dangling buffer was obtained here}}
+ s.push_back('c'); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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 {{Pointer to dangling buffer was obtained here}}
+ s.reserve(5); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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 {{Pointer to dangling buffer was obtained here}}
+ s.resize(5); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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_shrink_to_fit() {
const char *c;
std::string s;
+ c = s.data(); // expected-note {{Pointer to dangling buffer was obtained here}}
+ s.shrink_to_fit(); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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 {{Pointer to dangling buffer was obtained here}}
+ s1.swap(s2); // expected-note {{Method call is allowed to invalidate the internal buffer}}
+ 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
}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2917,7 +2917,7 @@
Msg = "Memory is released";
break;
case AF_InternalBuffer:
- Msg = "Internal buffer is released because the object was destroyed";
+ Msg = "Method call is allowed to invalidate the internal buffer";
break;
case AF_None:
llvm_unreachable("Unhandled allocation family!");
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<PtrSet>
- : public ProgramStatePartialTrait<PtrSet> {
+template <>
+struct ProgramStateTrait<PtrSet> : public ProgramStatePartialTrait<PtrSet> {
static void *GDMIndex() {
static int Index = 0;
return &Index;
@@ -46,7 +46,10 @@
class DanglingInternalBufferChecker
: public Checker<check::DeadSymbols, check::PostCall> {
- 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<CXXMemberOperatorCall>(&Call)) {
+ OverloadedOperatorKind Opc = MemOpCall->getOriginExpr()->getOperator();
+ if (Opc == OO_Equal || Opc == OO_PlusEqual)
+ return true;
+ return false;
+ }
+ return (isa<CXXDestructorCall>(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<CXXInstanceCall>(&Call);
@@ -127,7 +171,7 @@
return;
}
- if (isa<CXXDestructorCall>(ICall)) {
+ if (mayInvalidateBuffer(Call)) {
if (const PtrSet *PS = State->get<RawPtrMap>(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<RawPtrMap>(Entry.first)
- : State->set<RawPtrMap>(Entry.first, CleanedUpSet);
+ ? State->remove<RawPtrMap>(Entry.first)
+ : State->set<RawPtrMap>(Entry.first, CleanedUpSet);
}
}
C.addTransition(State);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
