diff mbox

[RFC,4.8] use ip+cfa to identify unwind frames, if possible

Message ID 4F3D7F5D.1080605@redhat.com
State New
Headers show

Commit Message

Richard Henderson Feb. 16, 2012, 10:12 p.m. UTC 
On 02/16/2012 11:54 AM, Richard Henderson wrote:
> I'll work on a proper fix to the unwinder; we can then decide
> whether and when to apply it...

Something like this.  Not the best of solutions, but we've run out
of bits in the exception structure, and thus we can't use this 
solution universally.

It shouldn't matter for most CISC targets, like x86, which always
push the return address onto the stack; one never winds up with
the situation you saw on mips where the return address is held 
only in a (special) register.


r~

Comments

David Daney Feb. 16, 2012, 10:43 p.m. UTC | #1 
On 02/16/2012 02:12 PM, Richard Henderson wrote:
[...]
> index 1c19f8b..59d4560 100644
> --- a/gcc/config/mips/mips.h
> +++ b/gcc/config/mips/mips.h
> @@ -2920,3 +2920,15 @@ extern GTY(()) struct target_globals *mips16_globals;
>      with arguments ARGS.  */
>   #define PMODE_INSN(NAME, ARGS) \
>     (Pmode == SImode ? NAME ## _si ARGS : NAME ## _di ARGS)
> +
> +/* For mips32 mode we have bits 0 and 1 zero free, but for mips16 mode,
> +   bit 0 indicates mips16 mode, and bit 1 is thence meaningful.  Thus
> +   the only "free" bits would be at the top of the address space.
> +   Can we trust that we'll never try to unwind in kernel mode?  */

That's too bad.  I guess if we ever want to unwind in kernel mode, we 
can say no mips16 and switch it to a low-order bit for that application. 
  Or write our own unwinder.

David Daney
Richard Sandiford Feb. 16, 2012, 11:32 p.m. UTC | #2 
David Daney <david.daney@cavium.com> writes:
> On 02/16/2012 02:12 PM, Richard Henderson wrote:
> [...]

Thanks for the patch.

>> index 1c19f8b..59d4560 100644
>> --- a/gcc/config/mips/mips.h
>> +++ b/gcc/config/mips/mips.h
>> @@ -2920,3 +2920,15 @@ extern GTY(()) struct target_globals *mips16_globals;
>>      with arguments ARGS.  */
>>   #define PMODE_INSN(NAME, ARGS) \
>>     (Pmode == SImode ? NAME ## _si ARGS : NAME ## _di ARGS)
>> +
>> +/* For mips32 mode we have bits 0 and 1 zero free, but for mips16 mode,
>> +   bit 0 indicates mips16 mode, and bit 1 is thence meaningful.  Thus
>> +   the only "free" bits would be at the top of the address space.
>> +   Can we trust that we'll never try to unwind in kernel mode?  */
>
> That's too bad.  I guess if we ever want to unwind in kernel mode, we 
> can say no mips16 and switch it to a low-order bit for that application. 
>   Or write our own unwinder.

I suppose the problem is that baremetal often runs in kernel mode.
(Normal mips*-elf gdbsim testing works that way.)

But I think we'd be fine if we restrict the IP matching to non-MIPS16 mode.
GCC doesn't have any other special RA save registers, and now's a good time
to say that such a thing won't be allowed in MIPS16 or microMIPS code
(or in anything, probably).

So maybe we could set private_1 to (IP | 2) when (IP & 3) == 0,
and leave it at 0 otherwise.  It's then a forced unwind unless
(IP & 3) == 2.  Not as elegant as the single bit though.

Richard
Joseph Myers Feb. 16, 2012, 11:58 p.m. UTC | #3 
On Thu, 16 Feb 2012, Richard Henderson wrote:

> On 02/16/2012 11:54 AM, Richard Henderson wrote:
> > I'll work on a proper fix to the unwinder; we can then decide
> > whether and when to apply it...
> 
> Something like this.  Not the best of solutions, but we've run out
> of bits in the exception structure, and thus we can't use this 
> solution universally.

The new target macro UNWIND_FORCE_UNWIND_MASK in this patch

* needs to be documented;

* is only used in code in libgcc so should be in libgcc_tm.h via 
libgcc/config.host, not in the host-side gcc/config/ headers.  
(<http://gcc.gnu.org/wiki/Top-Level_Libgcc_Migration> lists existing 
target macros that have yet to be transitioned to libgcc_tm.h, though 
various parts of that page describe projects Rainer has now done.)
David Daney Feb. 17, 2012, 5:31 p.m. UTC | #4 
On 02/16/2012 03:32 PM, Richard Sandiford wrote:
> David Daney<david.daney@cavium.com>  writes:
>> On 02/16/2012 02:12 PM, Richard Henderson wrote:
>> [...]
>
> Thanks for the patch.
>
>>> index 1c19f8b..59d4560 100644
>>> --- a/gcc/config/mips/mips.h
>>> +++ b/gcc/config/mips/mips.h
>>> @@ -2920,3 +2920,15 @@ extern GTY(()) struct target_globals *mips16_globals;
>>>       with arguments ARGS.  */
>>>    #define PMODE_INSN(NAME, ARGS) \
>>>      (Pmode == SImode ? NAME ## _si ARGS : NAME ## _di ARGS)
>>> +
>>> +/* For mips32 mode we have bits 0 and 1 zero free, but for mips16 mode,
>>> +   bit 0 indicates mips16 mode, and bit 1 is thence meaningful.  Thus
>>> +   the only "free" bits would be at the top of the address space.
>>> +   Can we trust that we'll never try to unwind in kernel mode?  */
>>
>> That's too bad.  I guess if we ever want to unwind in kernel mode, we
>> can say no mips16 and switch it to a low-order bit for that application.
>>    Or write our own unwinder.
>
> I suppose the problem is that baremetal often runs in kernel mode.
> (Normal mips*-elf gdbsim testing works that way.)
>
> But I think we'd be fine if we restrict the IP matching to non-MIPS16 mode.
> GCC doesn't have any other special RA save registers, and now's a good time
> to say that such a thing won't be allowed in MIPS16 or microMIPS code
> (or in anything, probably).
>
> So maybe we could set private_1 to (IP | 2) when (IP&  3) == 0,
> and leave it at 0 otherwise.  It's then a forced unwind unless
> (IP&  3) == 2.  Not as elegant as the single bit though.
>

Just off the top of my head (with out actually looking at the code).  Is 
there anything that could be done with the register zero save slot (if 
it even exists)?


> Richard
>
>
Richard Henderson Feb. 17, 2012, 5:42 p.m. UTC | #5 
On 02/17/12 09:31, David Daney wrote:
> Just off the top of my head (with out actually looking at the code).  Is there anything that could be done with the register zero save slot (if it even exists)?

No.  We're talking about saving information in the exception structure between unwindings.


r~
diff mbox

Patch

diff --git a/gcc/config/alpha/alpha.h b/gcc/config/alpha/alpha.h
index 07ffa9f..77e1ee5 100644
--- a/gcc/config/alpha/alpha.h
+++ b/gcc/config/alpha/alpha.h
@@ -1290,3 +1290,8 @@  do {							\
 
 /* The system headers under Alpha systems are generally C++-aware.  */
 #define NO_IMPLICIT_EXTERN_C
+
+/* Bits 0 and 1 are always zero in jump targets.  */
+#ifdef IN_LIBGCC2
+# define UNWIND_FORCE_UNWIND_MASK 1
+#endif
diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index 1c19f8b..59d4560 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -2920,3 +2920,15 @@  extern GTY(()) struct target_globals *mips16_globals;
    with arguments ARGS.  */
 #define PMODE_INSN(NAME, ARGS) \
   (Pmode == SImode ? NAME ## _si ARGS : NAME ## _di ARGS)
+
+/* For mips32 mode we have bits 0 and 1 zero free, but for mips16 mode,
+   bit 0 indicates mips16 mode, and bit 1 is thence meaningful.  Thus
+   the only "free" bits would be at the top of the address space.
+   Can we trust that we'll never try to unwind in kernel mode?  */
+#ifdef IN_LIBGCC2
+# if TARGET_64BIT
+#  define UNWIND_FORCE_UNWIND_MASK	0x8000000000000000ull
+# else
+#  define UNWIND_FORCE_UNWIND_MASK	0x80000000u
+# endif
+#endif
diff --git a/libgcc/unwind-sjlj.c b/libgcc/unwind-sjlj.c
index 1fc1c5d..4b32328 100644
--- a/libgcc/unwind-sjlj.c
+++ b/libgcc/unwind-sjlj.c
@@ -322,6 +322,9 @@  uw_identify_context (struct _Unwind_Context *context)
 #define _Unwind_Resume			_Unwind_SjLj_Resume
 #define _Unwind_Resume_or_Rethrow	_Unwind_SjLj_Resume_or_Rethrow
 
+/* Always use uw_identify_context to identify frames.  */
+#undef UNWIND_FORCE_UNWIND_MASK
+
 #include "unwind.inc"
 
 #endif /* USING_SJLJ_EXCEPTIONS */
diff --git a/libgcc/unwind.inc b/libgcc/unwind.inc
index 5e2ec29..1374f9d 100644
--- a/libgcc/unwind.inc
+++ b/libgcc/unwind.inc
@@ -1,5 +1,5 @@ 
 /* Exception handling and frame unwind runtime interface routines. -*- C -*-
-   Copyright (C) 2001, 2003, 2008, 2009 Free Software Foundation, Inc.
+   Copyright (C) 2001, 2003, 2008, 2009, 2012 Free Software Foundation, Inc.
 
    This file is part of GCC.
 
@@ -27,6 +27,64 @@ 
    This file is included from unwind-dw2.c, unwind-sjlj.c or
    unwind-ia64.c.  */
 
+/* This is a bit value within a function pointer or function return address
+   that is never set.  E.g. most RISC targets have instructions aligned to
+   four bytes, meaning bits 0 and 1 are "free" and thus either 1 or 2 would
+   be appropriate values to use here.
+
+   If non-zero, we will use this bit to distinguish forced unwind from normal
+   unwind, and in addition use both the IP+CFA to identify the target context
+   during unwinding.
+
+   If zero, i.e. there is no "free" bit, then we will only be able to use the
+   CFA to identify the target context during unwinding.  The consequence of
+   this is that we cannot reliably propagate an exception through a sequence
+   of functions that do not have a stack frame.  Normally this is not a
+   problem, in that most functions that do not have stack frames are leaf
+   functions, and thus do not throw exceptions.  */
+
+#include <stdbool.h>
+
+#ifndef UNWIND_FORCE_UNWIND_MASK
+# define UNWIND_FORCE_UNWIND_MASK 0
+#endif
+
+static inline bool
+is_forced_unwind (struct _Unwind_Exception *exc)
+{
+  if (UNWIND_FORCE_UNWIND_MASK != 0)
+    return (exc->private_1 & UNWIND_FORCE_UNWIND_MASK) != 0;
+  else
+    return exc->private_1 != 0;
+}
+
+static inline bool
+is_handler_match (struct _Unwind_Exception *exc,
+		  struct _Unwind_Context *context)
+{
+  if (UNWIND_FORCE_UNWIND_MASK != 0)
+    return (_Unwind_GetIP (context) == exc->private_1
+	    && _Unwind_GetCFA (context) == exc->private_2);
+  else
+    return uw_identify_context (context) == exc->private_2;
+}
+
+static inline void
+record_handler_match (struct _Unwind_Exception *exc,
+		      struct _Unwind_Context *context)
+{
+  if (UNWIND_FORCE_UNWIND_MASK != 0)
+    {
+      exc->private_1 = _Unwind_GetIP (context);
+      exc->private_2 = _Unwind_GetCFA (context);
+    }
+  else
+    {
+      exc->private_1 = 0;
+      exc->private_2 = uw_identify_context (context);
+    }
+}
+
 /* Subroutine of _Unwind_RaiseException also invoked from _Unwind_Resume. 
 
    Unwind the stack calling the personality routine to find both the
@@ -48,8 +106,7 @@  _Unwind_RaiseException_Phase2(struct _Unwind_Exception *exc,
       code = uw_frame_state_for (context, &fs);
 
       /* Identify when we've reached the designated handler context.  */
-      match_handler = (uw_identify_context (context) == exc->private_2
-		       ? _UA_HANDLER_FRAME : 0);
+      match_handler = is_handler_match (exc, context) ? _UA_HANDLER_FRAME : 0;
 
       if (code != _URC_NO_REASON)
 	/* Some error encountered.  Usually the unwinder doesn't
@@ -124,8 +181,7 @@  _Unwind_RaiseException(struct _Unwind_Exception *exc)
 
   /* Indicate to _Unwind_Resume and associated subroutines that this
      is not a forced unwind.  Further, note where we found a handler.  */
-  exc->private_1 = 0;
-  exc->private_2 = uw_identify_context (&cur_context);
+  record_handler_match (exc, &cur_context);
 
   cur_context = this_context;
   code = _Unwind_RaiseException_Phase2 (exc, &cur_context);
@@ -142,7 +198,8 @@  static _Unwind_Reason_Code
 _Unwind_ForcedUnwind_Phase2 (struct _Unwind_Exception *exc,
 			     struct _Unwind_Context *context)
 {
-  _Unwind_Stop_Fn stop = (_Unwind_Stop_Fn) (_Unwind_Ptr) exc->private_1;
+  _Unwind_Stop_Fn stop = (_Unwind_Stop_Fn)
+    (_Unwind_Ptr) (exc->private_1 & ~UNWIND_FORCE_UNWIND_MASK);
   void *stop_argument = (void *) (_Unwind_Ptr) exc->private_2;
   _Unwind_Reason_Code code, stop_code;
 
@@ -201,7 +258,7 @@  _Unwind_ForcedUnwind (struct _Unwind_Exception *exc,
   uw_init_context (&this_context);
   cur_context = this_context;
 
-  exc->private_1 = (_Unwind_Ptr) stop;
+  exc->private_1 = (_Unwind_Ptr) stop | UNWIND_FORCE_UNWIND_MASK;
   exc->private_2 = (_Unwind_Ptr) stop_argument;
 
   code = _Unwind_ForcedUnwind_Phase2 (exc, &cur_context);
@@ -226,10 +283,10 @@  _Unwind_Resume (struct _Unwind_Exception *exc)
 
   /* Choose between continuing to process _Unwind_RaiseException
      or _Unwind_ForcedUnwind.  */
-  if (exc->private_1 == 0)
-    code = _Unwind_RaiseException_Phase2 (exc, &cur_context);
-  else
+  if (is_forced_unwind (exc))
     code = _Unwind_ForcedUnwind_Phase2 (exc, &cur_context);
+  else
+    code = _Unwind_RaiseException_Phase2 (exc, &cur_context);
 
   gcc_assert (code == _URC_INSTALL_CONTEXT);
 
@@ -248,7 +305,7 @@  _Unwind_Resume_or_Rethrow (struct _Unwind_Exception *exc)
 
   /* Choose between continuing to process _Unwind_RaiseException
      or _Unwind_ForcedUnwind.  */
-  if (exc->private_1 == 0)
+  if (!is_forced_unwind (exc))
     return _Unwind_RaiseException (exc);
 
   uw_init_context (&this_context);