xazax.hun added a comment.
In https://reviews.llvm.org/D30489#689947, @zaks.anna wrote:
> Should we just remove ArrayBoundChecker.cpp or is there a value in keeping it
> around?
I think once ArrayBoundCheckerV2 is in production we should only keep
ArrayBoundChecker there as educational purpuses for now. However, in the
future, as the constraint manager and the core analyzer gets smarter, the V2
checker might get more and more similar to the original one. Once this patch is
accepted, I might run the analyzer with V2 check enabled on some open source
projects. It would be great to have it moved out of alpha soon.
>
>
>> 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);
> }
> }
You probably get an Unknown value for `State->getSVal(VLA->getSizeExpr(),
C.getLocationContext());`, because the sizeExpr is not in the environment, it
is already evaluated, its value has been used up, it is no longer needed. So in
case the correct constraints are missing from the VLAs, you should add these
assumptions at a point where this value is guaranteed to be available, probably
right after the evaluation of the declaration.
Repository:
rL LLVM
https://reviews.llvm.org/D30489
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
