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

Nico Weber nicolasweber at gmx.de
Thu Feb 26 14:34:34 PST 2015


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 {





More information about the cfe-commits mailing list