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

[H.J Lu via Phabricator via llvm-commits]

hjl.tools 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:
> > > > 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.


================
Comment at: libunwind/src/assembly.h:18
 
+#if (defined(__i386__) || defined(__x86_64__)) && defined(__linux__)
+#include <cet.h>
----------------
xiongji90 wrote:
> compnerd wrote:
> > Do you need to consider MinGW or cywin?
> Hi, @compnerd 
> I suggest to ignore MinGW and cywin currently.
> Hi, @hjl.tools 
> Do you think we can ignore MinGW and cywin?
> 
> Thanks very much.
If MinGW and cywin support CET, libunwind should support MinGW and cywin.


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

https://reviews.llvm.org/D105968