This is an archive of the discontinued LLVM Phabricator instance.

[StackProtector] Fix computation of GSCookieOffset and EHCookieOffset with SEH4
ClosedPublic

Authored by etienneb on Jun 10 2016, 9:44 AM.

Details

Summary

Fix the computation of the offsets present in the scopetable when using the
SEH (__except_handler4).

This patch added an intrinsic to track the position of the allocation on the
stack of the EHGuard. This position is needed when producing the ScopeTable.

struct _EH4_SCOPETABLE {
    DWORD GSCookieOffset;
    DWORD GSCookieXOROffset;
    DWORD EHCookieOffset;
    DWORD EHCookieXOROffset;
    _EH4_SCOPETABLE_RECORD ScopeRecord[1];
};

struct _EH4_SCOPETABLE_RECORD {
    DWORD EnclosingLevel;
    long (*FilterFunc)();
        union {
        void (*HandlerAddress)();
        void (*FinallyFunc)();
    };
};

The code to generate the EHCookie is added in X86WinEHState.cpp.
Which is adding these instructions when using SEH4.

Lfunc_begin0:
# BB#0:                                 # %entry
	pushl	%ebp
	movl	%esp, %ebp
	pushl	%ebx
	pushl	%edi
	pushl	%esi
	subl	$28, %esp
	movl	%ebp, %eax                <<-- Loading FramePtr
	movl	%esp, -36(%ebp)
	movl	$-2, -16(%ebp)
	movl	$L__ehtable$use_except_handler4_ssp, %ecx
	xorl	___security_cookie, %ecx
	movl	%ecx, -20(%ebp)
	xorl	___security_cookie, %eax  <<-- XOR FramePtr and Cookie
	movl	%eax, -40(%ebp)           <<-- Storing EHGuard
	leal	-28(%ebp), %eax
	movl	$__except_handler4, -24(%ebp)
	movl	%fs:0, %ecx
	movl	%ecx, -28(%ebp)
	movl	%eax, %fs:0
	movl	$0, -16(%ebp)
	calll	_may_throw_or_crash
LBB1_1:                                 # %cont
	movl	-28(%ebp), %eax
	movl	%eax, %fs:0
	addl	$28, %esp
	popl	%esi
	popl	%edi
	popl	%ebx
	popl	%ebp
	retl

And the corresponding offset is computed:

Luse_except_handler4_ssp$parent_frame_offset = -36
	.p2align	2
L__ehtable$use_except_handler4_ssp:
	.long	-2                      # GSCookieOffset
	.long	0                       # GSCookieXOROffset
	.long	-40                     # EHCookieOffset    <<----
	.long	0                       # EHCookieXOROffset
	.long	-2                      # ToState
	.long	_catchall_filt          # FilterFunction
	.long	LBB1_2                  # ExceptionHandler

Clang is not yet producing function using SEH4, but it's a work in progress.
This patch is a step toward having a valid implementation of SEH4.
Unfortunately, it is not yet fully working. The EH registration block is not
allocated at the right offset on the stack.

Diff Detail

Event Timeline

etienneb updated this revision to Diff 60367.Jun 10 2016, 9:44 AM
etienneb retitled this revision from to [StackProtector] Fix computation of GSCookieOffset and EHCookieOffset with SEH4.
etienneb updated this object.
etienneb added a reviewer: rnk.
etienneb added subscribers: chrisha, llvm-commits.
rnk edited edge metadata.Jun 10 2016, 12:13 PM

The direction is consistent with what we've done before, but in retrospect, I kinda wish we did this in MI rather than IR. Changing that is out of scope though.

I'm fine with this if David majnemer is fine with this.

majnemer added inline comments.Jun 10 2016, 12:53 PM
lib/CodeGen/AsmPrinter/WinException.cpp
968

Is it possible for this value to actually make it into the object file?

lib/Target/X86/X86WinEHState.cpp
302–304

Is __CxxLongjmpUnwind still used if we are using C++ exceptions + /GS?

643

You could just do EHGuardNode->getNextNode()

etienneb updated this revision to Diff 60396.Jun 10 2016, 1:43 PM
etienneb marked 3 inline comments as done.

fix comments

lib/CodeGen/AsmPrinter/WinException.cpp
968

For now, I think it can be there.
But, when the feature will be implemented correctly, this should be turn as an "assert".

Would you prefer a TODO?
It's still on my plate to cleanup this.

lib/Target/X86/X86WinEHState.cpp
302–304

I didn't look to C++ exceptions yet, neither the longjmp/setjmp implementation.
I want to fix the stack-frame layout to make SEH4 working for the _try/_catch case.
Then, I'll look to other cases using SEH4.

majnemer accepted this revision.Jun 10 2016, 2:39 PM
majnemer edited edge metadata.

LGTM

lib/CodeGen/AsmPrinter/WinException.cpp
968

Yeah, a TODO would be good.

lib/Target/X86/X86WinEHState.cpp
302–304

Maybe I was thinking of _seh_longjmp_unwind4, It looks like I already added some code for it.

This revision is now accepted and ready to land.Jun 10 2016, 2:39 PM
rnk accepted this revision.Jun 15 2016, 11:34 AM
rnk edited edge metadata.

lgtm

etienneb updated this revision to Diff 61382.Jun 21 2016, 8:43 AM
etienneb marked 2 inline comments as done.
etienneb edited edge metadata.

add todo

etienneb closed this revision.Jun 21 2016, 9:05 AM