[libcxx-commits] [PATCH] D124765: [libunwind][SystemZ] Unwind out of signal handlers

[Ulrich Weigand via Phabricator via libcxx-commits]

uweigand marked 9 inline comments as done.
uweigand added inline comments.


================
Comment at: libunwind/src/UnwindCursor.hpp:2667
+  // specific instruction (see below). Typically the trampoline comes from the
+  // vDSO (i.e. the __kernel_[rt_]sigreturn function). A libc might provide its
+  // own restorer function, though, or user-mode QEMU might write a trampoline
----------------
MaskRay wrote:
> Does s390 use both `__kernel_[rt_]sigreturn`? It seems that the Linux kernel source references both. For aarch64 there is only the `rt_` version.
> 
> Just a note for you to double check the comment:)
Yes, we use both - we have two distinct sigreturn vs. rt_sigreturn syscalls (see also the distinction in `stepThroughSigreturn` below), and there's a VDSO trampoline for each of them.


================
Comment at: libunwind/src/UnwindCursor.hpp:2684
+int UnwindCursor<A, R>::stepThroughSigReturn(Registers_s390x &) {
+  // Determine current SP and CFA (offset by 160).
+  const pint_t sp = static_cast<pint_t>(this->getReg(UNW_REG_SP));
----------------
MaskRay wrote:
> Best to leave a comment where 160 comes from. It can be a kernel source filename.
This is a general property of the s390x ABI (the CFA is always at "incoming SP + 160").  I'll add a comment to that effect.


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

https://reviews.llvm.org/D124765