[PATCH] D24307: calculate extent size for memory regions allocated by C++ new expression

[Daniel Krupp via cfe-commits]

dkrupp added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:83
@@ -78,1 +82,3 @@
+      // we can assume that the region starts at 0.
+      if (!state->isNull(extentVal).isConstrained()) {
         return UnknownVal();
----------------
NoQ wrote:
> Perhaps you could consider the memory space of the `region`, it would look a bit less hacky to me.
> 
> In my dreams, i wish heap regions were no longer symbolic regions, and this hack would go away then.
> 
> Also, i recall there is a bug in `isNull()`: in the `ConstraintManager` class (this time i actually mean //the abstract base class// of `RangeConstraintManager`) this function boils down to `assume()`, but in `RangeConstraintManager` it is overridden to do a direct lookup into the constraint map; which means that in fact this function does not simplify symbolic expressions before answering. This code is probably unaffected because extents are always either concrete or atomic symbols, but i think i'd make a patch for that.
Good point!
region->getMemorySpace() does a very similar recursion as the while loop in this function. So I  guess the while loop can be refactored like this:

```
static SVal computeExtentBegin(SValBuilder &svalBuilder, 
const MemRegion *region) {
  const MemSpaceRegion *SR = region->getMemorySpace();
  if (SR->getKind() == MemRegion::UnknownSpaceRegionKind)
    return UnknownVal();
  else
    return svalBuilder.makeZeroArrayIndex();
 }
```
All test cases pass. Particularly it filters out this false positive from out-of-bounds.c :

```
// Don't warn when indexing below the start of a symbolic region's whose 
// base extent we don't know.
int *get_symbolic();
void test_index_below_symboloc() {
  int *buf = get_symbolic();
  buf[-1] = 0; // no-warning;
}
```



https://reviews.llvm.org/D24307