diff --git a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp @@ -122,16 +122,19 @@ return State; if (const llvm::APSInt *IndexLVal = SVB.getKnownValue(State, IndexLength)) { + uint64_t IndexL = IndexLVal->getZExtValue(); + if (IndexL == 0) { + // Despite the previous assumption for positive size, value returned by + // getKnownValue could be zero. Double-check for zero here. + // See https://reviews.llvm.org/D80903 for discussion of this problem. + // This is just a safety check, the case could not be reproduced with + // the current code. + reportBug(VLA_Zero, SizeE, State, C); + return nullptr; + } // Check if the array size will overflow. // Size overflow check does not work with symbolic expressions because a // overflow situation can not be detected easily. - uint64_t IndexL = IndexLVal->getZExtValue(); - // FIXME: See https://reviews.llvm.org/D80903 for discussion of - // some difference in assume and getKnownValue that leads to - // unexpected behavior. Just bail on IndexL == 0 at this point. - if (IndexL == 0) - return nullptr; - if (KnownSize <= SizeMax / IndexL) { KnownSize *= IndexL; } else { @@ -171,35 +174,22 @@ return nullptr; } - // Check if the size is zero. + QualType SizeTy = SizeE->getType(); DefinedSVal SizeD = SizeV.castAs(); - - ProgramStateRef StateNotZero, StateZero; - std::tie(StateNotZero, StateZero) = State->assume(SizeD); - - if (StateZero && !StateNotZero) { - reportBug(VLA_Zero, SizeE, StateZero, C); - return nullptr; - } - - // From this point on, assume that the size is not zero. - State = StateNotZero; - - // Check if the size is negative. SValBuilder &SVB = C.getSValBuilder(); - - QualType SizeTy = SizeE->getType(); DefinedOrUnknownSVal Zero = SVB.makeZeroVal(SizeTy); - SVal LessThanZeroVal = SVB.evalBinOp(State, BO_LT, SizeD, Zero, SizeTy); - if (Optional LessThanZeroDVal = - LessThanZeroVal.getAs()) { - ConstraintManager &CM = C.getConstraintManager(); - ProgramStateRef StatePos, StateNeg; - - std::tie(StateNeg, StatePos) = CM.assumeDual(State, *LessThanZeroDVal); - if (StateNeg && !StatePos) { - reportBug(VLA_Negative, SizeE, State, C); + // Check if the size is zero or negative. + SVal PositiveVal = + SVB.evalBinOp(State, BO_GT, SizeD, Zero, SVB.getConditionType()); + if (Optional PositiveDVal = PositiveVal.getAs()) { + ProgramStateRef StatePos, StateNotPos; + + std::tie(StatePos, StateNotPos) = State->assume(*PositiveDVal); + if (StateNotPos && !StatePos) { + ConditionTruthVal IsZeroSize = StateNotPos->isNull(SizeD); + reportBug(IsZeroSize.isConstrainedTrue() ? VLA_Zero : VLA_Negative, SizeE, + State, C); return nullptr; } State = StatePos; diff --git a/clang/test/Analysis/vla.c b/clang/test/Analysis/vla.c --- a/clang/test/Analysis/vla.c +++ b/clang/test/Analysis/vla.c @@ -139,7 +139,7 @@ } // https://bugs.llvm.org/show_bug.cgi?id=46128 -// analyzer doesn't handle more than simple symbolic expressions. +// Analyzer doesn't handle more than simple symbolic expressions correct. // Just don't crash. extern void foo(void); int a;