[PATCH] D30489: [analyzer] catch out of bounds for VLA

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added a comment.



> Also, in your state dumps no information is actually lost. The fact that the value of variable sz is reg_$0<sz> is trivial: you could ask the Store what's the value of the variable sz and it'd say reg_$0<sz> if there are no bindings over it.

Thanks. I have tried to dig around in the framework. I don't see how I can get it.

I also don't want to get the "current" symbolvalue for the variable. For example:

  void f(int X) {
    int Buf[X];
    X--;
    Buf[X] = 0;
  }

So I envision that I need to tell the Store which "X" I want. I am trying to get the "X" symbol that defines the VLA size.

> If no, an alternative approach would be to properly set the constraints on the extent of the VLA's memory region.

For information I tried to set the extent for the VLA.. by copy/pasting some code from the other diff. Ideally I don't think it should be set in checkPreStmt but this was an experiment...

  void ArrayBoundCheckerV2::checkPreStmt(const ArraySubscriptExpr *A, CheckerContext &C) const
  {
    ASTContext &Ctx = C.getASTContext();
    QualType T = A->getBase()->IgnoreCasts()->getType();
    const VariableArrayType *VLA = Ctx.getAsVariableArrayType(T);
    if (!VLA)
      return;
  
    ProgramStateRef State = C.getState();
  
    SVal ElementCount = State->getSVal(VLA->getSizeExpr(), C.getLocationContext());
    QualType ElementType = VLA->getElementType();
    ASTContext &AstContext = C.getASTContext();
    CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType);
  
    if (Optional<DefinedOrUnknownSVal> DefinedSize =
      ElementCount.getAs<DefinedOrUnknownSVal>()) {
      SVal AV = State->getSVal(A->getBase(), C.getLocationContext());
      const SubRegion *Region = AV.getAsRegion()->getAs<SubRegion>();
      SValBuilder &svalBuilder = C.getSValBuilder();
      DefinedOrUnknownSVal Extent = Region->getExtent(svalBuilder);
      // size in Bytes = ElementCount*TypeSize
      SVal SizeInBytes = svalBuilder.evalBinOpNN(
        State, BO_Mul, ElementCount.castAs<NonLoc>(),
        svalBuilder.makeArrayIndex(TypeSize.getQuantity()),
        svalBuilder.getArrayIndexType());
      DefinedOrUnknownSVal extentMatchesSize = svalBuilder.evalEQ(
        State, Extent, SizeInBytes.castAs<DefinedOrUnknownSVal>());
      State = State->assume(extentMatchesSize, true);
      C.addTransition(State);
    }
  }

After this the ArrayBoundCheckerV2 still doesn't catch the bugs. As far as I could see that made very little difference. To me the extentValue looks the same.

> If not it's possible that we are hitting the Constraint Solver limitations.

Yes. I was hoping that I would be able to update SimpleSValBuilder::evalBinOpNN() somehow. And first I wanted to see if I could do it in the checker where all the CheckerContext and stuff is available.


Repository:
  rL LLVM

https://reviews.llvm.org/D30489