steakhal added a comment.
In D107051#2928089 <https://reviews.llvm.org/D107051#2928089>, @steakhal wrote:
> 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.
It seems like we cannot implement this in a way fitting into the current memory
model. To be able to do that we should be able to differentiate these two cases:
void test(int10 *arr) {
clang_analyzer_dump((int*)(arr + 2)); // allow: `arr` might be a pointer to
an array of `int10[100]`, in which case the resulting pointer is safe to use
clang_analyzer_dump(*arr + 20); // disallow: `*arr` :: `int[10]`,
creating a pointer to the 20th element
// Both dumps are the same: &Element{SymRegion{reg_$0<int10 * arr>},20
S64b,int}
}
So, I'm actually puzzled about this. I think you can leave out the extra test
case I proposed.
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
