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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
