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

[Ted Kremenek]

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.