r230697 - Don't crash on leaving nested __finally blocks through an EH edge.

Nico Weber thakis at chromium.org
Fri Feb 27 08:44:43 PST 2015


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/20150227/376ba88d/attachment.html>


More information about the cfe-commits mailing list