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)?
On Nov 27, 2010, at 11:49 PM, Xu Zhongxing wrote:
> No. I want to reduce the uses of StripCasts. It shouldn't be used everywhere.
> Here we should check the zero index explicitly.
>
> On Sun, Nov 28, 2010 at 3:45 PM, Ted Kremenek <[email protected]> wrote:
> Hi Zhongxing,
>
> What is the motivation for this change? Did the previous logic cause a test
> case to fail?
>
> On Nov 26, 2010, at 1:07 AM, Zhongxing Xu <[email protected]> wrote:
>
> > Author: zhongxingxu
> > Date: Fri Nov 26 03:07:38 2010
> > New Revision: 120177
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=120177&view=rev
> > Log:
> > Should not use StripCasts() in this context.
> >
> > Modified:
> > cfe/trunk/lib/Checker/ReturnPointerRangeChecker.cpp
> >
> > Modified: cfe/trunk/lib/Checker/ReturnPointerRangeChecker.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/ReturnPointerRangeChecker.cpp?rev=120177&r1=120176&r2=120177&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Checker/ReturnPointerRangeChecker.cpp (original)
> > +++ cfe/trunk/lib/Checker/ReturnPointerRangeChecker.cpp Fri Nov 26 03:07:38
> > 2010
> > @@ -48,19 +48,16 @@
> >
> > SVal V = state->getSVal(RetE);
> > const MemRegion *R = V.getAsRegion();
> > - if (!R)
> > - return;
> > -
> > - R = R->StripCasts();
> > - if (!R)
> > - return;
> >
> > const ElementRegion *ER = dyn_cast_or_null<ElementRegion>(R);
> > if (!ER)
> > return;
> >
> > DefinedOrUnknownSVal Idx = cast<DefinedOrUnknownSVal>(ER->getIndex());
> > -
> > + // Zero index is always in bound, this also passes ElementRegions
> > created for
> > + // pointer casts.
> > + if (Idx.isZeroConstant())
> > + return;
> > // FIXME: All of this out-of-bounds checking should eventually be
> > refactored
> > // into a common place.
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > [email protected]
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
