[cfe-commits] r131378 - in /cfe/trunk: lib/CodeGen/CGExprCXX.cpp lib/Sema/SemaExprCXX.cpp test/CodeGenCXX/arm.cpp
John McCall
rjmccall at apple.com
Sun May 15 00:14:44 PDT 2011
Author: rjmccall
Date: Sun May 15 02:14:44 2011
New Revision: 131378
URL: http://llvm.org/viewvc/llvm-project?rev=131378&view=rev
Log:
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.
Modified:
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/CodeGenCXX/arm.cpp
Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=131378&r1=131377&r2=131378&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Sun May 15 02:14:44 2011
@@ -476,172 +476,242 @@
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<llvm::IntegerType>(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");
+ numElements = CGF.EmitScalarExpr(e->getArraySize());
+ assert(isa<llvm::IntegerType>(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<llvm::IntegerType>(numElements->getType());
+ unsigned numElementsWidth = numElementsType->getBitWidth();
- uint64_t ArraySizeMultiplier = 1;
+ // 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<llvm::ConstantInt>(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);
+ if (llvm::ConstantInt *numElementsC =
+ dyn_cast<llvm::ConstantInt>(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);
- // Add the cookie size.
- SC += llvm::APInt(SizeWidth*2, CookieSize.getQuantity());
+ 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;
-
- llvm::Value *CookieSizeV
- = llvm::ConstantInt::get(SizeTy, CookieSize.getQuantity());
-
- 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 = CGF.Builder.CreateExtractValue(AddRes, 0);
- llvm::Value *DidOverflow = CGF.Builder.CreateExtractValue(AddRes, 1);
- Size = CGF.Builder.CreateSelect(DidOverflow,
- llvm::ConstantInt::get(SizeTy, -1),
- 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);
}
- // Otherwise use the int.umul.with.overflow intrinsic.
- } else {
- llvm::Value *OutermostElementSize
- = llvm::ConstantInt::get(SizeTy, TypeSize.getQuantity());
+ assert(numElements->getType() == CGF.SizeTy);
- llvm::Value *NumOutermostElements = NumElements;
+ size = 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
+ // Multiply by the type size if necessary. This multiplier
+ // includes all the factors for nested arrays.
//
- // 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);
+ // 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;
- 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 *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);
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=131378&r1=131377&r2=131378&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Sun May 15 02:14:44 2011
@@ -951,8 +951,8 @@
}
}
- 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;
Modified: cfe/trunk/test/CodeGenCXX/arm.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/arm.cpp?rev=131378&r1=131377&r2=131378&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/arm.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/arm.cpp Sun May 15 02:14:44 2011
@@ -136,8 +136,8 @@
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 @@
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]])
More information about the cfe-commits
mailing list