[libcxx-commits] [PATCH] D155066: [libunwind] Unwind through aarch64/FreeBSD sigreturn frame

[Alexander Richardson via Phabricator via libcxx-commits]

arichardson added inline comments.


================
Comment at: libunwind/src/UnwindCursor.hpp:2907
+    defined(_LIBUNWIND_TARGET_AARCH64)
+template <typename A, typename R>
+bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_arm64 &) {
----------------
this function is not aarch64 specific and I assume would be shared by other architectures that support it. I'd guard this block with _LIBUNWIND_CHECK_FREEBSD_SIGRETURN and `UnwindCursor<A, R>::stepThroughSigReturn(Registers_arm64 &)` with a nested defined(_LIBUNWIND_TARGET_AARCH64) so that other architectures can be added more easily.


================
Comment at: libunwind/src/UnwindCursor.hpp:2908
+template <typename A, typename R>
+bool UnwindCursor<A, R>::setInfoForSigReturn(Registers_arm64 &) {
+  // Look for the sigreturn trampoline.
----------------



================
Comment at: libunwind/src/UnwindCursor.hpp:2920-2922
+      _sigTrampStart = reinterpret_cast<pint_t>(kst.ksigtramp_start);
+      _sigTrampEnd = reinterpret_cast<pint_t>(kst.ksigtramp_end);
+    }
----------------
If the sysctl fails for some reason we would end up calling it every time. How about setting start and end to zero in that case to avoid calling it again? Also probably makes sense to log an error if the sysctl fails.


================
Comment at: libunwind/src/UnwindCursor.hpp:2935-2960
+
+template <typename A, typename R>
+int UnwindCursor<A, R>::stepThroughSigReturn(Registers_arm64 &) {
+  // In the signal trampoline frame, sp points to a sigframe
+  const pint_t kOffsetSpToSigcontext =
+      offsetof(struct sigframe, sf_uc) + offsetof(ucontext_t, uc_mcontext);
+
----------------
Maybe something like this for future extensibility?


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

https://reviews.llvm.org/D155066