From: John McCall Date: Sun, 15 May 2011 07:14:44 +0000 (+0000) Subject: The array-size operand to a new-expression is not necessarily a size_t. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7d16627081caede9691a6f46b796da4073ac14ad;p=clang The array-size operand to a new-expression is not necessarily a size_t. It can be larger, it can be smaller, it can be signed, whatever. Handle all the crazy cases with grace and spirit. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@131378 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp index cef5d916ff..734449005f 100644 --- a/lib/CodeGen/CGExprCXX.cpp +++ b/lib/CodeGen/CGExprCXX.cpp @@ -476,172 +476,242 @@ static CharUnits CalculateCookiePadding(CodeGenFunction &CGF, return CGF.CGM.getCXXABI().GetArrayCookieSize(E); } -static llvm::Value *EmitCXXNewAllocSize(ASTContext &Context, - CodeGenFunction &CGF, - const CXXNewExpr *E, - llvm::Value *&NumElements, - llvm::Value *&SizeWithoutCookie) { - QualType ElemType = E->getAllocatedType(); - - const llvm::IntegerType *SizeTy = - cast(CGF.ConvertType(CGF.getContext().getSizeType())); - - CharUnits TypeSize = CGF.getContext().getTypeSizeInChars(ElemType); - - if (!E->isArray()) { - SizeWithoutCookie = llvm::ConstantInt::get(SizeTy, TypeSize.getQuantity()); - return SizeWithoutCookie; +static llvm::Value *EmitCXXNewAllocSize(CodeGenFunction &CGF, + const CXXNewExpr *e, + llvm::Value *&numElements, + llvm::Value *&sizeWithoutCookie) { + QualType type = e->getAllocatedType(); + + if (!e->isArray()) { + CharUnits typeSize = CGF.getContext().getTypeSizeInChars(type); + sizeWithoutCookie + = llvm::ConstantInt::get(CGF.SizeTy, typeSize.getQuantity()); + return sizeWithoutCookie; } + // The width of size_t. + unsigned sizeWidth = CGF.SizeTy->getBitWidth(); + // Figure out the cookie size. - CharUnits CookieSize = CalculateCookiePadding(CGF, E); + llvm::APInt cookieSize(sizeWidth, + CalculateCookiePadding(CGF, e).getQuantity()); // Emit the array size expression. // We multiply the size of all dimensions for NumElements. // e.g for 'int[2][3]', ElemType is 'int' and NumElements is 6. - NumElements = CGF.EmitScalarExpr(E->getArraySize()); - assert(NumElements->getType() == SizeTy && "element count not a size_t"); - - uint64_t ArraySizeMultiplier = 1; + numElements = CGF.EmitScalarExpr(e->getArraySize()); + assert(isa(numElements->getType())); + + // The number of elements can be have an arbitrary integer type; + // essentially, we need to multiply it by a constant factor, add a + // cookie size, and verify that the result is representable as a + // size_t. That's just a gloss, though, and it's wrong in one + // important way: if the count is negative, it's an error even if + // the cookie size would bring the total size >= 0. + bool isSigned = e->getArraySize()->getType()->isSignedIntegerType(); + const llvm::IntegerType *numElementsType + = cast(numElements->getType()); + unsigned numElementsWidth = numElementsType->getBitWidth(); + + // Compute the constant factor. + llvm::APInt arraySizeMultiplier(sizeWidth, 1); while (const ConstantArrayType *CAT - = CGF.getContext().getAsConstantArrayType(ElemType)) { - ElemType = CAT->getElementType(); - ArraySizeMultiplier *= CAT->getSize().getZExtValue(); + = CGF.getContext().getAsConstantArrayType(type)) { + type = CAT->getElementType(); + arraySizeMultiplier *= CAT->getSize(); } - llvm::Value *Size; + CharUnits typeSize = CGF.getContext().getTypeSizeInChars(type); + llvm::APInt typeSizeMultiplier(sizeWidth, typeSize.getQuantity()); + typeSizeMultiplier *= arraySizeMultiplier; + + // This will be a size_t. + llvm::Value *size; // If someone is doing 'new int[42]' there is no need to do a dynamic check. // Don't bloat the -O0 code. - if (llvm::ConstantInt *NumElementsC = - dyn_cast(NumElements)) { - llvm::APInt NEC = NumElementsC->getValue(); - unsigned SizeWidth = NEC.getBitWidth(); - - // Determine if there is an overflow here by doing an extended multiply. - NEC = NEC.zext(SizeWidth*2); - llvm::APInt SC(SizeWidth*2, TypeSize.getQuantity()); - SC *= NEC; - - if (!CookieSize.isZero()) { - // Save the current size without a cookie. We don't care if an - // overflow's already happened because SizeWithoutCookie isn't - // used if the allocator returns null or throws, as it should - // always do on an overflow. - llvm::APInt SWC = SC.trunc(SizeWidth); - SizeWithoutCookie = llvm::ConstantInt::get(SizeTy, SWC); - - // Add the cookie size. - SC += llvm::APInt(SizeWidth*2, CookieSize.getQuantity()); + if (llvm::ConstantInt *numElementsC = + dyn_cast(numElements)) { + const llvm::APInt &count = numElementsC->getValue(); + + bool hasAnyOverflow = false; + + // If 'count' was a negative number, it's an overflow. + if (isSigned && count.isNegative()) + hasAnyOverflow = true; + + // We want to do all this arithmetic in size_t. If numElements is + // wider than that, check whether it's already too big, and if so, + // overflow. + else if (numElementsWidth > sizeWidth && + numElementsWidth - sizeWidth > count.countLeadingZeros()) + hasAnyOverflow = true; + + // Okay, compute a count at the right width. + llvm::APInt adjustedCount = count.zextOrTrunc(sizeWidth); + + // Scale numElements by that. This might overflow, but we don't + // care because it only overflows if allocationSize does, too, and + // if that overflows then we shouldn't use this. + numElements = llvm::ConstantInt::get(CGF.SizeTy, + adjustedCount * arraySizeMultiplier); + + // Compute the size before cookie, and track whether it overflowed. + bool overflow; + llvm::APInt allocationSize + = adjustedCount.umul_ov(typeSizeMultiplier, overflow); + hasAnyOverflow |= overflow; + + // Add in the cookie, and check whether it's overflowed. + if (cookieSize != 0) { + // Save the current size without a cookie. This shouldn't be + // used if there was overflow. + sizeWithoutCookie = llvm::ConstantInt::get(CGF.SizeTy, allocationSize); + + allocationSize = allocationSize.uadd_ov(cookieSize, overflow); + hasAnyOverflow |= overflow; } - - if (SC.countLeadingZeros() >= SizeWidth) { - SC = SC.trunc(SizeWidth); - Size = llvm::ConstantInt::get(SizeTy, SC); + + // On overflow, produce a -1 so operator new will fail. + if (hasAnyOverflow) { + size = llvm::Constant::getAllOnesValue(CGF.SizeTy); } else { - // On overflow, produce a -1 so operator new throws. - Size = llvm::Constant::getAllOnesValue(SizeTy); + size = llvm::ConstantInt::get(CGF.SizeTy, allocationSize); } - // Scale NumElements while we're at it. - uint64_t N = NEC.getZExtValue() * ArraySizeMultiplier; - NumElements = llvm::ConstantInt::get(SizeTy, N); - - // Otherwise, we don't need to do an overflow-checked multiplication if - // we're multiplying by one. - } else if (TypeSize.isOne()) { - assert(ArraySizeMultiplier == 1); - - Size = NumElements; - - // If we need a cookie, add its size in with an overflow check. - // This is maybe a little paranoid. - if (!CookieSize.isZero()) { - SizeWithoutCookie = Size; + // Otherwise, we might need to use the overflow intrinsics. + } else { + // There are up to four conditions we need to test for: + // 1) if isSigned, we need to check whether numElements is negative; + // 2) if numElementsWidth > sizeWidth, we need to check whether + // numElements is larger than something representable in size_t; + // 3) we need to compute + // sizeWithoutCookie := numElements * typeSizeMultiplier + // and check whether it overflows; and + // 4) if we need a cookie, we need to compute + // size := sizeWithoutCookie + cookieSize + // and check whether it overflows. + + llvm::Value *hasOverflow = 0; + + // If numElementsWidth > sizeWidth, then one way or another, we're + // going to have to do a comparison for (2), and this happens to + // take care of (1), too. + if (numElementsWidth > sizeWidth) { + llvm::APInt threshold(numElementsWidth, 1); + threshold <<= sizeWidth; + + llvm::Value *thresholdV + = llvm::ConstantInt::get(numElementsType, threshold); + + hasOverflow = CGF.Builder.CreateICmpUGE(numElements, thresholdV); + numElements = CGF.Builder.CreateTrunc(numElements, CGF.SizeTy); + + // Otherwise, if we're signed, we want to sext up to size_t. + } else if (isSigned) { + if (numElementsWidth < sizeWidth) + numElements = CGF.Builder.CreateSExt(numElements, CGF.SizeTy); + + // If there's a non-1 type size multiplier, then we can do the + // signedness check at the same time as we do the multiply + // because a negative number times anything will cause an + // unsigned overflow. Otherwise, we have to do it here. + if (typeSizeMultiplier == 1) + hasOverflow = CGF.Builder.CreateICmpSLT(numElements, + llvm::ConstantInt::get(CGF.SizeTy, 0)); + + // Otherwise, zext up to size_t if necessary. + } else if (numElementsWidth < sizeWidth) { + numElements = CGF.Builder.CreateZExt(numElements, CGF.SizeTy); + } - llvm::Value *CookieSizeV - = llvm::ConstantInt::get(SizeTy, CookieSize.getQuantity()); + assert(numElements->getType() == CGF.SizeTy); - const llvm::Type *Types[] = { SizeTy }; - llvm::Value *UAddF - = CGF.CGM.getIntrinsic(llvm::Intrinsic::uadd_with_overflow, Types, 1); - llvm::Value *AddRes - = CGF.Builder.CreateCall2(UAddF, Size, CookieSizeV); + size = numElements; - Size = CGF.Builder.CreateExtractValue(AddRes, 0); - llvm::Value *DidOverflow = CGF.Builder.CreateExtractValue(AddRes, 1); - Size = CGF.Builder.CreateSelect(DidOverflow, - llvm::ConstantInt::get(SizeTy, -1), - Size); + // Multiply by the type size if necessary. This multiplier + // includes all the factors for nested arrays. + // + // This step also causes numElements to be scaled up by the + // nested-array factor if necessary. Overflow on this computation + // can be ignored because the result shouldn't be used if + // allocation fails. + if (typeSizeMultiplier != 1) { + const llvm::Type *intrinsicTypes[] = { CGF.SizeTy }; + llvm::Value *umul_with_overflow + = CGF.CGM.getIntrinsic(llvm::Intrinsic::umul_with_overflow, + intrinsicTypes, 1); + + llvm::Value *tsmV = + llvm::ConstantInt::get(CGF.SizeTy, typeSizeMultiplier); + llvm::Value *result = + CGF.Builder.CreateCall2(umul_with_overflow, size, tsmV); + + llvm::Value *overflowed = CGF.Builder.CreateExtractValue(result, 1); + if (hasOverflow) + hasOverflow = CGF.Builder.CreateOr(hasOverflow, overflowed); + else + hasOverflow = overflowed; + + size = CGF.Builder.CreateExtractValue(result, 0); + + // Also scale up numElements by the array size multiplier. + if (arraySizeMultiplier != 1) { + // If the base element type size is 1, then we can re-use the + // multiply we just did. + if (typeSize.isOne()) { + assert(arraySizeMultiplier == typeSizeMultiplier); + numElements = size; + + // Otherwise we need a separate multiply. + } else { + llvm::Value *asmV = + llvm::ConstantInt::get(CGF.SizeTy, arraySizeMultiplier); + numElements = CGF.Builder.CreateMul(numElements, asmV); + } + } + } else { + // numElements doesn't need to be scaled. + assert(arraySizeMultiplier == 1); } + + // Add in the cookie size if necessary. + if (cookieSize != 0) { + sizeWithoutCookie = size; + + const llvm::Type *intrinsicTypes[] = { CGF.SizeTy }; + llvm::Value *uadd_with_overflow + = CGF.CGM.getIntrinsic(llvm::Intrinsic::uadd_with_overflow, + intrinsicTypes, 1); + + llvm::Value *cookieSizeV = llvm::ConstantInt::get(CGF.SizeTy, cookieSize); + llvm::Value *result = + CGF.Builder.CreateCall2(uadd_with_overflow, size, cookieSizeV); + + llvm::Value *overflowed = CGF.Builder.CreateExtractValue(result, 1); + if (hasOverflow) + hasOverflow = CGF.Builder.CreateOr(hasOverflow, overflowed); + else + hasOverflow = overflowed; - // Otherwise use the int.umul.with.overflow intrinsic. - } else { - llvm::Value *OutermostElementSize - = llvm::ConstantInt::get(SizeTy, TypeSize.getQuantity()); - - llvm::Value *NumOutermostElements = NumElements; - - // Scale NumElements by the array size multiplier. This might - // overflow, but only if the multiplication below also overflows, - // in which case this multiplication isn't used. - if (ArraySizeMultiplier != 1) - NumElements = CGF.Builder.CreateMul(NumElements, - llvm::ConstantInt::get(SizeTy, ArraySizeMultiplier)); - - // The requested size of the outermost array is non-constant. - // Multiply that by the static size of the elements of that array; - // on unsigned overflow, set the size to -1 to trigger an - // exception from the allocation routine. This is sufficient to - // prevent buffer overruns from the allocator returning a - // seemingly valid pointer to insufficient space. This idea comes - // originally from MSVC, and GCC has an open bug requesting - // similar behavior: - // http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19351 - // - // This will not be sufficient for C++0x, which requires a - // specific exception class (std::bad_array_new_length). - // That will require ABI support that has not yet been specified. - const llvm::Type *Types[] = { SizeTy }; - llvm::Value *UMulF - = CGF.CGM.getIntrinsic(llvm::Intrinsic::umul_with_overflow, Types, 1); - llvm::Value *MulRes = CGF.Builder.CreateCall2(UMulF, NumOutermostElements, - OutermostElementSize); - - // The overflow bit. - llvm::Value *DidOverflow = CGF.Builder.CreateExtractValue(MulRes, 1); - - // The result of the multiplication. - Size = CGF.Builder.CreateExtractValue(MulRes, 0); - - // If we have a cookie, we need to add that size in, too. - if (!CookieSize.isZero()) { - SizeWithoutCookie = Size; - - llvm::Value *CookieSizeV - = llvm::ConstantInt::get(SizeTy, CookieSize.getQuantity()); - llvm::Value *UAddF - = CGF.CGM.getIntrinsic(llvm::Intrinsic::uadd_with_overflow, Types, 1); - llvm::Value *AddRes - = CGF.Builder.CreateCall2(UAddF, SizeWithoutCookie, CookieSizeV); - - Size = CGF.Builder.CreateExtractValue(AddRes, 0); - - llvm::Value *AddDidOverflow = CGF.Builder.CreateExtractValue(AddRes, 1); - DidOverflow = CGF.Builder.CreateOr(DidOverflow, AddDidOverflow); + size = CGF.Builder.CreateExtractValue(result, 0); } - Size = CGF.Builder.CreateSelect(DidOverflow, - llvm::ConstantInt::get(SizeTy, -1), - Size); + // If we had any possibility of dynamic overflow, make a select to + // overwrite 'size' with an all-ones value, which should cause + // operator new to throw. + if (hasOverflow) + size = CGF.Builder.CreateSelect(hasOverflow, + llvm::Constant::getAllOnesValue(CGF.SizeTy), + size); } - if (CookieSize.isZero()) - SizeWithoutCookie = Size; + if (cookieSize == 0) + sizeWithoutCookie = size; else - assert(SizeWithoutCookie && "didn't set SizeWithoutCookie?"); + assert(sizeWithoutCookie && "didn't set sizeWithoutCookie?"); - return Size; + return size; } static void StoreAnyExprIntoOneUnit(CodeGenFunction &CGF, const CXXNewExpr *E, @@ -969,8 +1039,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { llvm::Value *numElements = 0; llvm::Value *allocSizeWithoutCookie = 0; llvm::Value *allocSize = - EmitCXXNewAllocSize(getContext(), *this, E, numElements, - allocSizeWithoutCookie); + EmitCXXNewAllocSize(*this, E, numElements, allocSizeWithoutCookie); allocatorArgs.add(RValue::get(allocSize), sizeType); diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index f0faa8c29c..c82a9f9d00 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -951,8 +951,8 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal, } } - ArraySize = ImpCastExprToType(ArraySize, Context.getSizeType(), - CK_IntegralCast).take(); + // Note that we do *not* convert the argument in any way. It can + // be signed, larger than size_t, whatever. } FunctionDecl *OperatorNew = 0; diff --git a/test/CodeGenCXX/arm.cpp b/test/CodeGenCXX/arm.cpp index 8d74d003c3..f56b1552ce 100644 --- a/test/CodeGenCXX/arm.cpp +++ b/test/CodeGenCXX/arm.cpp @@ -136,8 +136,8 @@ namespace test3 { void d(int n) { // CHECK: define void @_ZN5test31dEi( // CHECK: [[N:%.*]] = load i32* - // CHECK: [[NE:%.*]] = mul i32 [[N]], 20 // CHECK: @llvm.umul.with.overflow.i32(i32 [[N]], i32 80) + // CHECK: [[NE:%.*]] = mul i32 [[N]], 20 // CHECK: @llvm.uadd.with.overflow.i32(i32 {{.*}}, i32 8) // CHECK: [[SZ:%.*]] = select // CHECK: call noalias i8* @_Znam(i32 [[SZ]]) @@ -208,8 +208,8 @@ namespace test4 { void d(int n) { // CHECK: define void @_ZN5test41dEi( // CHECK: [[N:%.*]] = load i32* - // CHECK: [[NE:%.*]] = mul i32 [[N]], 20 // CHECK: @llvm.umul.with.overflow.i32(i32 [[N]], i32 80) + // CHECK: [[NE:%.*]] = mul i32 [[N]], 20 // CHECK: @llvm.uadd.with.overflow.i32(i32 {{.*}}, i32 8) // CHECK: [[SZ:%.*]] = select // CHECK: call noalias i8* @_Znam(i32 [[SZ]])