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,11 +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, + // this value might be non-positive (probably caused by other bugs). + // At least check for zero again. + // Assume that this is a more exact fact than the previous assumptions + // (in checkVLAIndexSize), so report error too. + 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(); - assert(IndexL > 0 && "Index length should have been checked for zero."); if (KnownSize <= SizeMax / IndexL) { KnownSize *= IndexL; } else { @@ -166,35 +174,21 @@ 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; + // Check if the size is zero or negative. + SVal PositiveVal = SVB.evalBinOp(State, BO_GT, SizeD, Zero, SizeTy); + if (Optional PositiveDVal = PositiveVal.getAs()) { + ProgramStateRef StatePos, StateNotPos; - std::tie(StateNeg, StatePos) = CM.assumeDual(State, *LessThanZeroDVal); - if (StateNeg && !StatePos) { - reportBug(VLA_Negative, SizeE, State, C); + 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 @@ -137,3 +137,17 @@ clang_analyzer_eval(clang_analyzer_getExtent(&vla3m) == 2 * x * 4 * sizeof(int)); // expected-warning@-1{{TRUE}} } + +// https://bugs.llvm.org/show_bug.cgi?id=46128 +// Analyzer doesn't handle more than simple symbolic expressions correct. +// Just don't crash. +extern void foo(void); +int a; +void b() { + int c = a + 1; + for (;;) { + int d[c]; + for (; 0 < c;) + foo(); + } +} // no-crash