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

[Balázs Kéri via Phabricator via cfe-commits]

balazske 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 was looking at CERT ARR32-C <https://wiki.sei.cmu.edu/confluence/display/c/ARR32-C.+Ensure+size+arguments+for+variable+length+arrays+are+in+a+valid+range> "Ensure size arguments for variable length arrays are in a valid range". The VLA should not have a size that is larger than `std::numeric_limits<size_t>::max()`, in other words "fit into a size_t value", or not?

Yes creating the too large VLA in itself is not a bug, only when `sizeof` is called on it because it can not return the correct size. A non-fatal error is a better option, or delay the check until the sizeof call. But probably the create of such a big array in itself is sign of code smell. The array actually does not need to be created to make the problem happen, only a sizeof call on a typedef-ed and too large VLA. (What does mean that "result of sizeof is implementation-defined"? Probably it can return not the size in bytes or "chars" but something other? In such a case the checker would be not correct.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79330/new/

https://reviews.llvm.org/D79330