[cfe-commits] r110760 - in /cfe/trunk: lib/CodeGen/CGException.cpp lib/CodeGen/CGObjCMac.cpp test/CodeGenObjC/exceptions.m

[John McCall]

Author: rjmccall
Date: Tue Aug 10 19:16:14 2010
New Revision: 110760

URL: http://llvm.org/viewvc/llvm-project?rev=110760&view=rev
Log:
Fix a bug in @finally emission in both the fragile and non-fragile EH schemes
where we weren't accounting for the possibility that a @finally block might
have internal cleanups and therefore might write to the cleanup destination slot.

Fixes <rdar://problem/8293901>.


Modified:
    cfe/trunk/lib/CodeGen/CGException.cpp
    cfe/trunk/lib/CodeGen/CGObjCMac.cpp
    cfe/trunk/test/CodeGenObjC/exceptions.m

Modified: cfe/trunk/lib/CodeGen/CGException.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGException.cpp?rev=110760&r1=110759&r2=110760&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGException.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGException.cpp Tue Aug 10 19:16:14 2010
@@ -1326,6 +1326,12 @@
         CGF.EHStack.pushCleanup<CallEndCatchForFinally>(NormalAndEHCleanup,
                                                         ForEHVar, EndCatchFn);
 
+      // Save the current cleanup destination in case there are
+      // cleanups in the finally block.
+      llvm::Value *SavedCleanupDest =
+        CGF.Builder.CreateLoad(CGF.getNormalCleanupDestSlot(),
+                               "cleanup.dest.saved");
+
       // Emit the finally block.
       CGF.EmitStmt(Body);
 
@@ -1349,6 +1355,10 @@
         CGF.Builder.CreateUnreachable();
 
         CGF.EmitBlock(ContBB);
+
+        // Restore the cleanup destination.
+        CGF.Builder.CreateStore(SavedCleanupDest,
+                                CGF.getNormalCleanupDestSlot());
       }
 
       // Leave the end-catch cleanup.  As an optimization, pretend that

Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=110760&r1=110759&r2=110760&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Tue Aug 10 19:16:14 2010
@@ -2674,11 +2674,22 @@
 
       if (isa<ObjCAtTryStmt>(S)) {
         if (const ObjCAtFinallyStmt* FinallyStmt =
-            cast<ObjCAtTryStmt>(S).getFinallyStmt())
+              cast<ObjCAtTryStmt>(S).getFinallyStmt()) {
+          // Save the current cleanup destination in case there's
+          // control flow inside the finally statement.
+          llvm::Value *CurCleanupDest =
+            CGF.Builder.CreateLoad(CGF.getNormalCleanupDestSlot());
+
           CGF.EmitStmt(FinallyStmt->getFinallyBody());
 
-        // Currently, the end of the cleanup must always exist.
-        CGF.EnsureInsertPoint();
+          if (CGF.HaveInsertPoint()) {
+            CGF.Builder.CreateStore(CurCleanupDest,
+                                    CGF.getNormalCleanupDestSlot());
+          } else {
+            // Currently, the end of the cleanup must always exist.
+            CGF.EnsureInsertPoint();
+          }
+        }
       } else {
         // Emit objc_sync_exit(expr); as finally's sole statement for
         // @synchronized.
@@ -3030,8 +3041,8 @@
   llvm::BasicBlock *TryBlock = CGF.createBasicBlock("try");
   llvm::BasicBlock *TryHandler = CGF.createBasicBlock("try.handler");
   llvm::Value *DidCatch =
-    CGF.Builder.CreateIsNull(SetJmpResult, "did_catch_exception");
-  CGF.Builder.CreateCondBr(DidCatch, TryBlock, TryHandler);
+    CGF.Builder.CreateIsNotNull(SetJmpResult, "did_catch_exception");
+  CGF.Builder.CreateCondBr(DidCatch, TryHandler, TryBlock);
 
   // Emit the protected block.
   CGF.EmitBlock(TryBlock);

Modified: cfe/trunk/test/CodeGenObjC/exceptions.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/exceptions.m?rev=110760&r1=110759&r2=110760&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjC/exceptions.m (original)
+++ cfe/trunk/test/CodeGenObjC/exceptions.m Tue Aug 10 19:16:14 2010
@@ -82,3 +82,54 @@
   }
   return x;
 }
+
+// Test that the cleanup destination is saved when entering a finally
+// block.  rdar://problem/8293901
+// CHECK: define void @f3()
+void f3() {
+  extern void f3_helper(int, int*);
+
+  // CHECK:      [[X:%.*]] = alloca i32
+  // CHECK:      store i32 0, i32* [[X]]
+  int x = 0;
+
+  // CHECK:      call void @objc_exception_try_enter(
+  // CHECK:      call i32 @_setjmp
+  // CHECK-NEXT: icmp eq
+  // CHECK-NEXT: br i1
+
+  @try {
+    // CHECK:    call void @f3_helper(i32 0, i32* [[X]])
+    // CHECK:    call void @objc_exception_try_exit(
+    f3_helper(0, &x);
+  } @finally {
+    // CHECK:    [[DEST1:%.*]] = phi i32 [ 0, {{%.*}} ], [ 3, {{%.*}} ]
+    // CHECK:    call void @objc_exception_try_enter
+    // CHECK:    call i32 @_setjmp
+    @try {
+      // CHECK:  call void @f3_helper(i32 1, i32* [[X]])
+      // CHECK:  call void @objc_exception_try_exit(
+      f3_helper(1, &x);
+    } @finally {
+      // CHECK:  [[DEST2:%.*]] = phi i32 [ 0, {{%.*}} ], [ 5, {{%.*}} ]
+      // CHECK:  call void @f3_helper(i32 2, i32* [[X]])
+      f3_helper(2, &x);
+
+      // This loop is large enough to dissuade the optimizer from just
+      // duplicating the finally block.
+      while (x) f3_helper(3, &x);
+
+      // It's okay for this to turn into a test against 0.
+      // CHECK:  icmp eq i32 [[DEST2]], 5
+      // CHECK:  br i1
+    }
+
+    // It's okay for this to turn into a test against 0.
+    // CHECK:    icmp eq i32 [[DEST1]], 3
+    // CHECK:    br i1
+  }
+
+  // CHECK:      call void @f3_helper(i32 4, i32* [[X]])
+  // CHECK-NEXT: ret void
+  f3_helper(4, &x);
+}