balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso,
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
Variable-length array (VLA) should have a size that fits into
a size_t value. At least if the size is queried with sizeof,
but it is better (and more simple) to check it always.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79330
Files:
clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
clang/test/Analysis/vla.c
Index: clang/test/Analysis/vla.c
===================================================================
--- clang/test/Analysis/vla.c
+++ clang/test/Analysis/vla.c
@@ -100,6 +100,21 @@
return s;
}
+#define BIGINDEX 65535U
+
+size_t check_VLA_overflow_sizeof(unsigned int x) {
+ if (x == BIGINDEX) {
+ size_t s = sizeof(int[x][x][x][x]); // expected-warning{{Declared variable-length array (VLA) has too large size}}
+ return s;
+ }
+ return 0;
+}
+
+void check_VLA_overflow_typedef() {
+ unsigned int x = BIGINDEX;
+ typedef int VLA[x][x][x][x]; // expected-warning{{Declared variable-length array (VLA) has too large size}}
+}
+
// Multi-dimensional arrays.
void check_zero_sized_VLA_multi1(int x) {
Index: clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
@@ -34,14 +34,18 @@
: public Checker<check::PreStmt<DeclStmt>,
check::PreStmt<UnaryExprOrTypeTraitExpr>> {
mutable std::unique_ptr<BugType> BT;
- enum VLASize_Kind { VLA_Garbage, VLA_Zero, VLA_Tainted, VLA_Negative };
+ enum VLASize_Kind {
+ VLA_Garbage,
+ VLA_Zero,
+ VLA_Tainted,
+ VLA_Negative,
+ VLA_Overflow
+ };
ProgramStateRef checkVLA(CheckerContext &C, ProgramStateRef State,
- const VariableArrayType *VLA,
- const VariableArrayType *&VLALast,
- llvm::SmallVector<const Expr *, 2> &VLASizes) const;
- ProgramStateRef checkVLASize(CheckerContext &C, ProgramStateRef State,
- const Expr *SizeE) const;
+ const VariableArrayType *VLA, SVal &ArraySize) const;
+ ProgramStateRef checkVLAIndexSize(CheckerContext &C, ProgramStateRef State,
+ const Expr *SizeE) const;
void reportBug(VLASize_Kind Kind, const Expr *SizeE, ProgramStateRef State,
CheckerContext &C,
@@ -54,14 +58,15 @@
};
} // end anonymous namespace
-ProgramStateRef
-VLASizeChecker::checkVLA(CheckerContext &C, ProgramStateRef State,
- const VariableArrayType *VLA,
- const VariableArrayType *&VLALast,
- llvm::SmallVector<const Expr *, 2> &VLASizes) const {
+ProgramStateRef VLASizeChecker::checkVLA(CheckerContext &C,
+ ProgramStateRef State,
+ const VariableArrayType *VLA,
+ SVal &ArraySize) const {
assert(VLA && "Function should be called with non-null VLA argument.");
- VLALast = nullptr;
+ const VariableArrayType *VLALast = nullptr;
+ llvm::SmallVector<const Expr *, 2> VLASizes;
+
// Walk over the VLAs for every dimension until a non-VLA is found.
// Collect the sizes in VLASizes, put the most inner VLA to `VLALast`.
// In "vla[x][2][y][3]" this will be the array for index "y".
@@ -69,7 +74,7 @@
// until a non-vla is found.
while (VLA) {
const Expr *SizeE = VLA->getSizeExpr();
- State = checkVLASize(C, State, SizeE);
+ State = checkVLAIndexSize(C, State, SizeE);
if (!State)
return nullptr;
VLASizes.push_back(SizeE);
@@ -79,12 +84,61 @@
assert(VLALast &&
"Array should have at least one variably-modified dimension.");
+ ASTContext &Ctx = C.getASTContext();
+ SValBuilder &SVB = C.getSValBuilder();
+ CanQualType SizeTy = Ctx.getSizeType();
+ uint64_t SizeMax =
+ SVB.getBasicValueFactory().getMaxValue(SizeTy).getZExtValue();
+
+ // Get the element size.
+ CharUnits EleSize = Ctx.getTypeSizeInChars(VLALast->getElementType());
+ NonLoc ArrSize =
+ SVB.makeIntVal(EleSize.getQuantity(), SizeTy).castAs<NonLoc>();
+
+ // Try to calculate the known real size of the array in KnownSize.
+ uint64_t KnownSize = 0;
+ if (const llvm::APSInt *KV = SVB.getKnownValue(State, ArrSize))
+ KnownSize = KV->getZExtValue();
+
+ for (const Expr *SizeE : VLASizes) {
+ auto SizeD = C.getSVal(SizeE).castAs<DefinedSVal>();
+ // Convert the array length to size_t.
+ NonLoc IndexLength =
+ SVB.evalCast(SizeD, SizeTy, SizeE->getType()).castAs<NonLoc>();
+ // Multiply the array length by the element size.
+ SVal Mul = SVB.evalBinOpNN(State, BO_Mul, ArrSize, IndexLength, SizeTy);
+ if (auto MulNonLoc = Mul.getAs<NonLoc>())
+ ArrSize = *MulNonLoc;
+ else
+ // Extent could not be determined.
+ return State;
+
+ if (const llvm::APSInt *IndexLVal = SVB.getKnownValue(State, IndexLength)) {
+ // 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 {
+ // Array size does not fit into size_t.
+ reportBug(VLA_Overflow, SizeE, State, C);
+ return nullptr;
+ }
+ } else {
+ KnownSize = 0;
+ }
+ }
+
+ ArraySize = ArrSize;
+
return State;
}
-ProgramStateRef VLASizeChecker::checkVLASize(CheckerContext &C,
- ProgramStateRef State,
- const Expr *SizeE) const {
+ProgramStateRef VLASizeChecker::checkVLAIndexSize(CheckerContext &C,
+ ProgramStateRef State,
+ const Expr *SizeE) const {
SVal SizeV = C.getSVal(SizeE);
if (SizeV.isUndef()) {
@@ -132,7 +186,7 @@
std::tie(StateNeg, StatePos) = CM.assumeDual(State, *LessThanZeroDVal);
if (StateNeg && !StatePos) {
- reportBug(VLA_Negative, SizeE, State, C); // FIXME: StateNeg ?
+ reportBug(VLA_Negative, SizeE, State, C);
return nullptr;
}
State = StatePos;
@@ -169,6 +223,9 @@
case VLA_Negative:
os << "has negative size";
break;
+ case VLA_Overflow:
+ os << "has too large size";
+ break;
}
auto report = std::make_unique<PathSensitiveBugReport>(*BT, os.str(), N);
@@ -201,53 +258,36 @@
return;
// Check the VLA sizes for validity.
- llvm::SmallVector<const Expr *, 2> VLASizes;
- const VariableArrayType *VLALast = nullptr;
- State = checkVLA(C, State, VLA, VLALast, VLASizes);
+ SVal ArraySize;
+
+ State = checkVLA(C, State, VLA, ArraySize);
if (!State)
return;
- if (!VD)
+ auto ArraySizeNL = ArraySize.getAs<NonLoc>();
+ if (!ArraySizeNL) {
+ // Array size could not be determined but state may contain new assumptions.
+ C.addTransition(State);
return;
+ }
// VLASizeChecker is responsible for defining the extent of the array being
// declared. We do this by multiplying the array length by the element size,
// then matching that with the array region's extent symbol.
- CanQualType SizeTy = Ctx.getSizeType();
- // Get the element size.
- CharUnits EleSize = Ctx.getTypeSizeInChars(VLALast->getElementType());
- NonLoc ArraySize =
- SVB.makeIntVal(EleSize.getQuantity(), SizeTy).castAs<NonLoc>();
+ if (VD) {
+ // Assume that the array's size matches the region size.
+ const LocationContext *LC = C.getLocationContext();
+ DefinedOrUnknownSVal DynSize =
+ getDynamicSize(State, State->getRegion(VD, LC), SVB);
- for (const Expr *SizeE : VLASizes) {
- auto SizeD = C.getSVal(SizeE).castAs<DefinedSVal>();
- // Convert the array length to size_t.
- NonLoc IndexLength =
- SVB.evalCast(SizeD, SizeTy, SizeE->getType()).castAs<NonLoc>();
- // Multiply the array length by the element size.
- SVal Mul = SVB.evalBinOpNN(State, BO_Mul, ArraySize, IndexLength, SizeTy);
- if (auto MulNonLoc = Mul.getAs<NonLoc>()) {
- ArraySize = *MulNonLoc;
- } else {
- // Extent could not be determined.
- // The state was probably still updated by the validation checks.
- C.addTransition(State);
- return;
- }
- }
+ DefinedOrUnknownSVal SizeIsKnown = SVB.evalEQ(State, DynSize, *ArraySizeNL);
+ State = State->assume(SizeIsKnown, true);
- // Finally, assume that the array's size matches the given size.
- const LocationContext *LC = C.getLocationContext();
- DefinedOrUnknownSVal DynSize =
- getDynamicSize(State, State->getRegion(VD, LC), SVB);
-
- DefinedOrUnknownSVal SizeIsKnown = SVB.evalEQ(State, DynSize, ArraySize);
- State = State->assume(SizeIsKnown, true);
-
- // Assume should not fail at this point.
- assert(State);
+ // Assume should not fail at this point.
+ assert(State);
+ }
// Remember our assumptions!
C.addTransition(State);
@@ -263,17 +303,15 @@
if (!UETTE->isArgumentType())
return;
- const VariableArrayType *VLA =
- C.getASTContext().getAsVariableArrayType(UETTE->getTypeOfArgument());
+ const VariableArrayType *VLA = C.getASTContext().getAsVariableArrayType(
+ UETTE->getTypeOfArgument().getCanonicalType());
// Ensure that the type is a VLA.
if (!VLA)
return;
ProgramStateRef State = C.getState();
-
- llvm::SmallVector<const Expr *, 2> VLASizes;
- const VariableArrayType *VLALast = nullptr;
- State = checkVLA(C, State, VLA, VLALast, VLASizes);
+ SVal ArraySize;
+ State = checkVLA(C, State, VLA, ArraySize);
if (!State)
return;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
