LValueElement returns UnknownVal for multi-dimensional arrays

Clang Frontend Static Analyzer
1

Hello everyone,

I just came across this implementation in Store.cpp, the purpose of which is not entirely clear to me.

SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
                                    SVal Base) {
  if (Offset.isZeroConstant()) {
    QualType BT = Base.getType(this->Ctx);
    if (!BT.isNull() && !elementType.isNull()) {
      QualType PointeeTy = BT->getPointeeType();
      if (!PointeeTy.isNull() &&
          PointeeTy.getCanonicalType() == elementType.getCanonicalType())
        return Base;
    }
  }

...
  if (!isa<nonloc::ConcreteInt>(Offset)) {
    if (isa<ElementRegion>(BaseRegion->StripCasts()))
      return UnknownVal();

    return loc::MemRegionVal(MRMgr.getElementRegion(
        elementType, Offset, cast<SubRegion>(ElemR->getSuperRegion()), Ctx));
  }
...
}

This function behaves as follows for these examples:

extern int unknown_index;
extern int matrix[3][3];

int main() {

  int val1 = matrix[0][unknown_index];
  int val2 = matrix[1][unknown_index];
  int val3 = matrix[unknown_index][unknown_index];

  return 0;
}

matrix[0][unknown_index]:

    1. dim: getLValueElement → LValue: SVal = &Element{matrix,0 S64b,int[3]}
    1. dim: getLValueElement → LValue: SVal = &Element{Element{matrix,0 S64b,int[3]},reg_$0,int}
  • RHS of the first assignment returns a meaningful MemRegionVal, since the evaluation of the first dimension returns the Base and therefore, the last return statement shown is taken

matrix[1][unknown_index]:

    1. dim: getLValueElement → LValue: SVal = &Element{matrix,1 S64b,int[3]}
    1. dim: getLValueElement → LValue: SVal = Unknown
  • RHS of the second assignment returns an Unknown since the BaseRegion is an ElementRegion itself. Therefore, the second last return statement in the function is taken.

matrix[unknown_index][unknown_index]:

    1. dim: getLValueElement → LValue: SVal = LValue: SVal = &Element{matrix,reg_$0,int[3]}
    1. dim: getLValueElement → LValue: SVal = Unknown
  • This results in the same paths in getLValueElement as in the case of the second assignment.

It is not clear to me what advantages this early return has for BaseRegions that are ElementRegions. If this were simply removed, meaningful MemRegionVals would be returned in all cases. Outlined here:

SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
                                    SVal Base) {
 ...
  if (!isa<nonloc::ConcreteInt>(Offset)) {
    // if (isa<ElementRegion>(BaseRegion->StripCasts()))
    //  return UnknownVal();

    return loc::MemRegionVal(MRMgr.getElementRegion(
        elementType, Offset, cast<SubRegion>(ElemR->getSuperRegion()), Ctx));
  }
...
}

matrix[0][unknown_index]:
→ LValue: SVal = &Element{Element{matrix,0 S64b,int[3]},reg_$0,int}

matrix[1][unknown_index]:
→ LValue: SVal = &Element{Element{matrix,1 S64b,int[3]},reg_$0,int}

matrix[unknown_index][unknown_index]:
→ LValue: SVal = &Element{Element{matrix,reg_$0,int[3]},reg_$0,int}

This proposed modification would have the advantage that possible callbacks such as checklocation will be triggered for the determined MemRegion and evaluated there (e.g. for array bound checks). At first glance, this solution seems very useful to me.
I would appreciate any feedback.

2

Hi @T-Gruber,
What you outlines makes sense at glance.
I’d suggest you propose a PR, and then we can see what impact this change would have.

Keep in mind that the region store grew organically, and aggregates a lot of legacy. Consequently, it’s fair to assume there is code that may not totally make sense by now.

1 Like
3

Thank you very much for your answer. That makes sense. I will take care of a PR.