[PATCH] Throw std::bad_array_new_length as expected

[Aaron Ballman]

I think this patch addresses your review comments.

Thanks!

~Aaron

On Wed, Aug 27, 2014 at 5:46 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oper_new.patch
Type: application/octet-stream
Size: 8887 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140828/3bb1aed8/attachment.obj>