<br><br><div class="gmail_quote">On Mon, May 31, 2010 at 9:26 AM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div class="im">On May 30, 2010, at 3:21 PM, Jordy Rose wrote:<br>
<br>
</div><div class="im">>> As for the code snippet in PR 7218:<br>
>>  char broken (char a) {<br>
>>      char buf[2];<br>
>>      buf[0] = a;<br>
>>      return buf[1]; // should warn but does not<br>
>>  }<br>
>> This should be fixed by detecting if we are going outside the bounds of<br>
>> the memory block.  I don't think this requires changing the binding<br>
> model<br>
>> in RegionStoreManager.<br>
><br>
> It's not an out-of-bounds issue, it's about garbage being returned. The<br>
> region bounds checking works fine (outofbound.c).<br>
<br>
</div>The issue is that the following code block in RegionStore::RetrieveElement:<br>
<div class="im"><br>
  if (const Optional<SVal> &V = getDirectBinding(B, superR)) {<br>
    if (SymbolRef parentSym = V->getAsSymbol()) {<br>
      return ValMgr.getDerivedRegionValueSymbolVal(parentSym, R);<br>
    }<br>
<br>
    if (V->isUnknownOrUndef())<br>
      return *V;<br>
<br>
    // Handle LazyCompoundVals for the immediate super region.  Other cases<br>
    // are handled in 'RetrieveFieldOrElementCommon'.<br>
    if (const nonloc::LazyCompoundVal *LCV =<br>
        dyn_cast<nonloc::LazyCompoundVal>(V)) {<br>
      R = MRMgr.getElementRegionWithSuper(R, LCV->getRegion());<br>
      return RetrieveElement(LCV->getStore(), R);<br>
    }<br>
<br>
    // Other cases: give up.<br>
    return UnknownVal();<br>
  }<br>
<br>
</div>is a hack.  If we remove this code only one test fails: no-outofbounds.c (with PR7218.c now passing).<br>
<br>
This code was added to support addressing within a larger (non-array, non-struct) object, e.g.:<br>
<br>
  int x = 10;<br>
  char *y = ((char*) &x) + 1;<br>
  return *y;<br>
<br>
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:<br>
<br>
 <a href="http://llvm.org/viewvc/llvm-project?view=rev&revision=105195" target="_blank">http://llvm.org/viewvc/llvm-project?view=rev&revision=105195</a><br>
<br>
Zhongxing: Please review this patch and let me know what you think.<br>
<br>
Jordy: Please review it as well.  Once we're satisfied, let's move on to talking about your changes to MallocChecker.<br>
<br>
<br>
<br></blockquote><div><br>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.<br>
 </div></div><br>