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

[a.sidorin]

Summary

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

[krememek]

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

[a.sidorin]

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.

[a.sidorin]

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]

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.

[a.sidorin]

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.

lib/StaticAnalyzer/Core/RegionStore.cpp

         const SymbolicRegion *SR = cast<SymbolicRegion>(MR);
         T = SR->getSymbol()->getType();
       }
+      if (T->isAnyPointerType() || T->isReferenceType())
+        T = T->getPointeeType();
     }
     MR = GetElementZeroRegion(MR, T);
   }