[libcxx-commits] [PATCH] D105968: [libunwind][CET] Support exception handling stack unwind in CET environment

[xiongji90 via Phabricator via libcxx-commits]

xiongji90 added inline comments.


================
Comment at: libunwind/src/UnwindLevel1.c:42
+    _LIBUNWIND_POP_CET_SSP(1);                                                 \
+    __libunwind_cet_jumpto((cursor), (frames_skipped));                        \
+  } while (0)
----------------
xiongji90 wrote:
> hjl.tools wrote:
> > xiongji90 wrote:
> > > hjl.tools wrote:
> > > > xiongji90 wrote:
> > > > > hjl.tools wrote:
> > > > > > xiongji90 wrote:
> > > > > > > hjl.tools wrote:
> > > > > > > > Why is this needed?  Can you unwind the shadow stack and jump to the landing pad as usual?
> > > > > > > Hi, @hjl.tools 
> > > > > > > I have updated the patch to remove some unnecessary code, could you help review again?
> > > > > > > Thanks very much.
> > > > > > __unw_phase2_resume looks quite fragile.   Can you call the function pointer,  cetJumpAddress, in C directly?
> > > > > Hi, @hjl.tools 
> > > > > I am afraid calling a function pointer in C is risky. The cetJumpAddress is address of __libunwind_Registers_x86/x86_64_jumpto and these functions will never return, if we call this function or call the function pointer, compiler codegen may generate following code:
> > > > > "
> > > > > mov __libunwind_Registers_x86_64_jumpto, %rax
> > > > > call *%rax 
> > > > > "
> > > > > Under such circumstance, we must adjust CET SSP by 1 in __libunwind_Registers_x86_64_jumpto since the call instruction will push CET shadow stack.
> > > > > 
> > > > > However, if compiler codegen use "jmp" instead of "call", for example:
> > > > > "
> > > > > jmp  xxxxx (the addr of __libunwind_Registers_x86_64_jumpto)
> > > > > "
> > > > > we shouldn't adjust CET shadow stack. It is difficult for us to decide whether or not should we adjust CET shadow stack since we can't know what code compiler will generate for the function/function pointer call in C source level.
> > > > > 
> > > > > Thanks very much.
> > > > GCC uses an undocumented intrinsic, __builtin_eh_return, to avoid such issue.
> > > Hi, @hjl.tools 
> > > The undocumented GCC intrinsic __builtin_eh_return are used by GCC unwinder and its semantic are strongly related to GCC unwind's implementation.
> > > For example, GCC's unwinder will install target context and then get target landing pad address, after that GCC unwinder will use __builtin_eh_return to adjust the SP value and jump to landing pad, __builtin_eh_return assumes that target context has been prepared.
> > > In our implementation, we need to adjust CET SSP and then jump to __libunwind_Registers_X86/_64_jumpto(asm code), this asm function will prepare target context (restore registers), adjust SP and jump to final landing pad. Our requirement here is just directly jump to __libunwind_Registers_X86/_64_jumpto function to avoid pushing CET shadow stack again.  So, I think __builtin_eh_return can't help much in libunwind.
> > > 
> > > Thanks very much.
> > One feature of __builtin_eh_return is to perform the special tail call.   Can clang provide a way to do that?
> Hi, @hjl.tools 
> I think clang is compatible with GCC for __builtin_eh_return and could you share some sample code or info about this special feature?
> If it can work, we can get rid of the inline asm.
> Thanks very much.
Hi, @hjl.tools 
I tried to use the undocumented intrinsic "__builtin_eh_return" to replace inline asm. The code for x86_64 is like following:
__builtin_eh_return(0, __libunwind_Registers_x86_64_jumpto);
With "-fcf-protection=full", both gcc and clang will use a "jmpq *%rcx" to jump to __libunwind_Registers_x86_64 which meets our requirement.
However, this intrinsic is used to jump to landing pad and expects to pass parameters via rax and rdx while __libunwind_Registers_x86_64_jumpto is not  a landing pad but a "normal" function which doesn't return. __libunwind_Registers_x86_64_jumpto follows x86_64 abi to receive input parameters which means it expects input parameters from rdi, rsi.... And __libunwind_Registers_x86_jumpto follows x86 abi to receive input parameters which means it expects input parameters stored in stack by caller. So I am afraid __builtin_eh_return can't fully meet our requirement.
Thanks very much.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105968/new/

https://reviews.llvm.org/D105968