r230697 - Don't crash on leaving nested __finally blocks through an EH edge.
Daniel Jasper
djasper at google.com
Sat Feb 28 11:10:03 PST 2015
Yes. I first replied here and then realized I didn't need to revert quite
as much.
On Fri, Feb 27, 2015 at 5:44 PM, Nico Weber <thakis at chromium.org> wrote:
> Thanks, sorry about that. I tried to run the tests in both +Asserts and
> -Asserts builds. I must've misconfigured my -Asserts build on my desktop to
> not actually be -Asserts. I could reproduce the failure on my -Asserts
> build on my laptop. I fixed relanded the two tests in r230764. (The test
> added by this revision here was fine as is though, right?)
>
> On Fri, Feb 27, 2015 at 12:19 AM, Daniel Jasper <djasper at google.com>
> wrote:
>
>> I'll just revert the test (r230717) as I think that should make the bots
>> happy again.
>>
>> On Fri, Feb 27, 2015 at 9:01 AM, Daniel Jasper <djasper at google.com>
>> wrote:
>>
>>> This seems to be breaking bots again:
>>>
>>> http://lab.llvm.org:8080/green/job/clang-stage2-configure-Rlto_check/2355/
>>>
>>> On Thu, Feb 26, 2015 at 11:34 PM, Nico Weber <nicolasweber at gmx.de>
>>> wrote:
>>>
>>>> Author: nico
>>>> Date: Thu Feb 26 16:34:33 2015
>>>> New Revision: 230697
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=230697&view=rev
>>>> Log:
>>>> Don't crash on leaving nested __finally blocks through an EH edge.
>>>>
>>>> The __finally emission block tries to be clever by removing unused
>>>> continuation
>>>> edges if there's an unconditional jump out of the __finally block. With
>>>> exception edges, the EH continuation edge isn't always unused though
>>>> and we'd
>>>> crash in a few places.
>>>>
>>>> Just don't be clever. That makes the IR for __finally blocks a bit
>>>> longer in
>>>> some cases (hence small and behavior-preserving changes to existing
>>>> tests), but
>>>> it makes no difference in general and it fixes the last crash from
>>>> PR22553.
>>>>
>>>> http://reviews.llvm.org/D7918
>>>>
>>>> Modified:
>>>> cfe/trunk/lib/CodeGen/CGException.cpp
>>>> cfe/trunk/test/CodeGen/exceptions-seh-finally.c
>>>> cfe/trunk/test/CodeGen/exceptions-seh-leave.c
>>>>
>>>> Modified: cfe/trunk/lib/CodeGen/CGException.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGException.cpp?rev=230697&r1=230696&r2=230697&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/CodeGen/CGException.cpp (original)
>>>> +++ cfe/trunk/lib/CodeGen/CGException.cpp Thu Feb 26 16:34:33 2015
>>>> @@ -1684,8 +1684,7 @@ llvm::BasicBlock *CodeGenFunction::getEH
>>>> const char *RethrowName = Personality.CatchallRethrowFn;
>>>> if (RethrowName != nullptr && !isCleanup) {
>>>> EmitRuntimeCall(getCatchallRethrowFn(CGM, RethrowName),
>>>> - getExceptionFromSlot())
>>>> - ->setDoesNotReturn();
>>>> + getExceptionFromSlot())->setDoesNotReturn();
>>>> Builder.CreateUnreachable();
>>>> Builder.restoreIP(SavedIP);
>>>> return EHResumeBlock;
>>>> @@ -1943,23 +1942,17 @@ void CodeGenFunction::ExitSEHTryStmt(con
>>>> Builder.SetInsertPoint(FI.FinallyBB);
>>>> EmitStmt(Finally->getBlock());
>>>>
>>>> - // If the finally block doesn't fall through, we don't need these
>>>> blocks.
>>>> - if (!HaveInsertPoint()) {
>>>> - FI.ContBB->eraseFromParent();
>>>> - if (FI.ResumeBB)
>>>> - FI.ResumeBB->eraseFromParent();
>>>> - return;
>>>> - }
>>>> -
>>>> - if (FI.ResumeBB) {
>>>> - llvm::Value *IsEH =
>>>> Builder.CreateLoad(getAbnormalTerminationSlot(),
>>>> - "abnormal.termination");
>>>> - IsEH = Builder.CreateICmpEQ(IsEH, llvm::ConstantInt::get(Int8Ty,
>>>> 0));
>>>> - Builder.CreateCondBr(IsEH, FI.ContBB, FI.ResumeBB);
>>>> - } else {
>>>> - // There was nothing exceptional in the try body, so we only
>>>> have normal
>>>> - // control flow.
>>>> - Builder.CreateBr(FI.ContBB);
>>>> + if (HaveInsertPoint()) {
>>>> + if (FI.ResumeBB) {
>>>> + llvm::Value *IsEH =
>>>> Builder.CreateLoad(getAbnormalTerminationSlot(),
>>>> + "abnormal.termination");
>>>> + IsEH = Builder.CreateICmpEQ(IsEH,
>>>> llvm::ConstantInt::get(Int8Ty, 0));
>>>> + Builder.CreateCondBr(IsEH, FI.ContBB, FI.ResumeBB);
>>>> + } else {
>>>> + // There was nothing exceptional in the try body, so we only
>>>> have normal
>>>> + // control flow.
>>>> + Builder.CreateBr(FI.ContBB);
>>>> + }
>>>> }
>>>>
>>>> Builder.restoreIP(SavedIP);
>>>>
>>>> Modified: cfe/trunk/test/CodeGen/exceptions-seh-finally.c
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/exceptions-seh-finally.c?rev=230697&r1=230696&r2=230697&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/test/CodeGen/exceptions-seh-finally.c (original)
>>>> +++ cfe/trunk/test/CodeGen/exceptions-seh-finally.c Thu Feb 26 16:34:33
>>>> 2015
>>>> @@ -193,12 +193,99 @@ int nested___finally___finally() {
>>>> // CHECK-NEXT: br label %[[finally:[^ ]*]]
>>>> //
>>>> // CHECK: [[finally]]
>>>> -// CHECK-NEXT: store i32 1, i32* %
>>>> -// CHECK-NEXT: store i8 0, i8* %
>>>> +// CHECK-NEXT: store i32 1, i32* %
>>>> +// CHECK-NEXT: store i32 1, i32* %
>>>> +// CHECK-NEXT: br label %[[cleanup:[^ ]*]]
>>>> +//
>>>> +// The finally's unreachable continuation block:
>>>> +// CHECK: store i32 0, i32* %
>>>> +// CHECK-NEXT: br label %[[cleanup]]
>>>> +//
>>>> +// CHECK: [[cleanup]]
>>>> +// CHECK-NEXT: store i8 0, i8* %
>>>> // CHECK-NEXT: br label %[[outerfinally:[^ ]*]]
>>>> //
>>>> // CHECK: [[outerfinally]]
>>>> // CHECK-NEXT: br label %[[finallycont:[^ ]*]]
>>>> //
>>>> // CHECK: [[finallycont]]
>>>> -// CHECK-NEXT: ret i32 1
>>>> +// CHECK-NEXT: %[[dest:[^ ]*]] = load i32* %
>>>> +// CHECK-NEXT: switch i32 %[[dest]]
>>>> +// CHECK-NEXT: i32 0, label %[[cleanupcont:[^ ]*]]
>>>> +//
>>>> +// CHECK: [[cleanupcont]]
>>>> +// CHECK-NEXT: store i32 0, i32* %
>>>> +// CHECK-NEXT: br label %[[return:[^ ]*]]
>>>> +//
>>>> +// CHECK: [[return]]
>>>> +// CHECK-NEXT: %[[reg:[^ ]*]] = load i32* %
>>>> +// CHECK-NEXT: ret i32 %[[reg]]
>>>> +
>>>> +int nested___finally___finally_with_eh_edge() {
>>>> + __try {
>>>> + __try {
>>>> + might_crash();
>>>> + } __finally {
>>>> + return 899;
>>>> + }
>>>> + } __finally {
>>>> + // Intentionally no return here.
>>>> + }
>>>> + return 912;
>>>> +}
>>>> +// CHECK-LABEL: define i32 @nested___finally___finally_with_eh_edge
>>>> +// CHECK: invoke void @might_crash() #3
>>>> +// CHECK-NEXT: to label %[[invokecont:[^ ]*]] unwind label %[[lpad:[^
>>>> ]*]]
>>>> +//
>>>> +// CHECK: [[invokecont]]
>>>> +// CHECK-NEXT: store i8 0, i8* %[[abnormal:[^ ]*]]
>>>> +// CHECK-NEXT: br label %[[finally:[^ ]*]]
>>>> +
>>>> +// CHECK: [[finally]]
>>>> +// CHECK-NEXT: store i32 899, i32* %
>>>> +// CHECK-NEXT: store i32 1, i32* %
>>>> +// CHECK-NEXT: br label %[[cleanup:[^ ]*]]
>>>> +//
>>>> +// The inner finally's unreachable continuation block:
>>>> +// CHECK: store i32 0, i32* %
>>>> +// CHECK-NEXT: br label %[[cleanup]]
>>>> +//
>>>> +// CHECK: [[cleanup]]
>>>> +// CHECK-NEXT: store i8 0, i8* %
>>>> +// CHECK-NEXT: br label %[[outerfinally:[^ ]*]]
>>>> +//
>>>> +// CHECK: [[outerfinally]]
>>>> +// CHECK-NEXT: %[[abnormallocal:[^ ]*]] = load i8* %[[abnormal]]
>>>> +// CHECK-NEXT: %[[reg:[^ ]*]] = icmp eq i8 %[[abnormallocal]], 0
>>>> +// CHECK-NEXT: br i1 %[[reg]], label %[[finallycont:[^ ]*]], label
>>>> %[[finallyresume:[^ ]*]]
>>>> +//
>>>> +// CHECK: [[finallycont]]
>>>> +// CHECK-NEXT: %[[dest:[^ ]*]] = load i32* %
>>>> +// CHECK-NEXT: switch i32 %[[dest]]
>>>> +// CHECK-NEXT: i32 0, label %[[cleanupcont:[^ ]*]]
>>>> +//
>>>> +// CHECK: [[cleanupcont]]
>>>> +// CHECK-NEXT: store i32 912, i32* %
>>>> +// CHECK-NEXT: br label %[[return:[^ ]*]]
>>>> +//
>>>> +//
>>>> +// CHECK: [[lpad]]
>>>> +// CHECK-NEXT: landingpad
>>>> +// CHECK-NEXT: cleanup
>>>> +// CHECK: store i8 1, i8* %[[abnormal]]
>>>> +// CHECK: br label %[[finally]]
>>>> +//
>>>> +// The inner finally's unreachable resume block:
>>>> +// CHECK: store i8 1, i8* %[[abnormal]]
>>>> +// CHECK-NEXT: br label %[[outerfinally]]
>>>> +//
>>>> +// CHECK: [[finallyresume]]
>>>> +// CHECK-NEXT: br label %[[ehresume:[^ ]*]]
>>>> +//
>>>> +// CHECK: [[return]]
>>>> +// CHECK-NEXT: %[[reg:[^ ]*]] = load i32* %
>>>> +// CHECK-NEXT: ret i32 %[[reg]]
>>>> +//
>>>> +// The ehresume block, not reachable either.
>>>> +// CHECK: [[ehresume]]
>>>> +// CHECK: resume
>>>>
>>>> Modified: cfe/trunk/test/CodeGen/exceptions-seh-leave.c
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/exceptions-seh-leave.c?rev=230697&r1=230696&r2=230697&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/test/CodeGen/exceptions-seh-leave.c (original)
>>>> +++ cfe/trunk/test/CodeGen/exceptions-seh-leave.c Thu Feb 26 16:34:33
>>>> 2015
>>>> @@ -162,6 +162,13 @@ int nested___except___finally() {
>>>> // CHECK-NEXT: br label %[[tryleave:[^ ]*]]
>>>> // CHECK-NOT: store i32 23
>>>>
>>>> +// Unused __finally continuation block
>>>> +// CHECK: store i32 51, i32* %
>>>> +// CHECK-NEXT: br label %[[tryleave]]
>>>> +
>>>> +// CHECK: [[tryleave]]
>>>> +// CHECK-NEXT: br label %[[trycont:[^ ]*]]
>>>> +
>>>> // CHECK: [[g1_lpad]]
>>>> // CHECK: store i8 1, i8* %
>>>> // CHECK-NEXT: br label %[[finally]]
>>>> @@ -171,14 +178,11 @@ int nested___except___finally() {
>>>> // CHECK: br label %[[except:[^ ]*]]
>>>>
>>>> // CHECK: [[except]]
>>>> -// CHECK-NEXT: br label %[[trycont:[^ ]*]]
>>>> +// CHECK-NEXT: br label %[[trycont]]
>>>>
>>>> // CHECK: [[trycont]]
>>>> // CHECK-NEXT: ret i32 1
>>>>
>>>> -// CHECK: [[tryleave]]
>>>> -// CHECK-NEXT: br label %[[trycont]]
>>>> -
>>>> int nested___except___except() {
>>>> int myres = 0;
>>>> __try {
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>>
>>>
>>
>> _______________________________________________
>> 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/20150228/94ab32fa/attachment.html>
More information about the cfe-commits
mailing list