diff mbox

PATCH [8/n]: Prepare x32: PR other/48007: Unwind library doesn't work with UNITS_PER_WORD > sizeof (void *)

Message ID 20110625161357.GA5401@intel.com
State New
Headers show

Commit Message

H.J. Lu June 25, 2011, 4:13 p.m. UTC 
Hi,

This patch introduces UNIQUE_UNWIND_CONTEXT and properly saves/stores
registers with UNITS_PER_WORD > sizeof (void *) as suggested in

http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01526.html

OK for trunk?

Thanks.

H.J.
---
2011-04-09  H.J. Lu  <hongjiu.lu@intel.com>

	PR other/48007
	* unwind-dw2.c (UNIQUE_UNWIND_CONTEXT): New.
	(_Unwind_Context): If UNIQUE_UNWIND_CONTEXT is defined, add
	dwarf_reg_size_table and value, remove version and by_value.
	(EXTENDED_CONTEXT_BIT): Don't define if UNIQUE_UNWIND_CONTEXT
	is defined.
	(_Unwind_IsExtendedContext): Likewise.
	(_Unwind_GetGR): Support UNIQUE_UNWIND_CONTEXT.
	(_Unwind_SetGR): Likewise.
	(_Unwind_GetGRPtr): Likewise.
	(_Unwind_SetGRPtr): Likewise.
	(_Unwind_SetGRValue): Likewise.
	(_Unwind_GRByValue): Likewise.
	(__frame_state_for): Initialize dwarf_reg_size_table field if
	UNIQUE_UNWIND_CONTEXT is defined.
	(uw_install_context_1): Likewise.  Support UNIQUE_UNWIND_CONTEXT.

Comments

Joseph Myers June 25, 2011, 8:19 p.m. UTC | #1 
On Sat, 25 Jun 2011, H.J. Lu wrote:

> +#ifndef UNIQUE_UNWIND_CONTEXT

The use of #ifndef seems to imply that this is a target macro, to be 
defined in libgcc_tm.h.  In that case it should be documented, and 
poisoned in system.h under the "only used for code built for the target" 
case, and this:

> +#if defined __x86_64 && !defined __LP64__

is inappropriate since you should instead put it in an appropriate header 
in libgcc/config/, rather than hardcoding an architecture-specific #if in 
an architecture-independent file.
Jason Merrill June 26, 2011, 7:54 p.m. UTC | #2 
On 06/25/2011 12:13 PM, H.J. Lu wrote:
> This patch introduces UNIQUE_UNWIND_CONTEXT

Where is this documented?  The ABI document doesn't seem to say anything 
about it.

Jason
H.J. Lu June 26, 2011, 8:06 p.m. UTC | #3 
On Sun, Jun 26, 2011 at 12:54 PM, Jason Merrill <jason@redhat.com> wrote:
> On 06/25/2011 12:13 PM, H.J. Lu wrote:
>>
>> This patch introduces UNIQUE_UNWIND_CONTEXT
>
> Where is this documented?  The ABI document doesn't seem to say anything
> about it.
>

I added a short paragraph in the updated patch:

http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01922.html

+@defmac UNIQUE_UNWIND_CONTEXT
+
+Define this macro if the target only supports single unqiue unwind
+context.  The default is to support multiple unwind contexts.
+
+@end defmac

This is the implementation detail and isn't required by the ABI.
With the unique unwind context, we can update the unwind
context format without breaking binary compatibility.

Thanks.
Jason Merrill June 26, 2011, 9:45 p.m. UTC | #4 
On 06/26/2011 04:06 PM, H.J. Lu wrote:
> I added a short paragraph in the updated patch:
>
> +@defmac UNIQUE_UNWIND_CONTEXT
> +
> +Define this macro if the target only supports single unqiue unwind
> +context.  The default is to support multiple unwind contexts.

I saw that.  I don't know what it means.

Jason
H.J. Lu June 26, 2011, 9:58 p.m. UTC | #5 
On Sun, Jun 26, 2011 at 2:45 PM, Jason Merrill <jason@redhat.com> wrote:
> On 06/26/2011 04:06 PM, H.J. Lu wrote:
>>
>> I added a short paragraph in the updated patch:
>>
>> +@defmac UNIQUE_UNWIND_CONTEXT
>> +
>> +Define this macro if the target only supports single unqiue unwind
>> +context.  The default is to support multiple unwind contexts.
>
> I saw that.  I don't know what it means.
>

The current unwind library scheme provides only one unwind
context and is backward compatible with multiple different unwind
contexts from multiple unwind libraries:

http://gcc.gnu.org/ml/gcc-patches/2006-12/msg01769.html

My patch fixes UNITS_PER_WORD > sizeof (void *) and
enforces single unwind context when backward compatibility
isn't needed.
Jason Merrill June 27, 2011, 2:58 p.m. UTC | #6 
On 06/26/2011 05:58 PM, H.J. Lu wrote:
> The current unwind library scheme provides only one unwind
> context and is backward compatible with multiple different unwind
> contexts from multiple unwind libraries:
>
> http://gcc.gnu.org/ml/gcc-patches/2006-12/msg01769.html
>
> My patch fixes UNITS_PER_WORD > sizeof (void *) and
> enforces single unwind context when backward compatibility
> isn't needed.

OK, there seem to be two things going on in this patch:

1) Handle registers larger than pointers.
2) Require that all code share a single copy of the unwinder.

For #2, how are you avoiding the issues Jakub describes in that message? 
  Isn't his scenario 2 still possible?  Are you deciding that it's 
better to abort at run-time in that case?

It seems to me that for targets newer than Jakub's patch we can 
hard-wire _Unwind_IsExtendedContext to true, but making further 
assumptions would be a mistake.

Then, if we're still trying to handle versioning, I think your earlier 
patch for #1 (r170716) that just changes the type of the reg array is a 
better way to go.  But that change should be dependent on a target macro 
to avoid ABI changes for existing targets.

Jason
diff mbox

Patch

diff --git a/gcc/unwind-dw2.c b/gcc/unwind-dw2.c
index 25990b4..5fa2723 100644
--- a/gcc/unwind-dw2.c
+++ b/gcc/unwind-dw2.c
@@ -59,6 +59,12 @@ 
 #define DWARF_REG_TO_UNWIND_COLUMN(REGNO) (REGNO)
 #endif
 
+#ifndef UNIQUE_UNWIND_CONTEXT
+#if defined __x86_64 && !defined __LP64__
+# define UNIQUE_UNWIND_CONTEXT
+#endif
+#endif
+
 /* This is the register and unwind state for a particular frame.  This
    provides the information necessary to unwind up past a frame and return
    to its caller.  */
@@ -69,6 +75,15 @@  struct _Unwind_Context
   void *ra;
   void *lsda;
   struct dwarf_eh_bases bases;
+#ifdef UNIQUE_UNWIND_CONTEXT
+  /* Used to check for unique _Unwind_Context.  */
+  void *dwarf_reg_size_table;
+  /* Signal frame context.  */
+#define SIGNAL_FRAME_BIT ((_Unwind_Word) 1 >> 0)
+  _Unwind_Word flags;
+  _Unwind_Word args_size;
+  _Unwind_Word value[DWARF_FRAME_REGISTERS+1];
+#else
   /* Signal frame context.  */
 #define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1)
   /* Context which has version/args_size/by_value fields.  */
@@ -79,6 +94,7 @@  struct _Unwind_Context
   _Unwind_Word version;
   _Unwind_Word args_size;
   char by_value[DWARF_FRAME_REGISTERS+1];
+#endif
 };
 
 /* Byte size of every register managed by these routines.  */
@@ -144,11 +160,13 @@  _Unwind_SetSignalFrame (struct _Unwind_Context *context, int val)
     context->flags &= ~SIGNAL_FRAME_BIT;
 }
 
+#ifndef UNIQUE_UNWIND_CONTEXT
 static inline _Unwind_Word
 _Unwind_IsExtendedContext (struct _Unwind_Context *context)
 {
   return context->flags & EXTENDED_CONTEXT_BIT;
 }
+#endif
 
 /* Get the value of register INDEX as saved in CONTEXT.  */
 
@@ -168,8 +186,14 @@  _Unwind_GetGR (struct _Unwind_Context *context, int index)
   size = dwarf_reg_size_table[index];
   ptr = context->reg[index];
 
+#ifdef UNIQUE_UNWIND_CONTEXT
+  gcc_assert (context->dwarf_reg_size_table == &dwarf_reg_size_table);
+  if (context->reg[index] == &context->value[index])
+    return context->value[index];
+#else
   if (_Unwind_IsExtendedContext (context) && context->by_value[index])
     return (_Unwind_Word) (_Unwind_Internal_Ptr) ptr;
+#endif
 
   /* This will segfault if the register hasn't been saved.  */
   if (size == sizeof(_Unwind_Ptr))
@@ -207,11 +231,20 @@  _Unwind_SetGR (struct _Unwind_Context *context, int index, _Unwind_Word val)
   gcc_assert (index < (int) sizeof(dwarf_reg_size_table));
   size = dwarf_reg_size_table[index];
 
+#ifdef UNIQUE_UNWIND_CONTEXT
+  gcc_assert (context->dwarf_reg_size_table == &dwarf_reg_size_table);
+  if (context->reg[index] == &context->value[index])
+    {
+      context->value[index] = val;
+      return;
+    }
+#else
   if (_Unwind_IsExtendedContext (context) && context->by_value[index])
     {
       context->reg[index] = (void *) (_Unwind_Internal_Ptr) val;
       return;
     }
+#endif
 
   ptr = context->reg[index];
 
@@ -230,8 +263,10 @@  static inline void *
 _Unwind_GetGRPtr (struct _Unwind_Context *context, int index)
 {
   index = DWARF_REG_TO_UNWIND_COLUMN (index);
+#ifndef UNIQUE_UNWIND_CONTEXT
   if (_Unwind_IsExtendedContext (context) && context->by_value[index])
     return &context->reg[index];
+#endif
   return context->reg[index];
 }
 
@@ -241,8 +276,10 @@  static inline void
 _Unwind_SetGRPtr (struct _Unwind_Context *context, int index, void *p)
 {
   index = DWARF_REG_TO_UNWIND_COLUMN (index);
+#ifndef UNIQUE_UNWIND_CONTEXT
   if (_Unwind_IsExtendedContext (context))
     context->by_value[index] = 0;
+#endif
   context->reg[index] = p;
 }
 
@@ -254,10 +291,15 @@  _Unwind_SetGRValue (struct _Unwind_Context *context, int index,
 {
   index = DWARF_REG_TO_UNWIND_COLUMN (index);
   gcc_assert (index < (int) sizeof(dwarf_reg_size_table));
+#ifdef UNIQUE_UNWIND_CONTEXT
+  gcc_assert (dwarf_reg_size_table[index] == sizeof (_Unwind_Word));
+  context->value[index] = val;
+  context->reg[index] = &context->value[index];
+#else
   gcc_assert (dwarf_reg_size_table[index] == sizeof (_Unwind_Ptr));
-
   context->by_value[index] = 1;
   context->reg[index] = (void *) (_Unwind_Internal_Ptr) val;
+#endif
 }
 
 /* Return nonzero if register INDEX is stored by value rather than
@@ -267,7 +309,11 @@  static inline int
 _Unwind_GRByValue (struct _Unwind_Context *context, int index)
 {
   index = DWARF_REG_TO_UNWIND_COLUMN (index);
+#ifdef UNIQUE_UNWIND_CONTEXT
+  return context->reg[index] == &context->value[index];
+#else
   return context->by_value[index];
+#endif
 }
 
 /* Retrieve the return address for CONTEXT.  */
@@ -1217,7 +1263,11 @@  __frame_state_for (void *pc_target, struct frame_state *state_in)
   int reg;
 
   memset (&context, 0, sizeof (struct _Unwind_Context));
+#ifdef UNIQUE_UNWIND_CONTEXT
+  context.dwarf_reg_size_table = &dwarf_reg_size_table;
+#else
   context.flags = EXTENDED_CONTEXT_BIT;
+#endif
   context.ra = pc_target + 1;
 
   if (uw_frame_state_for (&context, &fs) != _URC_NO_REASON)
@@ -1455,7 +1505,11 @@  uw_init_context_1 (struct _Unwind_Context *context,
 
   memset (context, 0, sizeof (struct _Unwind_Context));
   context->ra = ra;
+#ifdef UNIQUE_UNWIND_CONTEXT
+  context->dwarf_reg_size_table = &dwarf_reg_size_table;
+#else
   context->flags = EXTENDED_CONTEXT_BIT;
+#endif
 
   code = uw_frame_state_for (context, &fs);
   gcc_assert (code == _URC_NO_REASON);
@@ -1537,8 +1591,13 @@  uw_install_context_1 (struct _Unwind_Context *current,
       void *c = current->reg[i];
       void *t = target->reg[i];
 
+#ifdef UNIQUE_UNWIND_CONTEXT
+      gcc_assert (current->reg[i] != &current->value[i]);
+      if (target->reg[i] == &target->value[i] && c)
+#else
       gcc_assert (current->by_value[i] == 0);
       if (target->by_value[i] && c)
+#endif
 	{
 	  _Unwind_Word w;
 	  _Unwind_Ptr p;