[PATCH] D136671: [analyzer] Fix crash for array-delete of UnknownVal values.

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:1244
                                             SVal *ElementCountVal) {
+  assert(Region != nullptr && "Null-regions passed to prepareStateForArrayDestruction.");
 
----------------
I meant to assert this inside `getDynamicElementCount()` as that's the function that dereferences the pointer.

```
DefinedOrUnknownSVal getDynamicElementCount(..., const MemRegion *MR, ...) {
  MR = MR->StripCasts();                                                            <-- crash here if MR is a nullptr

  DefinedOrUnknownSVal Size = getDynamicExtent(State, MR, SVB);
  SVal ElementSize = getElementExtent(ElementTy, SVB);

  SVal ElementCount =
      SVB.evalBinOp(State, BO_Div, Size, ElementSize, SVB.getArrayIndexType());     <-- this could be the origin of 
                                                                                        a crash too

  return ElementCount.castAs<DefinedOrUnknownSVal>();
}
```

Also `SVB.evalBinOp()` can technically return an `UndefinedVal` and if it does that, then `ElementCount.castAs<DefinedOrUnknownSVal>()` will also cause a crash as it happened in D130974.

Also since this function can return an `UnknownSVal`, what do you think about returning that if `MR` is a `nullptr` instead of stopping execution? Would that behaviour even make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136671