[cfe-commits] r120177 - /cfe/trunk/lib/Checker/ReturnPointerRangeChecker.cpp

[Xu Zhongxing]

On Sun, Nov 28, 2010 at 4:03 PM, Ted Kremenek <kremenek at apple.com> wrote:

> Hmm.  I think I see what you mean.  I agree that removing the use of
> StripCasts() is appropriate.
>
> That said, do you think both ReturnPointerRangeChecker and
> ArrayBoundChecker are doing the bounds checks in the most appropriate way?
>  I think the motivation for using StripCasts() (even though it was wrong)
> was to try and reason about out-of-bounds accesses using the extent of the
> raw memory region.
>
> For example, this is what is going on in RegionStore::getSizeInElements():
>
>   DefinedOrUnknownSVal RegionStoreManager::getSizeInElements(const GRState
> *state,
>                                                              const
> MemRegion *R,
>                                                              QualType
> EleTy) {
>     SVal Size = cast<SubRegion>(R)->getExtent(ValMgr);
>     SValuator &SVator = ValMgr.getSValuator();
>     ...
>
> I think the motivation for using StripCasts() what so that we were doing
> the bounds check relative to the base memory region (for which we have an
> extent).  By removing the use of StripCasts(), any time (I believe) we
> introduce an ElementRegion due to a cast this bounds check won't work.
>
> Should we instead be doing the bounds check in terms of raw offsets
> (relative to the underlying base region)?
>

You are right! We should be doing the bounds check in terms of raw offsets.
The current check is wrong.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20101128/a30a3d7e/attachment.html>