[PATCH] D71612: [analyzer] Add PlacementNewChecker

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:91
+  SVal SizeOfPlace = getExtentSizeOfPlace(C, Place, State);
+  const auto SizeOfTargetCI = SizeOfTarget.getAs<nonloc::ConcreteInt>();
+  if (!SizeOfTargetCI)
----------------
martong wrote:
> xazax.hun wrote:
> > Here, instead of getting `SizeOfTarget` and `SizeOfPlace` as `ConcreteInt`s, I think you should rather use `evalBinOp` to compare them. That method is more future proof as if we cannot constraint these values down to a single integer but we still have some information about them a sufficiently smart solver could prove the relationship between the symbolic values.
> I am not sure if `evalBinOp` is that useful here, because we need the concrete values anyway when we issue the diagnostics. We'd like to present the concrete sizes in bytes.
The reason why evalbinop might be useful because we might have symbolic sizes:
```
void f(int a) {
 char *buffer = new char[a];
}
```

So in the code snippet above you cannot get a concrete integer for the size of the buffer. But in case we already have some constraints about the value of `a`, the constraint solver might be able to tell if we are sure that the type will not fit into the buffer. I can imagine that this scenario is relatively rare, but I think we need relatively little code to support this. 

So you could potentially warn when:
```
void f(int a) {
  char *buffer = new char[a];
  if (a > 3)
    return;
  int *p = new (buffer) int;
}
```

I know, this is silly code, but we might not know if there are reasonable code that has similar patterns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71612