[cfe-commits] [PATCH] getLValueElement

[Ted Kremenek]

On Oct 22, 2008, at 7:35 AM, Zhongxing Xu wrote:

> This implements the getLValueElement function for RegionStore. It  
> only handle integer indices currently.

Okay.

>
> The base region is assumed an ElementRegion because we expect  
> VisitCast() to evaluate the base expr's
> value to be loc::MemRegionVal(ElementRegion). This is consistent  
> with the C standard, which says
> expression of type array of T is cast to pointer to T.

Is the idea for VisitCast to return the ElementRegion at index 0?   
Where would this logic actually be performed?  VisitCast (in  
GRExprEngine) should have no notion of ElementRegion, as that is  
something specific to a given Store implementation.  Do we need to add  
another Store method that GRExprEngine::VisitCast calls back to?

+SVal RegionStoreManager::getLValueElement(const GRState* St,
+                                          SVal Base, SVal Offset) {
+  if (Base.isUnknownOrUndef())
+    return Base;
+
+  Loc BaseL = cast<Loc>(Base);
+
+  switch (BaseL.getSubKind()) {
+  default:
+    assert("Other cases are not handled yet.");

This assertion will always be true.  Did you mean assert(false &&  
"...")?

+    return UnknownVal();
+
+  case loc::MemRegionKind: {
+    // We expect BaseR is an ElementRegion, not a base VarRegion.
+    const MemRegion* BaseR =  
cast<loc::MemRegionVal>(BaseL).getRegion();
+
+    const ElementRegion* ElemR = cast<ElementRegion>(BaseR);
+
+    SVal Idx = ElemR->getIndex();
+
+    nonloc::ConcreteInt *CI1, *CI2;
+
+    // Only handle integer indices for now.
+    if ((CI1 = dyn_cast<nonloc::ConcreteInt>(&Idx)) &&
+        (CI2 = dyn_cast<nonloc::ConcreteInt>(&Offset))) {
+      SVal NewIdx = CI1->EvalBinOp(StateMgr.getBasicVals(),  
BinaryOperator::Add,
+                                   *CI2);
+      return loc::MemRegionVal(MRMgr.getElementRegion(NewIdx,
+                                                      ElemR- 
 >getSuperRegion()));
+    }
+    break;

This looks good assuming that the MemRegion for the base is an  
ElementRegion.

Incidentally, do you need a switch statement, or do we plan on adding  
support for bases other than MemRegion?