steakhal added a comment.
First and foremost, I think this is a great change. I think the diagnostic is
on the point.
BTW it seems like you missed two notes in `misc-ps-region-store.m` causing a
test fail:
error: 'note' diagnostics seen but not expected:
File
/home/steakhal/git/llvm-project/clang/test/Analysis/misc-ps-region-store.m Line
464: Original object declared here
File
/home/steakhal/git/llvm-project/clang/test/Analysis/misc-ps-region-store.m Line
467: Original object 'test_cwe466_return_outofbounds_pointer_a' is an array of
10 'int' objects, returned pointer points at index 11
2 errors generated.
---
The following comments are not about your actual change, I'm rather pointing
out further possibilities for improving the checker.
The checker seems to cover cases when we see the array, fine.
What about pointers/references to arrays? IMO we should trust those
declarations about the size of the pointee.
using size_t = decltype(sizeof 1);
using int10 = int[10];
template <class T> void clang_analyzer_dump(T);
size_t clang_analyzer_getExtent(void *);
int conjure_index();
int *test_ptr_to_array(int10 *arr) {
int x = conjure_index();
if (x != 20)
return *arr; // no-warning
clang_analyzer_dump(clang_analyzer_getExtent(*arr));
// expected-warning-re@-1
{{extent_${{[0-9]+}}{SymRegion{reg_${{[0-9]+}}<int10 * arr>}}}}
// FIXME: Above should be '40 S64b' on Linux x86_64.
int *result = *arr + x;
clang_analyzer_dump(result);
// expected-warning-re@-1 {{&Element{SymRegion{reg_${{[0-9]+}}<int10 *
arr>},20 S64b,int}}}
return result; // We should expect a warning here.
}
Please add this test to your patch.
It seems like the checker would catch it if `MemRegionManager::getStaticSize()`
would consider the type of the `SymbolicRegion` for `ConstantArray` pointee
types. I'll have a patch about that.
================
Comment at: clang/test/Analysis/return-ptr-range.cpp:11
+int *test_global_ptr() {
+ do { // expected-note{{Loop condition is false. Exiting loop}}
int x = conjure_index();
----------------
I would rather use a simple block `{...}` for opening a scope, but I don't know
why you don't declare `ptr` in the original scope in the first place.
People usually use `do {} while(0)` constructs if they want to use `break`
somewhere ~~ like a `goto` OR they implement a macro. You are doing none of
these.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107051/new/
https://reviews.llvm.org/D107051
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
