[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:51
+  do {                                                                         \
+    _LIBUNWIND_POP_CET_SSP(1 + (fn));                                          \
+    void *cetRegContext = __libunwind_cet_get_registers((cursor));             \
----------------
MaskRay wrote:
> Question: why is this `1 + (fn)`?
Hi, @MaskRay 
This is related to libunwind implementation for _Unwind_RaiseException/Resume and _Unwind_ForcedUnwind.
For example, following is _Unwind_RaiseException code:

/// Called by __cxa_throw.  Only returns if there is a fatal error.
_LIBUNWIND_EXPORT _Unwind_Reason_Code
_Unwind_RaiseException(_Unwind_Exception *exception_object) {
  _LIBUNWIND_TRACE_API("_Unwind_RaiseException(ex_obj=%p)",
                       (void *)exception_object);
  unw_context_t uc;
  unw_cursor_t cursor;
  __unw_getcontext(&uc);

  // Mark that this is a non-forced unwind, so _Unwind_Resume()
  // can do the right thing.
  exception_object->private_1 = 0;
  exception_object->private_2 = 0;

  // phase 1: the search phase
  _Unwind_Reason_Code phase1 = unwind_phase1(&uc, &cursor, exception_object);
  if (phase1 != _URC_NO_REASON)
    return phase1;

  // phase 2: the clean up phase
  return unwind_phase2(&uc, &cursor, exception_object);
}

In phase2, _Unwind_RaiseException will call unwind_phase2 and will never return if landing pad is found. So, we need to adjust CET shadow stack for "unwind_phase2" too.
In unwind_phase2, we will record how many stack frames we need to skip(starting from _Unwind_RaiseException and unwind_phase2 is not included), store the value into framesWalked. At last, we use _LIBUNWIND_POP_CET_SSP to adjust CET shadow stack by 1 + framesWalked frames.

It seems we can have a clear way to achieve this, initializing framesWalked to 1 instead 0 and add some comments to demonstrate the initialization is for unwind_phase2.
Thanks very  much.



================
Comment at: libunwind/src/UnwindLevel1.c:55
+    __asm__ volatile("push %%edi\n\t"                                          \
+                     "sub $4, %%esp\n\t"                                       \
+                     "jmp *%%edx\n\t" ::"D"(cetRegContext),                    \
----------------
MaskRay wrote:
> Is the calling convention here wrong?
> 
> 
Hi, @MaskRay 
The code here is to emulate x86 calling convention. As we know, x86 uses stack to pass input parameter and store return address.
We will jump to "__libunwind_Registers_x86_jumpto" instead of calling it to avoid any change to CET shadow stack.
In libunwind, "__libunwind_Registers_x86_jumpto" is a pure x86 assembly function, the implementation expects the stack frame to be a standard x86 function call stack frame like following:

+______________+
+ input param   +
+______________+
+ return addr    +
+___________ __+
.....
__libunwind_Registers_x86_jumpto will never return, so the return address stored in stack will not be used. However, the implementation does rely on the stack layout, following code is from __libunwind_Registers_x86_jumpto:

DEFINE_LIBUNWIND_FUNCTION(__libunwind_Registers_x86_jumpto)
#
# extern "C" void __libunwind_Registers_x86_jumpto(Registers_x86 *);
#
# On entry:
#  +                       +
#  +-----------------------+
#  + thread_state pointer  +
#  +-----------------------+
#  + return address        +
#  +-----------------------+   <-- SP
#  +                       +
  _LIBUNWIND_CET_ENDBR
  movl   4(%esp), %eax
  # set up eax and ret on new stack location
  movl  28(%eax), %edx # edx holds new stack pointer
  subl  $8,%edx
  movl  %edx, 28(%eax)
  movl  0(%eax), %ebx
......
As we can see, we must ensure the input parameter is placed in 4(%esp), 0(%esp) stores dummy return address which will never be used.
So, after pushing input parameter for __libunwind_Registers_x86_jumpto into stack, we have to "sub $4, %%esp" in order to emulate pushing return address into stack, the pushing is aimed to make stack layout to meet __libunwind_Registers_x86_jumpto's requirement.
Thanks very much.


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

https://reviews.llvm.org/D105968