[cfe-commits] [PATCH] Fix for PR7218, and analyzer support for calloc()

[Zhongxing Xu]

On Mon, May 31, 2010 at 9:26 AM, Ted Kremenek <kremenek at apple.com> wrote:

> On May 30, 2010, at 3:21 PM, Jordy Rose wrote:
>
> >> As for the code snippet in PR 7218:
> >>  char broken (char a) {
> >>      char buf[2];
> >>      buf[0] = a;
> >>      return buf[1]; // should warn but does not
> >>  }
> >> This should be fixed by detecting if we are going outside the bounds of
> >> the memory block.  I don't think this requires changing the binding
> > model
> >> in RegionStoreManager.
> >
> > It's not an out-of-bounds issue, it's about garbage being returned. The
> > region bounds checking works fine (outofbound.c).
>
> The issue is that the following code block in RegionStore::RetrieveElement:
>
>  if (const Optional<SVal> &V = getDirectBinding(B, superR)) {
>    if (SymbolRef parentSym = V->getAsSymbol()) {
>      return ValMgr.getDerivedRegionValueSymbolVal(parentSym, R);
>    }
>
>    if (V->isUnknownOrUndef())
>      return *V;
>
>    // Handle LazyCompoundVals for the immediate super region.  Other cases
>    // are handled in 'RetrieveFieldOrElementCommon'.
>    if (const nonloc::LazyCompoundVal *LCV =
>        dyn_cast<nonloc::LazyCompoundVal>(V)) {
>      R = MRMgr.getElementRegionWithSuper(R, LCV->getRegion());
>      return RetrieveElement(LCV->getStore(), R);
>    }
>
>    // Other cases: give up.
>    return UnknownVal();
>  }
>
> is a hack.  If we remove this code only one test fails: no-outofbounds.c
> (with PR7218.c now passing).
>
> This code was added to support addressing within a larger (non-array,
> non-struct) object, e.g.:
>
>  int x = 10;
>  char *y = ((char*) &x) + 1;
>  return *y;
>
> The code was too aggressive in the cases it handled.  I've now checked in a
> refinement which causes the test case for PR 7218 to now pass:
>
>  http://llvm.org/viewvc/llvm-project?view=rev&revision=105195
>
> Zhongxing: Please review this patch and let me know what you think.
>
> Jordy: Please review it as well.  Once we're satisfied, let's move on to
> talking about your changes to MallocChecker.
>
>
>
>
I think this fix suffices for now. Limiting the hack to the only case
mentioned above is reasonable. For other cases we don't query the direct
binding of super regions of element regions.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100531/1ac1fa8a/attachment.html>