[PATCH] D129280: [analyzer] PlacementNewChecker, properly handle array overhead (cookie)

[Gabor Marton via Phabricator via cfe-commits]

martong marked an inline comment as done.
martong added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:157
             "Storage provided to placement new is only {0} bytes, "
-            "whereas the allocated array type requires more space for "
-            "internal needs",
-            SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue()));
+            "whereas the allocated array type might require more space for "
+            "allocation overhead",
----------------
NoQ wrote:
> "might" is not very convincing, it may cause a reaction like "I've no idea what it's talking about and the compiler itself isn't sure, must be false positive". Can we do anything to point the user in the right direction? Say, if this is implementation-defined, are we looking at a portability issue?
Okay, I see your point. Let's dissect the corresponding sections of the standard:
```
new(2, f) T[5] results in a call of operator new[](sizeof(T) * 5 + y, 2, f).

Here, ... and y are non-negative unspecified values representing array allocation overhead;
```
The array overhead is an **unspecified value**. What does it mean exactly? My understanding is that this means it is implementation defined and the implementation is not required to document or guarantee anything. I came to this conclusion based on this [[ https://stackoverflow.com/questions/2397984/undefined-unspecified-and-implementation-defined-behavior | stackoverflow question ]]. 

My interpretation could be wrong, but that does not matter. I think, we should just simply display the user what the standard says, and let them digest themselves the meaning of "unspecified". I am updating the patch like so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129280