public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/vendors/ARM/heads/morello)] Handle split stacks in unwinder
@ 2022-11-02 14:53 Matthew Malcomson
  0 siblings, 0 replies; only message in thread
From: Matthew Malcomson @ 2022-11-02 14:53 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:ef254a8c7f201389ff7963bacb84254c61a2053e

commit ef254a8c7f201389ff7963bacb84254c61a2053e
Author: Matthew Malcomson <matthew.malcomson@arm.com>
Date:   Wed Nov 2 14:51:17 2022 +0000

    Handle split stacks in unwinder
    
    A signal handler can be registered with sigaction flags including SA_ONSTACK.
    This flag indicates that the signal handler should be created on a different
    stack to the current thread.
    
    The mechanism by which the unwinder adjusts its stack pointer is by receiving a
    delta between the current CFA and the target CFA, then adding that offset to the
    stack pointer once the existing frame has been destroyed.
    When the unwinder was called on one stack and needs to unwind to another stack
    this addition of an offset gives us an invalid capability.
    
    This patch resolves that by getting the unwinder to communicate to the epilogue
    the new stack pointer that we want to end up with (rather than the offset
    between the new stack pointer and the existing CFA).  The epilogue then uses
    that value as the new stack pointer instead of adding an offset.
    
    This means changing some generic code and the interface between generic code and
    target code.  Such a change would break existing target code that does not get
    updated.  While we have not been ensuring that other targets stay functional
    with CHERI changes, we still do not want to knowingly break them.  Hence here we
    choose to only use the new interface with purecap code.
    
    I believe that changing the interface of this builtin is OK to do since it is
    not documented and hence I believe it is only used by our runtime (which has
    now been updated).
    
    The adjustments that we make to the interface are:
      - __builtin_eh_return now takes an unwind_word type rather than the type for
        an offset_mode (ptr_mode) mode.
        (On non-purecap code this simply means a different integer -- usually of the
        same size, but sometimes larger.  On purecap code this means a uintcap_t).
      - For purecap only __builtin_eh_return now takes "the target value for csp"
        rather than "the offset between the current frames CFA and the target stack
        pointer".
      - For purecap only, the backend now receives "the target value for csp" rather
        than "the offset to be applied to csp after resetting to CFA" in
        EH_RETURN_STACKADJ_RTX.  (And hence this RTX needs to be in Pmode rather
        than POmode for purecap targets).
    
    This fixes tests cleanup-10 and cleanup-11 in gcc.dg and g++.dg.  It also gets
    nptl/tst-cancelx21 passing (on top of an earlier unwinder patch).
    
    N.b. this patch leaves an unnecessary instruction in the epilogue of functions
    which use `__builtin_eh_return`.  That is the instruction which moves the stack
    pointer up to the CFA for the current function.  Before this patch that was
    required since the unwinder provided the offset between the current functions
    CFA and the target CFA so the epilogue needed to adjust the stack pointer both
    by this offset and by the amount to get to the current functions CFA.
    Now it is not necessary, but we leave it in for simplicity of this patch.

Diff:
---
 gcc/builtin-types.def                        |  2 ++
 gcc/builtins.def                             |  2 +-
 gcc/config/aarch64/aarch64.c                 | 10 ++++++++++
 gcc/config/aarch64/aarch64.h                 |  2 +-
 gcc/except.c                                 |  9 ++++++---
 gcc/testsuite/gcc.target/aarch64/eh_return.c |  8 ++++++++
 libgcc/unwind-dw2.c                          | 18 +++++++++++++++---
 7 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/gcc/builtin-types.def b/gcc/builtin-types.def
index ff6bc6effd8..6af0d78274b 100644
--- a/gcc/builtin-types.def
+++ b/gcc/builtin-types.def
@@ -412,6 +412,8 @@ DEF_FUNCTION_TYPE_2 (BT_FN_ULONG_PTR_ULONG,
 		     BT_ULONG, BT_PTR, BT_ULONG)
 DEF_FUNCTION_TYPE_2 (BT_FN_VOID_PTRMODE_PTR,
 		     BT_VOID, BT_PTRMODE, BT_PTR)
+DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UNWINDWORD_PTR,
+		     BT_VOID, BT_UNWINDWORD, BT_PTR)
 DEF_FUNCTION_TYPE_2 (BT_FN_VOID_PTR_PTRMODE,
 		     BT_VOID, BT_PTR, BT_PTRMODE)
 DEF_FUNCTION_TYPE_2 (BT_FN_VOID_UINT8_UINT8,
diff --git a/gcc/builtins.def b/gcc/builtins.def
index caa7371cc8f..e65711b6ff9 100644
--- a/gcc/builtins.def
+++ b/gcc/builtins.def
@@ -860,7 +860,7 @@ DEF_EXT_LIB_BUILTIN    (BUILT_IN_DCGETTEXT, "dcgettext", BT_FN_STRING_CONST_STRI
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_DGETTEXT, "dgettext", BT_FN_STRING_CONST_STRING_CONST_STRING, ATTR_FORMAT_ARG_2)
 DEF_GCC_BUILTIN        (BUILT_IN_DWARF_CFA, "dwarf_cfa", BT_FN_PTR, ATTR_NULL)
 DEF_GCC_BUILTIN        (BUILT_IN_DWARF_SP_COLUMN, "dwarf_sp_column", BT_FN_UINT, ATTR_NULL)
-DEF_GCC_BUILTIN        (BUILT_IN_EH_RETURN, "eh_return", BT_FN_VOID_PTRMODE_PTR, ATTR_NORETURN_NOTHROW_LEAF_LIST)
+DEF_GCC_BUILTIN        (BUILT_IN_EH_RETURN, "eh_return", BT_FN_VOID_UNWINDWORD_PTR, ATTR_NORETURN_NOTHROW_LEAF_LIST)
 DEF_GCC_BUILTIN        (BUILT_IN_EH_RETURN_DATA_REGNO, "eh_return_data_regno", BT_FN_INT_INT, ATTR_LEAF_LIST)
 DEF_EXT_LIB_BUILTIN        (BUILT_IN_EXECL, "execl", BT_FN_INT_CONST_STRING_CONST_STRING_VAR, ATTR_SENTINEL_NOTHROW_LIST)
 DEF_EXT_LIB_BUILTIN        (BUILT_IN_EXECLP, "execlp", BT_FN_INT_CONST_STRING_CONST_STRING_VAR, ATTR_SENTINEL_NOTHROW_LIST)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d4be34d1d35..c5829e2b0f7 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -9014,6 +9014,16 @@ aarch64_expand_epilogue (bool for_sibcall)
   /* Stack adjustment for exception handler.  */
   if (crtl->calls_eh_return && !for_sibcall)
     {
+      /* On pure capability targets we may be unwinding over a split stack
+	 (e.g. when a signal handler with SA_ONSTACK has been triggered).
+	 In this case simply adjusting the value with an offset usually takes
+	 the stack capability out of bounds.  On those targets we change the
+	 ABI by which the unwinder communicates with the epilogue of a function
+	 so the unwinder returns a "new stack pointer" to use and the epilogue
+	 uses that.  */
+      if (TARGET_CAPABILITY_PURE)
+	emit_move_insn (stack_pointer_rtx, EH_RETURN_STACKADJ_RTX);
+      else
       /* We need to unwind the stack by the offset computed by
 	 EH_RETURN_STACKADJ_RTX.  We have already reset the CFA
 	 to be SP; letting the CFA move during this adjustment
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 76a4de5aadf..635f1cc0eca 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -688,7 +688,7 @@ extern unsigned aarch64_architecture_version;
 #define ASM_DO_CFI_STARTPROC  aarch64_do_cfi_startproc
 
 /* For EH returns X4 contains the stack adjustment.  */
-#define EH_RETURN_STACKADJ_RTX	gen_rtx_REG (POmode, R4_REGNUM)
+#define EH_RETURN_STACKADJ_RTX	gen_rtx_REG (Pmode, R4_REGNUM)
 #define EH_RETURN_HANDLER_RTX  aarch64_eh_return_handler_rtx ()
 
 /* Don't use __builtin_setjmp until we've defined it.  */
diff --git a/gcc/except.c b/gcc/except.c
index 32f16ef6e78..96953df030b 100644
--- a/gcc/except.c
+++ b/gcc/except.c
@@ -2283,9 +2283,9 @@ expand_builtin_eh_return (tree stackadj_tree ATTRIBUTE_UNUSED,
 #ifdef EH_RETURN_STACKADJ_RTX
   tmp = expand_expr (stackadj_tree, crtl->eh.ehr_stackadj,
 		     VOIDmode, EXPAND_NORMAL);
-  tmp = convert_to_offset_mode (tmp);
+  tmp = convert_memory_address (Pmode, tmp);
   if (!crtl->eh.ehr_stackadj)
-    crtl->eh.ehr_stackadj = copy_to_mode_reg (POmode, tmp);
+    crtl->eh.ehr_stackadj = copy_addr_to_reg (tmp);
   else if (tmp != crtl->eh.ehr_stackadj)
     emit_move_insn (crtl->eh.ehr_stackadj, tmp);
 #endif
@@ -2318,7 +2318,10 @@ expand_eh_return (void)
   crtl->calls_eh_return = 1;
 
 #ifdef EH_RETURN_STACKADJ_RTX
-  emit_move_insn (EH_RETURN_STACKADJ_RTX, const0_rtx);
+  if (CAPABILITY_MODE_P (Pmode))
+    emit_move_insn (EH_RETURN_STACKADJ_RTX, virtual_cfa_rtx);
+  else
+    emit_move_insn (EH_RETURN_STACKADJ_RTX, CONST0_RTX (Pmode));
 #endif
 
   around_label = gen_label_rtx ();
diff --git a/gcc/testsuite/gcc.target/aarch64/eh_return.c b/gcc/testsuite/gcc.target/aarch64/eh_return.c
index 32179488085..fc5077dc14c 100644
--- a/gcc/testsuite/gcc.target/aarch64/eh_return.c
+++ b/gcc/testsuite/gcc.target/aarch64/eh_return.c
@@ -21,7 +21,11 @@ eh1 (void *p, int x)
 {
   void *q = __builtin_alloca (x);
   eh0 (q);
+#ifdef __CHERI_PURE_CAPABILITY__
+  __builtin_eh_return ((unsigned __intcap)__builtin_dwarf_cfa (), p);
+#else
   __builtin_eh_return (0, p);
+#endif
 }
 
 void
@@ -34,7 +38,11 @@ void
 eh2 (void *p)
 {
   eh2a (val, val, val, val, val, val, val, val, p);
+#ifdef __CHERI_PURE_CAPABILITY__
+  __builtin_eh_return ((unsigned __intcap)__builtin_dwarf_cfa (), p);
+#else
   __builtin_eh_return (0, p);
+#endif
 }
 
 
diff --git a/libgcc/unwind-dw2.c b/libgcc/unwind-dw2.c
index 15eb625c201..541c0e5d609 100644
--- a/libgcc/unwind-dw2.c
+++ b/libgcc/unwind-dw2.c
@@ -1713,15 +1713,15 @@ _Unwind_DebugHook (void *cfa __attribute__ ((__unused__)),
 #define uw_install_context(CURRENT, TARGET, FRAMES)			\
   do									\
     {									\
-      long offset = uw_install_context_1 ((CURRENT), (TARGET));		\
+      _Unwind_Word cfaarg = uw_install_context_1 ((CURRENT), (TARGET));	\
       void *handler = __builtin_frob_return_addr ((TARGET)->ra);	\
       _Unwind_DebugHook ((TARGET)->cfa, handler);			\
       _Unwind_Frames_Extra (FRAMES);					\
-      __builtin_eh_return (offset, handler);				\
+      __builtin_eh_return (cfaarg, handler);				\
     }									\
   while (0)
 
-static long
+static _Unwind_Word
 uw_install_context_1 (struct _Unwind_Context *current,
 		      struct _Unwind_Context *target)
 {
@@ -1768,13 +1768,25 @@ uw_install_context_1 (struct _Unwind_Context *current,
 
       target_cfa = _Unwind_GetPtr (target, __builtin_dwarf_sp_column ());
 
+#ifdef __CHERI_PURE_CAPABILITY__
+      /* We return the target CFA (which includes both metadata and value).  */
+      if (__LIBGCC_STACK_GROWS_DOWNWARD__)
+	return (_Unwind_Word)target_cfa + target->args_size;
+      else
+	return (_Unwind_Word)target_cfa - target->args_size;
+#else
       /* We adjust SP by the difference between CURRENT and TARGET's CFA.  */
       if (__LIBGCC_STACK_GROWS_DOWNWARD__)
 	return target_cfa - current->cfa + target->args_size;
       else
 	return current->cfa - target_cfa - target->args_size;
+#endif
     }
+#ifdef __CHERI_PURE_CAPABILITY__
+  return _Unwind_GetGR (current, __builtin_dwarf_sp_column ());
+#else
   return 0;
+#endif
 }
 
 static inline _Unwind_Ptr

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-11-02 14:53 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-02 14:53 [gcc(refs/vendors/ARM/heads/morello)] Handle split stacks in unwinder Matthew Malcomson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).