[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-29 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. Strange: Even if the assume for zero index size does not indicate problem (assume that index length is not zero) the `getKnownValue` returns 0 for it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79330/new/ https://revie

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. https://bugs.llvm.org/show_bug.cgi?id=46128 looks like a crash caused by this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79330/new/ https://reviews.llvm.org/D79330 ___ c

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-19 Thread Balázs Kéri via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG51bb2128ef03: [Analyzer][VLASizeChecker] Check for VLA size overflow. (authored by balazske). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79330/new/ https

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-18 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 264572. balazske added a comment. Rebase, added checker name to overflow test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79330/new/ https://reviews.llvm.org/D79330 Files: clang/lib/StaticAnalyzer/Checke

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-15 Thread Whisperity via Phabricator via cfe-commits
whisperity added a comment. In D79330#2035850 , @balazske wrote: > I was looking at CERT ARR32-C > > "Ensure size arguments for

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-15 Thread Whisperity via Phabricator via cfe-commits
whisperity added inline comments. Comment at: clang/test/Analysis/vla-overflow.c:8 + if (x == BIGINDEX) { +// We expect here that size_t is a 64 bit value. +// Size of this array should be the first to overflow. While it's generally true nowadays, instea

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-14 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. Is it worth to improve the checker by emitting a warning only if a `sizeof` is used on a `typedef`-ed VLA type where the size is too large (and at a VLA declaration with size overflow)? Or can this be done in a later change? Repository: rG LLVM Github Monorepo CHAN

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-14 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. In D79330#2033990 , @Szelethus wrote: > > Variable-length array (VLA) should have a size that fits into a size_t > > value. At least if the size is queried with sizeof, but it is better (and > > more simple) to check it always >

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. In D79330#2034414 , @martong wrote: > I am not sure if I can follow your concern here. > `sizeof(size_t)` is typically 8, so that is not a bug, n

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-13 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision. martong added a comment. In D79330#2033990 , @Szelethus wrote: > > Variable-length array (VLA) should have a size that fits into a size_t > > value. At least if the size is queried with sizeof, but it is better (and > > mo

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-13 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D79330#2026564 , @balazske wrote: > I do not know if it would me **much** cleaner. > > - First part: Move calculation of `ArraySize` into `checkVLA` and rename > `checkVLASize` to `checkVLAIndexSize`. > - Second part: Add the c

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. > Variable-length array (VLA) should have a size that fits into a size_t value. > At least if the size is queried with sizeof, but it is better (and more > simple) to check it

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-13 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added inline comments. Comment at: clang/test/Analysis/vla-overflow.c:10 +// Size of this array should be the first to overflow. +size_t s = sizeof(char[x][x][x][x]); // expected-warning{{Declared variable-length array (VLA) has too large size}} +return s;

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-11 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 263151. balazske added a comment. Added target dependent tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79330/new/ https://reviews.llvm.org/D79330 Files: clang/lib/StaticAnalyzer/Checkers/VLASizeCheck

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-11 Thread Endre Fülöp via Phabricator via cfe-commits
gamesh411 accepted this revision. gamesh411 added a comment. This revision is now accepted and ready to land. With the minor adjustment in the one test case this LGTM. Comment at: clang/test/Analysis/vla.c:107 + if (x == BIGINDEX) { +size_t s = sizeof(int[x][x][x][x]); //

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-08 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. I do not know if it would me **much** cleaner. - First part: Move calculation of `ArraySize` into `checkVLA` and rename `checkVLASize` to `checkVLAIndexSize`. - Second part: Add the check for total array size to `checkVLA`. The first part in itself does not make sense

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-07 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Would it be possible to split this patch into two? 1. The refactoring part where you move code out from checkPreStmt to checkVLA. This should be an NFC. 2. Handling of the overflow. Would be much cleaner I guess. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-04 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added a comment. Question: Is there a way to check for overflow in the symbolic domain (that is the `ArrSize` value)? Or get the maximal possible value of a `SVal` if it is constrained? (The `ArrSize` that is a multiplication of every dimension size can not be checked simply for too bi

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-04 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments. Comment at: clang/test/Analysis/vla.c:107 + if (x == BIGINDEX) { +size_t s = sizeof(int[x][x][x][x]); // expected-warning{{Declared variable-length array (VLA) has too large size}} +return s; I think we could make the arit

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

2020-05-04 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: clang. Variable-length