martong accepted this revision. martong added a comment. In D79330#2033990 <https://reviews.llvm.org/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 > > So creating VLA larger than `sizeof(size_t)` isn't a bug, bur rather a sign > of code smell? Then we shouldn't create a fatal error node for it, **unless** > we're trying to fit it in a variable that isn't sufficiently large. The fact > that `sizeof`ing it is a bug wasn't immediately obvious to me either, so a > quote from the standard as comments would be appreciated: > > §6.5.3.4.4 <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf>, about > operator sizeof: The value of the result is implementation-defined, and its > type (an unsigned integer type) is `size_t`, defined in `<stddef.h>` (and > other headers). I am not sure if I can follow your concern here. `sizeof(size_t)` is typically 8, so that is not a bug, neither a code smell to have `char VLA[sizeof(size_t)];`. The problem is when the size is bigger than the maximum value of `size_t`, that ix 0xff...ff, as we can see that in the new tests. Besides, not having the size printed out in the warning is not a blocker for me, this looks good enough. ================ 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; ---------------- Szelethus wrote: > Let's not trim be checker name here. > > Also, we could mention what the specific size is. Yes, that's a good idea to print the actual size of the VLA when we have that info. But I think we cannot just print that when it overflows! :D We can, however, print the maximum allowed value in case of the overflow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79330/new/ https://reviews.llvm.org/D79330 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
