[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334348: [analyzer] Add dangling internal buffer check.
(authored by rkovacs, committed by ).
Herald added subscribers: llvm-commits, mikhail.ramalho.
Changed prior to commit:
https://reviews.llvm.org/D47135?vs=148827=150623#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47135
Files:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/test/Analysis/dangling-internal-buffer.cpp
Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -300,6 +300,11 @@
let ParentPackage = CplusplusAlpha in {
+def DanglingInternalBufferChecker : Checker<"DanglingInternalBuffer">,
+ HelpText<"Check for internal raw pointers of C++ standard library containers "
+ "used after deallocation">,
+ DescFile<"DanglingInternalBufferChecker.cpp">;
+
def DeleteWithNonVirtualDtorChecker : Checker<"DeleteWithNonVirtualDtor">,
HelpText<"Reports destructions of polymorphic objects with a non-virtual "
"destructor in their base class">,
Index: cfe/trunk/test/Analysis/dangling-internal-buffer.cpp
===
--- cfe/trunk/test/Analysis/dangling-internal-buffer.cpp
+++ cfe/trunk/test/Analysis/dangling-internal-buffer.cpp
@@ -0,0 +1,71 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.DanglingInternalBuffer %s -analyzer-output=text -verify
+
+namespace std {
+
+template< typename CharT >
+class basic_string {
+public:
+ ~basic_string();
+ const CharT *c_str();
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end namespace std
+
+void consume(const char *) {}
+void consume(const wchar_t *) {}
+void consume(const char16_t *) {}
+void consume(const char32_t *) {}
+
+void deref_after_scope_char() {
+ const char *c;
+ {
+std::string s;
+c = 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_wchar_t() {
+ const wchar_t *w;
+ {
+std::wstring ws;
+w = 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_char16_t() {
+ const char16_t *c16;
+ {
+std::u16string s16;
+c16 = s16.c_str();
+ }
+ consume(c16); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char32_t() {
+ const char32_t *c32;
+ {
+std::u32string s32;
+c32 = s32.c_str();
+ }
+ consume(c32); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_ok() {
+ const char *c;
+ std::string s;
+ {
+c = s.c_str();
+ }
+ consume(c); // no-warning
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -0,0 +1,88 @@
+//=== DanglingInternalBufferChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+#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;
+
+namespace {
+
+class DanglingInternalBufferChecker : public Checker {
+ CallDescription CStrFn;
+
+public:
+
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
xbolva00 added a comment. Ping https://reviews.llvm.org/D47135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
xbolva00 accepted this revision. xbolva00 added a comment. Looks fine, awesome feature! https://reviews.llvm.org/D47135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:33
+public:
+ DanglingInternalBufferChecker() : CStrFn("c_str") {}
+
xbolva00 wrote:
> string.data() support?
Yup, there's a lot of API support improvements planned for later reviews.
https://reviews.llvm.org/D47135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
rnkovacs updated this revision to Diff 148827.
rnkovacs added a comment.
Added a check for `UnknownVal` and two FIXMEs (one for the `OriginExpr` and one
for the new `CheckKind`).
https://reviews.llvm.org/D47135
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/AllocationState.h
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/dangling-internal-buffer.cpp
Index: test/Analysis/dangling-internal-buffer.cpp
===
--- /dev/null
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -0,0 +1,71 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.DanglingInternalBuffer %s -analyzer-output=text -verify
+
+namespace std {
+
+template< typename CharT >
+class basic_string {
+public:
+ ~basic_string();
+ const CharT *c_str();
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end namespace std
+
+void consume(const char *) {}
+void consume(const wchar_t *) {}
+void consume(const char16_t *) {}
+void consume(const char32_t *) {}
+
+void deref_after_scope_char() {
+ const char *c;
+ {
+std::string s;
+c = 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_wchar_t() {
+ const wchar_t *w;
+ {
+std::wstring ws;
+w = 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_char16_t() {
+ const char16_t *c16;
+ {
+std::u16string s16;
+c16 = s16.c_str();
+ }
+ consume(c16); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char32_t() {
+ const char32_t *c32;
+ {
+std::u32string s32;
+c32 = s32.c_str();
+ }
+ consume(c32); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_ok() {
+ const char *c;
+ std::string s;
+ {
+c = s.c_str();
+ }
+ consume(c); // no-warning
+}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -30,6 +30,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
+#include "AllocationState.h"
#include
#include
@@ -45,7 +46,8 @@
AF_CXXNew,
AF_CXXNewArray,
AF_IfNameIndex,
- AF_Alloca
+ AF_Alloca,
+ AF_InternalBuffer
};
class RefState {
@@ -1467,6 +1469,7 @@
case AF_CXXNew: os << "'new'"; return;
case AF_CXXNewArray: os << "'new[]'"; return;
case AF_IfNameIndex: os << "'if_nameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific allocator"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("not a deallocation expression");
}
@@ -1479,6 +1482,7 @@
case AF_CXXNew: os << "'delete'"; return;
case AF_CXXNewArray: os << "'delete[]'"; return;
case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific deallocator"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("suspicious argument");
}
@@ -1653,7 +1657,9 @@
return Optional();
}
case AF_CXXNew:
- case AF_CXXNewArray: {
+ case AF_CXXNewArray:
+ // FIXME: Add new CheckKind for AF_InternalBuffer.
+ case AF_InternalBuffer: {
if (IsALeakCheck) {
if (ChecksEnabled[CK_NewDeleteLeaksChecker])
return CK_NewDeleteLeaksChecker;
@@ -2991,6 +2997,20 @@
}
}
+namespace clang {
+namespace ento {
+namespace allocation_state {
+
+ProgramStateRef
+markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) {
+ AllocationFamily Family = AF_InternalBuffer;
+ return State->set(Sym, RefState::getReleased(Family, Origin));
+}
+
+} // end namespace allocation_state
+} // end namespace ento
+} // end namespace clang
+
void ento::registerNewDeleteLeaksChecker(CheckerManager ) {
registerCStringCheckerBasic(mgr);
MallocChecker *checker = mgr.registerChecker();
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -0,0 +1,88 @@
+//=== DanglingInternalBufferChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
NoQ accepted this revision.
NoQ added a comment.
Looks great, thanks! I think you should add a check for UnknownVal and commit.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65
+ if (Call.isCalled(CStrFn)) {
+SymbolRef RawPtr = Call.getReturnValue().getAsSymbol();
+State = State->set(TypedR, RawPtr);
xazax.hun wrote:
> xazax.hun wrote:
> > I wonder if we can always get a symbol.
> > I can think of two cases when the call above could fail:
> > * Non-standard implementation that does not return a pointer
> > * The analyzer able to inline stuff and the returned value is a constant (a
> > specific address that is shared between all empty strings in some
> > implementation?)
> >
> > Even though I do find any of the above likely. @NoQ what do you think? Does
> > this worth a check?
> Sorry for the spam. Unfortunately, it is not possible to edit the comments.
> I do *not* find any of the above likely.
We should almost always check if any value is an `UnknownVal`. The engine
simply doesn't give any guarantees: it can give up any time and fall back to
`UnknownVal`.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:73
+if (State->contains(TypedR)) {
+ const SymbolRef *StrBufferPtr = State->get(TypedR);
+ const Expr *Origin = Call.getOriginExpr();
xazax.hun wrote:
> xazax.hun wrote:
> > What if no symbol is associated with the region? Won't this return null
> > that we dereference later on?
> Oh, never mind this one, I did not notice the `contains` call above.
The interesting part here is what happens if no expression is associated with
the call. For instance, if the call is an automatic destructor at the end of
scope. I hope it works, but i'm not sure how Origin is used.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1661
+ case AF_CXXNewArray:
+ case AF_InternalBuffer: {
if (IsALeakCheck) {
rnkovacs wrote:
> Is tying this new family to NewDeleteChecker reasonable? I did it because it
> was NewDelete that warned naturally before creating the new
> `AllocationFamily`. Should I introduce a new checker kind in MallocChecker
> for this purpose instead?
Uhm, this code is weird.
I think you can add an "external" ("associated", "plugin") CheckKind that
always warns.
https://reviews.llvm.org/D47135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
xbolva00 added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:33
+public:
+ DanglingInternalBufferChecker() : CStrFn("c_str") {}
+
string.data() support?
https://reviews.llvm.org/D47135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
rnkovacs added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1661
+ case AF_CXXNewArray:
+ case AF_InternalBuffer: {
if (IsALeakCheck) {
Is tying this new family to NewDeleteChecker reasonable? I did it because it
was NewDelete that warned naturally before creating the new `AllocationFamily`.
Should I introduce a new checker kind in MallocChecker for this purpose instead?
https://reviews.llvm.org/D47135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D47135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
rnkovacs updated this revision to Diff 148732.
rnkovacs added a comment.
Address (most) comments.
https://reviews.llvm.org/D47135
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/AllocationState.h
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/dangling-internal-buffer.cpp
Index: test/Analysis/dangling-internal-buffer.cpp
===
--- /dev/null
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -0,0 +1,71 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.DanglingInternalBuffer %s -analyzer-output=text -verify
+
+namespace std {
+
+template< typename CharT >
+class basic_string {
+public:
+ ~basic_string();
+ const CharT *c_str();
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end namespace std
+
+void consume(const char *) {}
+void consume(const wchar_t *) {}
+void consume(const char16_t *) {}
+void consume(const char32_t *) {}
+
+void deref_after_scope_char() {
+ const char *c;
+ {
+std::string s;
+c = 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_wchar_t() {
+ const wchar_t *w;
+ {
+std::wstring ws;
+w = 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_char16_t() {
+ const char16_t *c16;
+ {
+std::u16string s16;
+c16 = s16.c_str();
+ }
+ consume(c16); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char32_t() {
+ const char32_t *c32;
+ {
+std::u32string s32;
+c32 = s32.c_str();
+ }
+ consume(c32); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_ok() {
+ const char *c;
+ std::string s;
+ {
+c = s.c_str();
+ }
+ consume(c); // no-warning
+}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -30,6 +30,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
+#include "AllocationState.h"
#include
#include
@@ -45,7 +46,8 @@
AF_CXXNew,
AF_CXXNewArray,
AF_IfNameIndex,
- AF_Alloca
+ AF_Alloca,
+ AF_InternalBuffer
};
class RefState {
@@ -1467,6 +1469,7 @@
case AF_CXXNew: os << "'new'"; return;
case AF_CXXNewArray: os << "'new[]'"; return;
case AF_IfNameIndex: os << "'if_nameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific allocator"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("not a deallocation expression");
}
@@ -1479,6 +1482,7 @@
case AF_CXXNew: os << "'delete'"; return;
case AF_CXXNewArray: os << "'delete[]'"; return;
case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific deallocator"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("suspicious argument");
}
@@ -1653,7 +1657,8 @@
return Optional();
}
case AF_CXXNew:
- case AF_CXXNewArray: {
+ case AF_CXXNewArray:
+ case AF_InternalBuffer: {
if (IsALeakCheck) {
if (ChecksEnabled[CK_NewDeleteLeaksChecker])
return CK_NewDeleteLeaksChecker;
@@ -2991,6 +2996,20 @@
}
}
+namespace clang {
+namespace ento {
+namespace allocation_state {
+
+ProgramStateRef
+markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) {
+ AllocationFamily Family = AF_InternalBuffer;
+ return State->set(Sym, RefState::getReleased(Family, Origin));
+}
+
+} // end namespace allocation_state
+} // end namespace ento
+} // end namespace clang
+
void ento::registerNewDeleteLeaksChecker(CheckerManager ) {
registerCStringCheckerBasic(mgr);
MallocChecker *checker = mgr.registerChecker();
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -0,0 +1,85 @@
+//=== DanglingInternalBufferChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file defines a check that marks a raw
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65
+ if (Call.isCalled(CStrFn)) {
+SymbolRef RawPtr = Call.getReturnValue().getAsSymbol();
+State = State->set(TypedR, RawPtr);
xazax.hun wrote:
> I wonder if we can always get a symbol.
> I can think of two cases when the call above could fail:
> * Non-standard implementation that does not return a pointer
> * The analyzer able to inline stuff and the returned value is a constant (a
> specific address that is shared between all empty strings in some
> implementation?)
>
> Even though I do find any of the above likely. @NoQ what do you think? Does
> this worth a check?
Sorry for the spam. Unfortunately, it is not possible to edit the comments.
I do *not* find any of the above likely.
https://reviews.llvm.org/D47135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:73
+if (State->contains(TypedR)) {
+ const SymbolRef *StrBufferPtr = State->get(TypedR);
+ const Expr *Origin = Call.getOriginExpr();
xazax.hun wrote:
> What if no symbol is associated with the region? Won't this return null that
> we dereference later on?
Oh, never mind this one, I did not notice the `contains` call above.
https://reviews.llvm.org/D47135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
xazax.hun added a comment.
Looks good so far, some comments inline.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:58
+
+ auto *TypeDecl = TypedR->getValueType().getTypePtr()->getAsCXXRecordDecl();
+ if (TypeDecl->getName() != "basic_string")
QualType should have overloaded `->` operator, I think you can remove the
`getTypePtr`.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65
+ if (Call.isCalled(CStrFn)) {
+SymbolRef RawPtr = Call.getReturnValue().getAsSymbol();
+State = State->set(TypedR, RawPtr);
I wonder if we can always get a symbol.
I can think of two cases when the call above could fail:
* Non-standard implementation that does not return a pointer
* The analyzer able to inline stuff and the returned value is a constant (a
specific address that is shared between all empty strings in some
implementation?)
Even though I do find any of the above likely. @NoQ what do you think? Does
this worth a check?
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:73
+if (State->contains(TypedR)) {
+ const SymbolRef *StrBufferPtr = State->get(TypedR);
+ const Expr *Origin = Call.getOriginExpr();
What if no symbol is associated with the region? Won't this return null that we
dereference later on?
Comment at: test/Analysis/dangling-internal-buffer.cpp:24
+
+void deref_after_scope_char() {
+ const char *c;
I would like to see test cases that does not trigger warning.
https://reviews.llvm.org/D47135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++
rnkovacs updated this revision to Diff 148727.
rnkovacs retitled this revision from "[analyzer][WIP] A checker for dangling
string pointers in C++" to "[analyzer] A checker for dangling internal buffer
pointers in C++".
rnkovacs edited the summary of this revision.
rnkovacs added a comment.
- All `basic_string` types are now supported.
- Mock tests added.
- New `AllocationFamily` `AF_InternalBuffer` introduced.
- NewDeleteChecker dependency added.
https://reviews.llvm.org/D47135
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/AllocationState.h
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/dangling-internal-buffer.cpp
Index: test/Analysis/dangling-internal-buffer.cpp
===
--- /dev/null
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -0,0 +1,62 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.DanglingInternalBuffer %s -verify
+
+namespace std {
+
+template< typename CharT >
+class basic_string {
+public:
+ ~basic_string();
+ const CharT *c_str();
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end namespace std
+
+void consume(const char *) {}
+void consume(const wchar_t *) {}
+void consume(const char16_t *) {}
+void consume(const char32_t *) {}
+
+void deref_after_scope_char() {
+ const char *c;
+ {
+std::string s;
+c = 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_wchar_t() {
+ const wchar_t *w;
+ {
+std::wstring ws;
+w = 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_char16_t() {
+ const char16_t *c16;
+ {
+std::u16string s16;
+c16 = s16.c_str();
+ }
+ consume(c16); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char32_t() {
+ const char32_t *c32;
+ {
+std::u32string s32;
+c32 = s32.c_str();
+ }
+ consume(c32); // expected-warning {{Use of memory after it is freed}}
+ // expected-note@-1 {{Use of memory after it is freed}}
+}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -30,6 +30,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringExtras.h"
+#include "AllocationState.h"
#include
#include
@@ -45,7 +46,8 @@
AF_CXXNew,
AF_CXXNewArray,
AF_IfNameIndex,
- AF_Alloca
+ AF_Alloca,
+ AF_InternalBuffer
};
class RefState {
@@ -1467,6 +1469,7 @@
case AF_CXXNew: os << "'new'"; return;
case AF_CXXNewArray: os << "'new[]'"; return;
case AF_IfNameIndex: os << "'if_nameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific allocator"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("not a deallocation expression");
}
@@ -1479,6 +1482,7 @@
case AF_CXXNew: os << "'delete'"; return;
case AF_CXXNewArray: os << "'delete[]'"; return;
case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific deallocator"; return;
case AF_Alloca:
case AF_None: llvm_unreachable("suspicious argument");
}
@@ -1653,7 +1657,8 @@
return Optional();
}
case AF_CXXNew:
- case AF_CXXNewArray: {
+ case AF_CXXNewArray:
+ case AF_InternalBuffer: {
if (IsALeakCheck) {
if (ChecksEnabled[CK_NewDeleteLeaksChecker])
return CK_NewDeleteLeaksChecker;
@@ -2991,6 +2996,20 @@
}
}
+namespace clang {
+namespace ento {
+namespace allocation_state {
+
+ProgramStateRef
+markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) {
+ AllocationFamily Family = AF_InternalBuffer;
+ return State->set(Sym, RefState::getReleased(Family, Origin));
+}
+
+} // end namespace allocation_state
+} // end namespace ento
+} // end namespace clang
+
void ento::registerNewDeleteLeaksChecker(CheckerManager ) {
registerCStringCheckerBasic(mgr);
MallocChecker *checker = mgr.registerChecker();
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -0,0 +1,85 @@
+//=== DanglingInternalBufferChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of
