[PATCH] Throw std::bad_array_new_length as expected

[Aaron Ballman]

On Wed, Aug 27, 2014 at 5:41 PM, Reid Kleckner <rnk at google.com> wrote:
> -// 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
> %t-1.ll %s
> +// RUN: FileCheck -check-prefix SANE98 --input-file=%t-1.ll %s
> +// RUN: %clang_cc1 -triple i686-pc-linux-gnu -std=c++11 -emit-llvm -o
> %t-2.ll %s
> +// RUN: FileCheck -check-prefix SANE11 --input-file=%t-2.ll %s
> +// RUN: %clang_cc1 -triple i686-pc-win32-msvc -std=c++11 -emit-llvm -o
> %t-3.ll %s
> +// RUN: FileCheck -check-prefix SANE11MS --input-file=%t-3.ll %s
> +// RUN: %clang_cc1 -triple i686-pc-linux-gnu -emit-llvm
> -fno-assume-sane-operator-new -o %t-4.ll %s
> +// RUN: FileCheck -check-prefix SANENOT --input-file=%t-4.ll %s
>
> These RUN lines should use pipes. The less tempfiles we need, the better.

Can do.

>
> +// 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: br i1 [[OVER]], label %[[BAD:.*]], label %[[GOOD:.*]]
> +// SANE11MS: [[BAD]]:
> +// SANE11MS-NEXT: br label %[[GOOD]]
> +// SANE11MS: [[GOOD]]:
> +// SANE11MS-NEXT: call noalias i8* @"\01??_U at YAPAXI@Z"(i32 [[SUM]])
>
> I think we should keep passing -1 on overflow on Windows in C++11,
> especially given that it's our default language mode in clang-cl.

Yes, good catch! That was unintentionally changed.

>
> +    if (hasOverflow) {
> +      // In C++11 and later, overflow for a call to operator new[] should
> throw
> +      // a std::bad_array_new_length exception.
> +      if (CGF.getLangOpts().CPlusPlus11) {
> +        llvm::BasicBlock *BadArrayNewLengthBlock =
> +          CGF.createBasicBlock("new.bad_array_new_length");
> +        llvm::BasicBlock *EndBlock = CGF.createBasicBlock("new.end");
> +
> +        CGF.Builder.CreateCondBr(hasOverflow, BadArrayNewLengthBlock,
> EndBlock);
> +        CGF.EmitBlock(BadArrayNewLengthBlock);
> +        CGF.CGM.getCXXABI().EmitBadArrayNewLengthCall(CGF);
> +        CGF.EmitBlock(EndBlock);
> +      } else
> +        size = CGF.Builder.CreateSelect(
> +            hasOverflow, llvm::Constant::getAllOnesValue(CGF.SizeTy),
> size);
> +    }
>
> This is semi-duplicated from above. Here's how I think we can do it with the
> least duplication. Make the CGCXXABI prototype be something like:
> Value *EmitNewArrayLengthOverflowCheck(CodeGenFunction &CGF, bool
> ConstantOverflow, Value *DynamicOverflow, Value *Size);
>
> Then, provide an implementation in CGCXXABI which implements the old
> behavior of returning -1 if ConstantOverflow is true and emitting a select
> i1 on DynamicOverflow with -1.
>
> In ItaniumCXXABI, we can override the method, and in non-C++11 mode,
> delegate back to the base class implementation. In C++11 mode, we can emit
> the basic blocks if we need dynamic overflow checks. MicrosoftCXXABI doesn't
> need to change at all.
>
> Sound good?

Sounds good, I will come back with an updated patch.

Thank you!

~Aaron