Page MenuHomePhabricator

[analyzer][Bugfix] RegionStore: use pointee type to create one-element regions
Needs ReviewPublic

Authored by a.sidorin on Aug 19 2014, 10:50 AM.

Details

Summary

This patch fixes an issue that makes analyzer to create additional symbols for pointer variables (binding is not recognized even if it exists).

Diff Detail

Event Timeline

a.sidorin updated this revision to Diff 12671.Aug 19 2014, 10:50 AM
a.sidorin retitled this revision from to [analyzer][Bugfix] RegionStore: use pointee type to create one-element regions.
a.sidorin updated this object.
a.sidorin edited the test plan for this revision. (Show Details)
a.sidorin added a subscriber: Unknown Object (MLST).
krememek edited edge metadata.Aug 19 2014, 12:02 PM

Can you please provide a test case that shows what problem this fixed?

I met this issue while debugging so it will take time to write a test case. But you can see that GetElementZeroRegion() expects element type but pointer type is returned if Type.isNull() == true branch. I'll provide a test case later.

Here is a sample test case:

char passAndModifyPtr(int *p) {
  if (*p > 10) {
    *p = 4;
    return 0;
  } else {
    if (*p < 5) {
      *p = 7;
      return 1;
    }
  }
  return 2;
}

And this code should be inserted in checkDeadSymbols() callback of some checker:

for (SymbolReaper::dead_iterator I = SymReaper.dead_begin(),
    E = SymReaper.dead_end(); I != E; ++I) {
  SymbolRef Sym = *I;
  Sym->dump();
  llvm::errs() << "\n";
  // One of the symbols reaped is a value associated with *p
  if (const SymbolRegionValue *V = dyn_cast<SymbolRegionValue>(Sym)) {
    const MemRegion *MR = V->getRegion();
    // one-element ElementRegion is created automatically
    if (const ElementRegion *ER = dyn_cast<ElementRegion>(MR)) {
      const MemRegion *BaseReg = ER->getBaseRegion();
      // BaseReg is a SymRegion{reg_$0<p>}
      // Its value should be nonloc::SymbolVal(Sym)
      // i.e. reg_$1<element{SymRegion{reg_$0<p>},0 S32b,int}>
      BaseReg->dump();
      llvm::errs() << "\n";
      State->getSVal(BaseReg).dump();// but wrong SVal is created here
    }
  }
}

SVal related to *p is nonloc::SymbolVal(reg_$1<element{SymRegion{reg_$0<p>},0 S32b,int}>). The same value should be retrieved by getSVal(SymRegion{reg_$0<p>}). But the result of this action is &SymRegion{reg_$2<element{SymRegion{reg_$0<p>},0 S32b,int *}>} which seems to be incorrect.

jordan_rose edited edge metadata.Sep 4 2014, 9:32 AM

Looking at this again I think this is correct (since both TypedRegion::getLocationType and SR->getSymbol()->getType() should return the type of the region's address, not its contents). Is there any way that this can affect results, though? Say, by resulting in an assertion failure or a false positive? We'd rather not add tests that depend on debugging output.

I have made some investigations. This method is not used frequently and its users ignore return type so I was unable to get a false positive or crash due to this bug. If you have any idea about a test case, please share it.