[PATCH] D56463: [SEH] Pass the frame pointer from SEH finally to finally functions

[Sanjin Sijaric via Phabricator via cfe-commits]

ssijaric created this revision.
ssijaric added reviewers: rnk, efriedma, mstorsjo, TomTan.

The following test case, compiled with -OO -target=x86_64-windows-win32, returns an incorrect value.  It returns 5, when it should return 9.  The problem is that the frame pointer that the first finally block receives is not passed onto the second finally block, but is regenerated using the localaddr intrinsic.

The test case is:

  int
  main() {
    int Check = 0;
    __try {
      Check = 3;
    } __finally {
      __try {
        Check += 2;
      } __finally {
        Check += 4; 
      }
    }
    return Check;
  }

The code generated with "-O0 --target=x86_64-windows-win32" is:

  main:                                   # @main
  .seh_proc main
  # %bb.0:                                # %entry
  	subq	$40, %rsp
  	.seh_stackalloc 40
  	.seh_endprologue
  	xorl	%ecx, %ecx
  .set .Lmain$frame_escape_0, 32  <==== Check is at %rsp of main + 32
  	movl	$0, 36(%rsp)
  	movl	$0, 32(%rsp)
  	movl	$3, 32(%rsp)
  	movq	%rsp, %rax
  	movq	%rax, %rdx  <==== main's %rsp is passed to fin$0
  	callq	"?fin$0 at 0@main@@"
  	movl	32(%rsp), %eax
  	addq	$40, %rsp
  	retq
  
  ?fin$0 at 0@main@@:
          subq    $56, %rsp
  	leaq	.Lmain$frame_escape_0(%rdx), %r8
  	movq	%rdx, 48(%rsp)
  	movb	%cl, 47(%rsp)
  	movl	(%r8), %r9d   <==== Check is at %rsp of main + 32
  	addl	$2, %r9d
  	movq	%rsp, %rdx    <====  %rsp of fin$0 is passed to fin$1, should be %rsp of main 
  	movl	%eax, %ecx
          callq	"?fin$1 at 0@main@@"
  
  
  "?fin$1 at 0@main@@":                      # @"?fin$1 at 0@main@@"
  # %bb.0:                                # %entry
  	subq	$16, %rsp
  	.seh_stackalloc 16
  	.seh_endprologue
  	leaq	.Lmain$frame_escape_0(%rdx), %rax
  	movq	%rdx, 8(%rsp)
  	movb	%cl, 7(%rsp)
  	movl	(%rax), %r8d
  	addl	$4, %r8d
  	movl	%r8d, (%rax)
  	addq	$16, %rsp
  	retq
  	.seh_handlerdata
  	.text

With this change, we get the following:

  "?fin$0 at 0@main@@":                      # @"?fin$0 at 0@main@@"
  .seh_proc "?fin$0 at 0@main@@"
  # %bb.0:                                # %entry
  	subq	$56, %rsp
  	.seh_stackalloc 56
  	.seh_endprologue
  	xorl	%eax, %eax
  	leaq	.Lmain$frame_escape_0(%rdx), %r8
  	movq	%rdx, 48(%rsp)
  	movb	%cl, 47(%rsp)
  	movl	(%r8), %r9d
  	addl	$2, %r9d
  	movl	%r9d, (%r8)
  	movl	%eax, %ecx
  	callq	"?fin$1 at 0@main@@"  <==== %rsp of main is passed in %rdx onto fin$1
  	nop
  	addq	$56, %rsp
  	retq

The change assumes that no compiler generated filter function can directly invoke a compiler generated finally function.  I haven't been able to come up with a test case where this occurs.  Otherwise, the check for IsOutlinedSEHHelper is insufficient.   All SEH finally functions have two parameters, the second being the frame pointer.   This is not the case for filter functions, as they differ on x86 vs x86_64.


Repository:
  rC Clang

https://reviews.llvm.org/D56463

Files:
  lib/CodeGen/CGException.cpp
  test/CodeGen/exceptions-seh-nested-finally.c


Index: test/CodeGen/exceptions-seh-nested-finally.c
===================================================================
--- /dev/null
+++ test/CodeGen/exceptions-seh-nested-finally.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 %s -triple x86_64-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN:         | FileCheck %s
+// RUN: %clang_cc1 %s -triple i686-pc-win32 -fms-extensions -emit-llvm -o - \
+// RUN:         | FileCheck %s
+// RUN: %clang_cc1 %s -triple aarch64-windows -fms-extensions -emit-llvm -o - \
+// RUN:         | FileCheck %s
+
+// Check that the first finally block passes the enclosing function's frame
+// pointer to the second finally block, instead of generating it via localaddr.
+
+// CHECK-LABEL: define internal void @"?fin$0 at 0@main@@"({{i8( zeroext)?}} %abnormal_termination, i8* %frame_pointer)
+// CHECK: call void @"?fin$1 at 0@main@@"({{i8( zeroext)?}} 0, i8* %frame_pointer)
+int
+main() {
+  int Check = 0;
+  __try {
+    Check = 3;
+  } __finally {
+    __try {
+      Check += 2;
+    } __finally {
+      Check += 4;
+    }
+  }
+  return Check;
+}
Index: lib/CodeGen/CGException.cpp
===================================================================
--- lib/CodeGen/CGException.cpp
+++ lib/CodeGen/CGException.cpp
@@ -1627,8 +1627,15 @@
 
     // Compute the two argument values.
     QualType ArgTys[2] = {Context.UnsignedCharTy, Context.VoidPtrTy};
-    llvm::Value *LocalAddrFn = CGM.getIntrinsic(llvm::Intrinsic::localaddress);
-    llvm::Value *FP = CGF.Builder.CreateCall(LocalAddrFn);
+    llvm::Value *FP = nullptr;
+    // If CFG.IsOutlinedSEHHelper is true, then we are within a finally block.
+    if (CGF.IsOutlinedSEHHelper)
+      FP = &CGF.CurFn->arg_begin()[1];
+    else {
+      llvm::Value *LocalAddrFn = CGM.getIntrinsic(llvm::Intrinsic::localaddress);
+      FP = CGF.Builder.CreateCall(LocalAddrFn);
+    }
+
     llvm::Value *IsForEH =
         llvm::ConstantInt::get(CGF.ConvertType(ArgTys[0]), F.isForEHCleanup());
     Args.add(RValue::get(IsForEH), ArgTys[0]);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56463.180763.patch
Type: text/x-patch
Size: 2018 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190109/bac3f738/attachment.bin>