Page MenuHomePhabricator

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

Authored by ssijaric on Jan 8 2019, 4:29 PM.

Details

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.

Diff Detail

Repository
rC Clang

Event Timeline

ssijaric created this revision.Jan 8 2019, 4:29 PM

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 added a subscriber: mgrang.Jan 11 2019, 10:32 AM
mgrang added inline comments.
lib/CodeGen/CGException.cpp
1635

80 char limit.

ssijaric updated this revision to Diff 181337.Jan 11 2019, 11:33 AM

Address formatting comments.

rnk accepted this revision.Jan 14 2019, 1:14 PM

lgtm

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

This revision is now accepted and ready to land.Jan 14 2019, 1:14 PM
This revision was automatically updated to reflect the committed changes.