[PATCH] Throw std::bad_array_new_length as expected

[Reid Kleckner]

lgtm, thanks!



On Thu, Aug 28, 2014 at 6:11 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> 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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140828/b1a585e0/attachment.html>