[PATCH] D49570: [analyzer] Improve warning messages and notes of DanglingInternalBufferChecker
rnkovacs marked an inline comment as done.
rnkovacs added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:253
+ allocation_state::getContainerObjRegion(N->getState(), PtrToBuf);
+ const auto *TypedRegion = dyn_cast(ObjRegion);
+ QualType ObjTy = TypedRegion->getValueType();
NoQ wrote:
> `dyn_cast` may fail by returning a null pointer. This either needs to be
> changed to `cast` or there needs to be a check for a null pointer before use.
> I guess it should be a `cast` because you're only acting on typed regions in
> the checker itself.
I hope that in a few more reviews I'll learn to use all of these properly.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2931-2932
+OS << "deallocated by call to destructor";
+StackHint = new StackHintGeneratorForSymbol(Sym,
+ "Returning; inner buffer was
deallocated");
} else {
NoQ wrote:
> Cool stuff!
:)
Comment at: test/Analysis/dangling-internal-buffer.cpp:63
// 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}}
+consume(c); // expected-warning {{Deallocated pointer returned to the
caller}}
+// expected-note@-1 {{Deallocated pointer returned to the caller}}
NoQ wrote:
> Mm, nono, there's no `return` statement here, so we shouldn't say that our
> pointer is returned to the caller. Whether it should say "returned to the
> caller" or not, should not depend on the allocation family, but on the kind
> of "use" we encounter "after" "free".
I don't even know how this went so off, sorry.
https://reviews.llvm.org/D49570
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49570: [analyzer] Improve warning messages and notes of DanglingInternalBufferChecker
rnkovacs updated this revision to Diff 160193.
rnkovacs marked 3 inline comments as done.
rnkovacs added a comment.
Address comments & rebase.
https://reviews.llvm.org/D49570
Files:
lib/StaticAnalyzer/Checkers/AllocationState.h
lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/inner-pointer.cpp
Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -65,106 +65,106 @@
const char *c, *d;
{
std::string 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}}
+c = s.c_str(); // expected-note {{Pointer to inner buffer of 'std::string' obtained here}}
+d = s.data(); // expected-note {{Pointer to inner buffer of 'std::string' obtained here}}
+ }// expected-note {{Inner buffer of 'std::string' deallocated by call to destructor}}
+ // expected-note@-1 {{Inner buffer of 'std::string' deallocated by call to destructor}}
std::string s;
const char *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}}
+consume(c); // expected-warning {{Inner pointer of container used after re/deallocation}}
+// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
} else {
-consume(d); // expected-warning {{Use of memory after it is freed}}
-// expected-note@-1 {{Use of memory after it is freed}}
+consume(d); // expected-warning {{Inner pointer of container used after re/deallocation}}
+// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
}
}
void deref_after_scope_char_data_non_const() {
char *c;
{
std::string s;
-c = s.data(); // expected-note {{Dangling inner pointer obtained here}}
- } // expected-note {{Inner pointer invalidated by call to destructor}}
+c = s.data(); // expected-note {{Pointer to inner buffer of 'std::string' obtained here}}
+ } // expected-note {{Inner buffer of 'std::string' deallocated 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}}
+ consume(c); // expected-warning {{Inner pointer of container used after re/deallocation}}
+ // expected-note@-1 {{Inner pointer of container used after re/deallocation}}
}
void deref_after_scope_wchar_t(bool cond) {
const wchar_t *c, *d;
{
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}}
+c = s.c_str(); // expected-note {{Pointer to inner buffer of 'std::wstring' obtained here}}
+d = s.data(); // expected-note {{Pointer to inner buffer of 'std::wstring' obtained here}}
+ }// expected-note {{Inner buffer of 'std::wstring' deallocated by call to destructor}}
+ // expected-note@-1 {{Inner buffer of 'std::wstring' deallocated 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}}
+consume(c); // expected-warning {{Inner pointer of container used after re/deallocation}}
+// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
} else {
-consume(d); // expected-warning {{Use of memory after it is freed}}
-// expected-note@-1 {{Use of memory after it is freed}}
+consume(d); // expected-warning {{Inner pointer of container used after re/deallocation}}
+// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
}
}
void deref_after_scope_char16_t_cstr() {
const char16_t *c16;
{
std::u16string s16;
-c16 = s16.c_str(); // expected-note {{Dangling inner pointer obtained here}}
- }// expected-note
[PATCH] D49570: [analyzer] Improve warning messages and notes of DanglingInternalBufferChecker
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:253
+ allocation_state::getContainerObjRegion(N->getState(), PtrToBuf);
+ const auto *TypedRegion = dyn_cast(ObjRegion);
+ QualType ObjTy = TypedRegion->getValueType();
`dyn_cast` may fail by returning a null pointer. This either needs to be
changed to `cast` or there needs to be a check for a null pointer before use. I
guess it should be a `cast` because you're only acting on typed regions in the
checker itself.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2931-2932
+OS << "deallocated by call to destructor";
+StackHint = new StackHintGeneratorForSymbol(Sym,
+ "Returning; inner buffer was
deallocated");
} else {
Cool stuff!
Comment at: test/Analysis/dangling-internal-buffer.cpp:63
// 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}}
+consume(c); // expected-warning {{Deallocated pointer returned to the
caller}}
+// expected-note@-1 {{Deallocated pointer returned to the caller}}
Mm, nono, there's no `return` statement here, so we shouldn't say that our
pointer is returned to the caller. Whether it should say "returned to the
caller" or not, should not depend on the allocation family, but on the kind of
"use" we encounter "after" "free".
Comment at: test/Analysis/dangling-internal-buffer.cpp:75
std::string s;
-c = s.data(); // expected-note {{Dangling inner pointer obtained here}}
- } // expected-note {{Inner pointer invalidated by call to
destructor}}
+c = s.data(); // expected-note {{Pointer to inner buffer of 'std::string'
object obtained here}}
+ } // expected-note {{Inner buffer deallocated by call to
destructor}}
I suggest a shorter `on a 'std::string'` instead of `on 'std::string' object`.
Repository:
rC Clang
https://reviews.llvm.org/D49570
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49570: [analyzer] Improve warning messages and notes of DanglingInternalBufferChecker
rnkovacs created this revision.
rnkovacs added reviewers: NoQ, dcoughlin, xazax.hun, george.karpenkov.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, szepet,
baloghadamsoftware, whisperity.
Following the discussion at https://reviews.llvm.org/D49360.
Added two more test cases that show "returning"-type-of notes as well.
Repository:
rC Clang
https://reviews.llvm.org/D49570
Files:
lib/StaticAnalyzer/Checkers/AllocationState.h
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
@@ -49,232 +49,264 @@
const char *c, *d;
{
std::string 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}}
+c = s.c_str(); // expected-note {{Pointer to inner buffer of 'std::string' object obtained here}}
+d = s.data(); // expected-note {{Pointer to inner buffer of 'std::string' object obtained here}}
+ }// expected-note {{Inner buffer deallocated by call to destructor}}
+ // expected-note@-1 {{Inner buffer deallocated by call to destructor}}
std::string s;
const char *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}}
+consume(c); // expected-warning {{Deallocated pointer returned to the caller}}
+// expected-note@-1 {{Deallocated pointer returned to the caller}}
} else {
-consume(d); // expected-warning {{Use of memory after it is freed}}
-// expected-note@-1 {{Use of memory after it is freed}}
+consume(d); // expected-warning {{Deallocated pointer returned to the caller}}
+// expected-note@-1 {{Deallocated pointer returned to the caller}}
}
}
void deref_after_scope_char_data_non_const() {
char *c;
{
std::string s;
-c = s.data(); // expected-note {{Dangling inner pointer obtained here}}
- } // expected-note {{Inner pointer invalidated by call to destructor}}
+c = s.data(); // expected-note {{Pointer to inner buffer of 'std::string' object obtained here}}
+ } // expected-note {{Inner buffer deallocated 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}}
+ consume(c); // expected-warning {{Deallocated pointer returned to the caller}}
+ // expected-note@-1 {{Deallocated pointer returned to the caller}}
}
void deref_after_scope_wchar_t(bool cond) {
const wchar_t *c, *d;
{
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}}
+c = s.c_str(); // expected-note {{Pointer to inner buffer of 'std::wstring' object obtained here}}
+d = s.data(); // expected-note {{Pointer to inner buffer of 'std::wstring' object obtained here}}
+ }// expected-note {{Inner buffer deallocated by call to destructor}}
+ // expected-note@-1 {{Inner buffer deallocated 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}}
+consume(c); // expected-warning {{Deallocated pointer returned to the caller}}
+// expected-note@-1 {{Deallocated pointer returned to the caller}}
} else {
-consume(d); // expected-warning {{Use of memory after it is freed}}
-// expected-note@-1 {{Use of memory after it is freed}}
+consume(d); // expected-warning {{Deallocated pointer returned to the caller}}
+// expected-note@-1 {{Deallocated pointer returned to the caller}}
}
}
void deref_after_scope_char16_t_cstr() {
const char16_t *c16;
{
std::u16string s16;
-c16 =
