[SEH] Pass the frame pointer from SEH finally to finally functions

[ssijaric]

Summary

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@0@main@@"
	movl	32(%rsp), %eax
	addq	$40, %rsp
	retq

?fin$0@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@0@main@@"


"?fin$1@0@main@@":                      # @"?fin$1@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@0@main@@":                      # @"?fin$0@0@main@@"
.seh_proc "?fin$0@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@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.

[efriedma]

It's currently possible to write, in clang:

void f() {
  __try {
  }
  __except(({__try{}__finally{}; 3;})) {
  }
}

And the following currently crashes if you try to build it with clang:

struct A { ~A(); };
int f(const A&);
void g() {
  __try {
  }
  __except(f(A())) {
  }
}

But both of those should be rejected; it should be safe to assume it's impossible to have an __finally block inside a filter.

I think this looks fine, but I'd prefer if Reid could take a quick look.

lib/CodeGen/CGException.cpp

1632

Please use braces consistently.

[mgrang]

lib/CodeGen/CGException.cpp

1635

80 char limit.

[ssijaric]

Address formatting comments.

[rnk]

lgtm

Thanks! I'm surprised we passed as much of the Microsoft exception tests as we did with this bug. Maybe it regressed.

lib/CodeGen/CGException.cpp

struct PerformSEHFinally final : EHScopeStack::Cleanup {
   void Emit(CodeGenFunction &CGF, Flags F) override {
     ASTContext &Context = CGF.getContext();
     CodeGenModule &CGM = CGF.CGM;
 
     CallArgList Args;
 
     // 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]);
     Args.add(RValue::get(FP), ArgTys[1]);
 
     // Arrange a two-arg function info and type.
     const CGFunctionInfo &FnInfo =
         CGM.getTypes().arrangeBuiltinFunctionCall(Context.VoidTy, Args);

test/CodeGen/exceptions-seh-nested-finally.c

+// 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@0@main@@"({{i8( zeroext)?}} %abnormal_termination, i8* %frame_pointer)
+// CHECK: call void @"?fin$1@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;
+}