r216675 - Throw a std::bad_array_new_length exception when the expression (or constant-expression) passed to operator new[] results in overflow in conformance with [expr.new]p7. Fixes PR11644.
Richard Smith
richard at metafoo.co.uk
Thu Aug 28 14:34:53 PDT 2014
On Thu, Aug 28, 2014 at 9:48 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:
> Author: aaronballman
> Date: Thu Aug 28 11:48:44 2014
> New Revision: 216675
>
> URL: http://llvm.org/viewvc/llvm-project?rev=216675&view=rev
> Log:
> Throw a std::bad_array_new_length exception when the expression (or
> constant-expression) passed to operator new[] results in overflow in
> conformance with [expr.new]p7. Fixes PR11644.
>
This is certainly an improvement, but we're still not implementing
[expr.new]p7 correctly. Here are some cases we get wrong:
// We convert this to -1 rather than throwing.
int k = 0x80000000; char *p = new char[k];
// We convert these to -1 rather than rejecting with an error.
char *p = new char[-1];
char *q = new char[1]{1, 2};
// We reject this at compile time even though it is valid.
int k = 10; char *p = new char[k]{"foobar"};
Modified:
> cfe/trunk/lib/CodeGen/CGCXXABI.cpp
> cfe/trunk/lib/CodeGen/CGCXXABI.h
> cfe/trunk/lib/CodeGen/CGExprCXX.cpp
> cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> cfe/trunk/test/CodeGenCXX/new-array-init.cpp
> cfe/trunk/test/CodeGenCXX/operator-new.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGCXXABI.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.cpp?rev=216675&r1=216674&r2=216675&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGCXXABI.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGCXXABI.cpp Thu Aug 28 11:48:44 2014
> @@ -325,3 +325,12 @@ LValue CGCXXABI::EmitThreadLocalVarDeclL
> bool CGCXXABI::NeedsVTTParameter(GlobalDecl GD) {
> return false;
> }
> +
> +llvm::Value *CGCXXABI::EmitNewArrayLengthOverflowCheck(
> + CodeGenFunction &CGF, bool ConstantOverflow, llvm::Value
> *DynamicOverflow,
> + llvm::Value *Size) {
> + llvm::Value *AllOnes = llvm::Constant::getAllOnesValue(CGF.SizeTy);
> + if (ConstantOverflow)
> + return AllOnes;
> + return CGF.Builder.CreateSelect(DynamicOverflow, AllOnes, Size);
> +}
>
> Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=216675&r1=216674&r2=216675&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGCXXABI.h (original)
> +++ cfe/trunk/lib/CodeGen/CGCXXABI.h Thu Aug 28 11:48:44 2014
> @@ -236,6 +236,13 @@ public:
>
> virtual bool EmitBadCastCall(CodeGenFunction &CGF) = 0;
>
> + /// Emit the code required to throw a std::bad_array_new_length
> exception by
> + /// the ABI, and returns the array length size to allocate.
> + virtual llvm::Value *
> + EmitNewArrayLengthOverflowCheck(CodeGenFunction &CGF, bool
> ConstantOverflow,
> + llvm::Value *DynamicOverflow,
> + llvm::Value *Size);
> +
> virtual llvm::Value *GetVirtualBaseClassOffset(CodeGenFunction &CGF,
> llvm::Value *This,
> const CXXRecordDecl
> *ClassDecl,
>
> Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=216675&r1=216674&r2=216675&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Thu Aug 28 11:48:44 2014
> @@ -572,11 +572,11 @@ static llvm::Value *EmitCXXNewAllocSize(
> }
>
> // On overflow, produce a -1 so operator new will fail.
> - if (hasAnyOverflow) {
> - size = llvm::Constant::getAllOnesValue(CGF.SizeTy);
> - } else {
> + if (hasAnyOverflow)
> + size = CGF.CGM.getCXXABI().EmitNewArrayLengthOverflowCheck(
> + CGF, true, nullptr,
> llvm::Constant::getAllOnesValue(CGF.SizeTy));
> + else
> size = llvm::ConstantInt::get(CGF.SizeTy, allocationSize);
> - }
>
> // Otherwise, we might need to use the overflow intrinsics.
> } else {
> @@ -714,9 +714,9 @@ static llvm::Value *EmitCXXNewAllocSize(
> // 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);
> + size = CGF.CGM.getCXXABI().EmitNewArrayLengthOverflowCheck(CGF,
> false,
> +
> hasOverflow,
> + size);
> }
>
> if (cookieSize == 0)
>
> Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=216675&r1=216674&r2=216675&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
> +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Thu Aug 28 11:48:44 2014
> @@ -133,6 +133,11 @@ public:
>
> bool EmitBadCastCall(CodeGenFunction &CGF) override;
>
> + llvm::Value *EmitNewArrayLengthOverflowCheck(CodeGenFunction &CGF,
> + bool ConstantOverflow,
> + llvm::Value
> *DynamicOverflow,
> + llvm::Value *Size)
> override;
> +
> llvm::Value *
> GetVirtualBaseClassOffset(CodeGenFunction &CGF, llvm::Value *This,
> const CXXRecordDecl *ClassDecl,
> @@ -1044,6 +1049,36 @@ bool ItaniumCXXABI::EmitBadCastCall(Code
> return true;
> }
>
> +llvm::Value *ItaniumCXXABI::EmitNewArrayLengthOverflowCheck(
> + CodeGenFunction &CGF, bool ConstantOverflow, llvm::Value
> *DynamicOverflow,
> + llvm::Value *Size) {
> + // In C++11 and later, an overflow results in throwing
> + // std::bad_array_new_length.
> + if (!CGF.getLangOpts().CPlusPlus11)
> + return CGCXXABI::EmitNewArrayLengthOverflowCheck(CGF,
> ConstantOverflow,
> + DynamicOverflow,
> Size);
> +
> + llvm::BasicBlock *BadArrayNewLengthBlock =
> + CGF.createBasicBlock("new.bad_array_new_length");
> + llvm::BasicBlock *EndBlock = CGF.createBasicBlock("new.end");
> +
> + if (!ConstantOverflow) {
> + assert(DynamicOverflow && "Called for dynamic case, but no overflow
> value");
> + CGF.Builder.CreateCondBr(DynamicOverflow, BadArrayNewLengthBlock,
> EndBlock);
> + }
> + CGF.EmitBlock(BadArrayNewLengthBlock);
> +
> + // void __cxa_bad_array_new_length();
> + llvm::FunctionType *FTy = llvm::FunctionType::get(CGF.VoidTy, false);
> + llvm::Value *Fn =
> + CGF.CGM.CreateRuntimeFunction(FTy, "__cxa_bad_array_new_length");
>
This is incorrect; see
http://mentorembedded.github.io/cxx-abi/abi.html#array-ctor
The function is named __cxa_*throw_*bad_array_new_length
Have you thought about how we might support this when our C++ runtime
library doesn't contain this symbol? I expect that will be the case for a
lot of systems. We could emit a weak definition of this function that calls
_Zna[lm](-1) then calls std::terminate, for instance.
+ CGF.EmitRuntimeCallOrInvoke(Fn).setDoesNotReturn();
> + CGF.Builder.CreateUnreachable();
> +
> + CGF.EmitBlock(EndBlock);
> + return Size;
> +}
> +
> llvm::Value *
> ItaniumCXXABI::GetVirtualBaseClassOffset(CodeGenFunction &CGF,
> llvm::Value *This,
>
> Modified: cfe/trunk/test/CodeGenCXX/new-array-init.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/new-array-init.cpp?rev=216675&r1=216674&r2=216675&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/new-array-init.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/new-array-init.cpp Thu Aug 28 11:48:44 2014
> @@ -14,7 +14,12 @@ void fn(int n) {
> // CHECK-LABEL: define void @_Z15const_underflowv
> void const_underflow() {
> // CHECK-NOT: icmp ult i{{32|64}} %{{[^ ]+}}, 3
> - // CHECK: call noalias i8* @_Zna{{.}}(i{{32|64}} -1)
> + // CHECK: br label %[[BAD:.*]]
> + // CHECK: [[BAD]]:
> + // CHECK-NEXT: call void @__cxa_bad_array_new_length()
> + // CHECK-NEXT: unreachable
> + // CHECK: {{.*}}:
> + // CHECK: ret void
> new int[2] { 1, 2, 3 };
> }
>
>
> Modified: cfe/trunk/test/CodeGenCXX/operator-new.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/operator-new.cpp?rev=216675&r1=216674&r2=216675&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/operator-new.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/operator-new.cpp Thu Aug 28 11:48:44 2014
> @@ -1,8 +1,7 @@
> -// RUN: %clang_cc1 -triple i686-pc-linux-gnu -emit-llvm -o %t-1.ll %s
> -// RUN: FileCheck -check-prefix SANE --input-file=%t-1.ll %s
> -// RUN: %clang_cc1 -triple i686-pc-linux-gnu -emit-llvm
> -fno-assume-sane-operator-new -o %t-2.ll %s
> -// RUN: FileCheck -check-prefix SANENOT --input-file=%t-2.ll %s
> -
> +// RUN: %clang_cc1 -triple i686-pc-linux-gnu -std=c++98 -emit-llvm -o -
> %s | FileCheck -check-prefix SANE98 %s
> +// RUN: %clang_cc1 -triple i686-pc-linux-gnu -std=c++11 -emit-llvm -o -
> %s | FileCheck -check-prefix SANE11 %s
> +// RUN: %clang_cc1 -triple i686-pc-win32-msvc -std=c++11 -emit-llvm -o -
> %s | FileCheck -check-prefix SANE11MS %s
> +// RUN: %clang_cc1 -triple i686-pc-linux-gnu -emit-llvm
> -fno-assume-sane-operator-new -o - %s | FileCheck -check-prefix SANENOT %s
>
> class teste {
> int A;
> @@ -20,10 +19,44 @@ void f1() {
> // rdar://5739832 - operator new should check for overflow in multiply.
> void *f2(long N) {
> return new int[N];
> -
> -// SANE: [[UWO:%.*]] = call {{.*}} @llvm.umul.with.overflow
> -// SANE-NEXT: [[OVER:%.*]] = extractvalue {{.*}} [[UWO]], 1
> -// SANE-NEXT: [[SUM:%.*]] = extractvalue {{.*}} [[UWO]], 0
> -// SANE-NEXT: [[RESULT:%.*]] = select i1 [[OVER]], i32 -1, i32 [[SUM]]
> -// SANE-NEXT: call noalias i8* @_Znaj(i32 [[RESULT]])
> +
> +// SANE98: [[UWO:%.*]] = call {{.*}} @llvm.umul.with.overflow
> +// SANE98-NEXT: [[OVER:%.*]] = extractvalue {{.*}} [[UWO]], 1
> +// SANE98-NEXT: [[SUM:%.*]] = extractvalue {{.*}} [[UWO]], 0
> +// SANE98-NEXT: [[RESULT:%.*]] = select i1 [[OVER]], i32 -1, i32 [[SUM]]
> +// SANE98-NEXT: call noalias i8* @_Znaj(i32 [[RESULT]])
> +
> +// SANE11: [[UWO:%.*]] = call {{.*}} @llvm.umul.with.overflow
> +// SANE11-NEXT: [[OVER:%.*]] = extractvalue {{.*}} [[UWO]], 1
> +// SANE11-NEXT: [[SUM:%.*]] = extractvalue {{.*}} [[UWO]], 0
> +// SANE11-NEXT: br i1 [[OVER]], label %[[BAD:.*]], label %[[GOOD:.*]]
> +// SANE11: [[BAD]]:
> +// SANE11-NEXT: call void @__cxa_bad_array_new_length()
> +// SANE11-NEXT: unreachable
> +// SANE11: [[GOOD]]:
> +// SANE11-NEXT: call noalias i8* @_Znaj(i32 [[SUM]])
> +
> +// FIXME: There should be a call to generate the std::bad_array_new_length
> +// exception in the Microsoft ABI, however, this is not implemented
> currently.
> +// SANE11MS: [[UWO:%.*]] = call {{.*}} @llvm.umul.with.overflow
> +// SANE11MS-NEXT: [[OVER:%.*]] = extractvalue {{.*}} [[UWO]], 1
> +// SANE11MS-NEXT: [[SUM:%.*]] = extractvalue {{.*}} [[UWO]], 0
> +// SANE11MS-NEXT: [[RESULT:%.*]] = select i1 [[OVER]], i32 -1, i32 [[SUM]]
> +// SANE11MS-NEXT: call noalias i8* @"\01??_U at YAPAXI@Z"(i32 [[RESULT]])
> +}
> +
> +#if __cplusplus > 199711L
> +void *f3() {
> + return new int[0x7FFFFFFF];
> +// SANE11: br label %[[BAD:.*]]
> +// SANE11: [[BAD]]:
> +// SANE11-NEXT: call void @__cxa_bad_array_new_length()
> +// SANE11-NEXT: unreachable
> +// SANE11: {{.*}}:
> +// SANE11-NEXT: call noalias i8* @_Znaj(i32 -1)
> +
> +// FIXME: There should be a call to generate the std::bad_array_new_length
> +// exception in the Microsoft ABI, however, this is not implemented
> currently.
> +// SANE11MS: call noalias i8* @"\01??_U at YAPAXI@Z"(i32 -1)
> }
> +#endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140828/0e4ac415/attachment.html>
More information about the cfe-commits
mailing list