From: Douglas Gregor Date: Wed, 18 Aug 2010 00:39:00 +0000 (+0000) Subject: Emit an error if an array is too large. We're slightly more strict X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2767ce2e21d8bc17869b8436220bce719b3369e4;p=clang Emit an error if an array is too large. We're slightly more strict than GCC 4.2 here when building 32-bit (where GCC will allow allocation of an array for which we can't get a valid past-the-end pointer), and emulate its odd behavior in 64-bit where it only allows 63 bits worth of storage in the array. The former is a correctness issue; the latter is harmless in practice (you wouldn't be able to use such an array anyway) and helps us pass a GCC DejaGNU test. Fixes . git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@111338 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h index 7a522f42fa..4a9f10a870 100644 --- a/include/clang/AST/Type.h +++ b/include/clang/AST/Type.h @@ -1458,6 +1458,17 @@ public: bool isSugared() const { return false; } QualType desugar() const { return QualType(this, 0); } + + /// \brief Determine the number of bits required to address a member of + // an array with the given element type and number of elements. + static unsigned getNumAddressingBits(ASTContext &Context, + QualType ElementType, + const llvm::APInt &NumElements); + + /// \brief Determine the maximum number of active bits that an array's size + /// can require, which limits the maximum size of the array. + static unsigned getMaxSizeBits(ASTContext &Context); + void Profile(llvm::FoldingSetNodeID &ID) { Profile(ID, getElementType(), getSize(), getSizeModifier(), getIndexTypeCVRQualifiers()); diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index f20c199246..b333a833b0 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1797,6 +1797,8 @@ def err_typecheck_field_variable_size : Error< "extension will never be supported">; def err_vm_func_decl : Error< "function declaration cannot have variably modified type">; +def err_array_too_large : Error< + "array is too large (%0 elements)">; def err_typecheck_negative_array_size : Error<"array size is negative">; def warn_typecheck_function_qualifiers : Warning< diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 31af6fb661..4ab463699c 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "clang/AST/ASTContext.h" +#include "clang/AST/CharUnits.h" #include "clang/AST/Type.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" @@ -21,6 +22,7 @@ #include "clang/Basic/Specifiers.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/raw_ostream.h" +#include using namespace clang; bool QualType::isConstant(QualType T, ASTContext &Ctx) { @@ -35,6 +37,32 @@ bool QualType::isConstant(QualType T, ASTContext &Ctx) { Type::~Type() { } +unsigned ConstantArrayType::getNumAddressingBits(ASTContext &Context, + QualType ElementType, + const llvm::APInt &NumElements) { + llvm::APSInt SizeExtended(NumElements, true); + unsigned SizeTypeBits = Context.getTypeSize(Context.getSizeType()); + SizeExtended.extend(std::max(SizeTypeBits, SizeExtended.getBitWidth()) * 2); + + uint64_t ElementSize + = Context.getTypeSizeInChars(ElementType).getQuantity(); + llvm::APSInt TotalSize(llvm::APInt(SizeExtended.getBitWidth(), ElementSize)); + TotalSize *= SizeExtended; + + return TotalSize.getActiveBits(); +} + +unsigned ConstantArrayType::getMaxSizeBits(ASTContext &Context) { + unsigned Bits = Context.getTypeSize(Context.getSizeType()); + + // GCC appears to only allow 63 bits worth of address space when compiling + // for 64-bit, so we do the same. + if (Bits == 64) + --Bits; + + return Bits; +} + void DependentSizedArrayType::Profile(llvm::FoldingSetNodeID &ID, ASTContext &Context, QualType ET, diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 550dcb8aeb..dfbc7e1559 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -2364,20 +2364,26 @@ Sema::HandleDeclarator(Scope *S, Declarator &D, /// be errors (for GCC compatibility). static QualType TryToFixInvalidVariablyModifiedType(QualType T, ASTContext &Context, - bool &SizeIsNegative) { + bool &SizeIsNegative, + llvm::APSInt &Oversized) { // This method tries to turn a variable array into a constant // array even when the size isn't an ICE. This is necessary // for compatibility with code that depends on gcc's buggy // constant expression folding, like struct {char x[(int)(char*)2];} SizeIsNegative = false; - + Oversized = 0; + + if (T->isDependentType()) + return QualType(); + QualifierCollector Qs; const Type *Ty = Qs.strip(T); if (const PointerType* PTy = dyn_cast(Ty)) { QualType Pointee = PTy->getPointeeType(); QualType FixedType = - TryToFixInvalidVariablyModifiedType(Pointee, Context, SizeIsNegative); + TryToFixInvalidVariablyModifiedType(Pointee, Context, SizeIsNegative, + Oversized); if (FixedType.isNull()) return FixedType; FixedType = Context.getPointerType(FixedType); return Qs.apply(FixedType); @@ -2396,15 +2402,24 @@ static QualType TryToFixInvalidVariablyModifiedType(QualType T, !EvalResult.Val.isInt()) return QualType(); + // Check whether the array size is negative. llvm::APSInt &Res = EvalResult.Val.getInt(); - if (Res >= llvm::APSInt(Res.getBitWidth(), Res.isUnsigned())) { - // TODO: preserve the size expression in declarator info - return Context.getConstantArrayType(VLATy->getElementType(), - Res, ArrayType::Normal, 0); + if (Res.isSigned() && Res.isNegative()) { + SizeIsNegative = true; + return QualType(); } - SizeIsNegative = true; - return QualType(); + // Check whether the array is too large to be addressed. + unsigned ActiveSizeBits + = ConstantArrayType::getNumAddressingBits(Context, VLATy->getElementType(), + Res); + if (ActiveSizeBits > ConstantArrayType::getMaxSizeBits(Context)) { + Oversized = Res; + return QualType(); + } + + return Context.getConstantArrayType(VLATy->getElementType(), + Res, ArrayType::Normal, 0); } /// \brief Register the given locally-scoped external C declaration so @@ -2501,8 +2516,10 @@ Sema::ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC, if (S->getFnParent() == 0) { bool SizeIsNegative; + llvm::APSInt Oversized; QualType FixedTy = - TryToFixInvalidVariablyModifiedType(T, Context, SizeIsNegative); + TryToFixInvalidVariablyModifiedType(T, Context, SizeIsNegative, + Oversized); if (!FixedTy.isNull()) { Diag(D.getIdentifierLoc(), diag::warn_illegal_constant_array_size); NewTD->setTypeSourceInfo(Context.getTrivialTypeSourceInfo(FixedTy)); @@ -2511,6 +2528,9 @@ Sema::ActOnTypedefDeclarator(Scope* S, Declarator& D, DeclContext* DC, Diag(D.getIdentifierLoc(), diag::err_typecheck_negative_array_size); else if (T->isVariableArrayType()) Diag(D.getIdentifierLoc(), diag::err_vla_decl_in_file_scope); + else if (Oversized.getBoolValue()) + Diag(D.getIdentifierLoc(), diag::err_array_too_large) + << Oversized.toString(10); else Diag(D.getIdentifierLoc(), diag::err_vm_decl_in_file_scope); NewTD->setInvalidDecl(); @@ -2931,8 +2951,10 @@ void Sema::CheckVariableDeclaration(VarDecl *NewVD, if ((isVM && NewVD->hasLinkage()) || (T->isVariableArrayType() && NewVD->hasGlobalStorage())) { bool SizeIsNegative; + llvm::APSInt Oversized; QualType FixedTy = - TryToFixInvalidVariablyModifiedType(T, Context, SizeIsNegative); + TryToFixInvalidVariablyModifiedType(T, Context, SizeIsNegative, + Oversized); if (FixedTy.isNull() && T->isVariableArrayType()) { const VariableArrayType *VAT = Context.getAsVariableArrayType(T); @@ -5965,14 +5987,19 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, QualType T, // than a variably modified type. if (!InvalidDecl && T->isVariablyModifiedType()) { bool SizeIsNegative; + llvm::APSInt Oversized; QualType FixedTy = TryToFixInvalidVariablyModifiedType(T, Context, - SizeIsNegative); + SizeIsNegative, + Oversized); if (!FixedTy.isNull()) { Diag(Loc, diag::warn_illegal_constant_array_size); T = FixedTy; } else { if (SizeIsNegative) Diag(Loc, diag::err_typecheck_negative_array_size); + else if (Oversized.getBoolValue()) + Diag(Loc, diag::err_array_too_large) + << Oversized.toString(10); else Diag(Loc, diag::err_typecheck_field_variable_size); InvalidDecl = true; diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index 35e679f53e..5e46090c05 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -716,8 +716,20 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal, llvm::APInt::getNullValue(Value.getBitWidth()), Value.isUnsigned())) return ExprError(Diag(ArraySize->getSourceRange().getBegin(), - diag::err_typecheck_negative_array_size) + diag::err_typecheck_negative_array_size) << ArraySize->getSourceRange()); + + if (!AllocType->isDependentType()) { + unsigned ActiveSizeBits + = ConstantArrayType::getNumAddressingBits(Context, AllocType, Value); + if (ActiveSizeBits > ConstantArrayType::getMaxSizeBits(Context)) { + Diag(ArraySize->getSourceRange().getBegin(), + diag::err_array_too_large) + << Value.toString(10) + << ArraySize->getSourceRange(); + return ExprError(); + } + } } else if (TypeIdParens.isValid()) { // Can't have dynamic array size when the type-id is in parentheses. Diag(ArraySize->getLocStart(), diag::ext_new_paren_array_nonconst) diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 6df16bd300..accd7e63ed 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -677,7 +677,7 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM, << ArraySize->getType() << ArraySize->getSourceRange(); return QualType(); } - llvm::APSInt ConstVal(32); + llvm::APSInt ConstVal(Context.getTypeSize(Context.getSizeType())); if (!ArraySize) { if (ASM == ArrayType::Star) T = Context.getVariableArrayType(T, 0, ASM, Quals, Brackets); @@ -707,7 +707,17 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM, isSFINAEContext()? diag::err_typecheck_zero_array_size : diag::ext_typecheck_zero_array_size) << ArraySize->getSourceRange(); + } else if (!T->isDependentType() && !T->isVariablyModifiedType() && + !T->isIncompleteType()) { + // Is the array too large? + unsigned ActiveSizeBits + = ConstantArrayType::getNumAddressingBits(Context, T, ConstVal); + if (ActiveSizeBits > ConstantArrayType::getMaxSizeBits(Context)) + Diag(ArraySize->getLocStart(), diag::err_array_too_large) + << ConstVal.toString(10) + << ArraySize->getSourceRange(); } + T = Context.getConstantArrayType(T, ConstVal, ASM, Quals); } // If this is not C99, extwarn about VLA's and C99 array size modifiers. diff --git a/test/Sema/array-size-64.c b/test/Sema/array-size-64.c new file mode 100644 index 0000000000..f22e8e77d2 --- /dev/null +++ b/test/Sema/array-size-64.c @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin -verify %s + +void f() { + int a[2147483647U][2147483647U]; // expected-error{{array is too large}} + int b[1073741825U - 1U][2147483647U]; + int c[18446744073709551615U/sizeof(int)/2]; +} diff --git a/test/Sema/array-size.c b/test/Sema/array-size.c new file mode 100644 index 0000000000..7580e3ecc5 --- /dev/null +++ b/test/Sema/array-size.c @@ -0,0 +1,10 @@ +// RUN: %clang_cc1 -triple i686-apple-darwin -verify %s + +void f() { + int x0[1073741824]; // expected-error{{array is too large}} + int x1[1073741824 + 1]; // expected-error{{array is too large}} + int x2[(unsigned)1073741824]; // expected-error{{array is too large}} + int x3[(unsigned)1073741824 + 1]; // expected-error{{array is too large}} + int x4[1073741824 - 1]; +} +