public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix _Unwind_Context backward compatibility issues
@ 2006-12-28 20:46 Jakub Jelinek
  2006-12-29  6:54 ` [PATCH]: for Score backend liqin
  2007-01-03 18:07 ` [PATCH] Fix _Unwind_Context backward compatibility issues Richard Henderson
  0 siblings, 2 replies; 5+ messages in thread
From: Jakub Jelinek @ 2006-12-28 20:46 UTC (permalink / raw)
  To: gcc-patches

Hi!

The
http://gcc.gnu.org/ml/gcc-patches/2006-02/msg01986.html
http://gcc.gnu.org/ml/gcc-patches/2006-03/msg00208.html
patches added 2 new fields at the end of struct _Unwind_Context.
While that structure is only defined inside unwind-dw2.c, unfortunately
it seems to be part of the ABI.  That's because if some application
or library has its own copy of some older version of GCC unwinder
linked into it, the two unwinders don't play together nicely.
It is arguably a bug to provide its own unwinder (except for statically
linked apps), but unfortunately it seems quite a common bug.
With older GCCs (before --as-needed was used in specs), all that
was needed was link some C++ program or shared library with
gcc -o foo *.o -lstdc++
rather than
g++ -o foo *.o
and with very old GCCs the libgcc_eh.a routines weren't even hidden
in the executable.  With contemporary GCCs one has to try harder,
either use -static-libgcc switch or link with explicit -lgcc_eh (one
example of the latter is e.g. libwvstreams).
The scenarios in which things break:
1) unwinder in the executable, exported from it (i.e. before it was
made .hidden in libgcc_eh.a), links against shared libstdc++.so and
that links against shared libgcc_s.so.  If something calls say
_Unwind_RaiseException, it will be resolved to the copy in the executable,
because it comes earlier in the search path.  The old _Unwind_RaiseException
sets up 2 struct _Unwind_Context variables, with the old layout (i.e.
args_size as the last member).  During unwinding __gxx_personality_v0
in the shared libstdc++.so is called and that calls _Unwind_GetIPInfo.
A pointer to the small old struct _Unwind_Context in old
_Unwind_RaiseException's stack frame is passed through.
As this is a new function in GCC 4.2, it won't be in the unwinder in
the executable, so the call jumps to _Unwind_GetIPInfo in GCC 4.2+
libgcc_s.so.  But, this routine looks at context->signal_frame,
i.e. depends on some uninitialized byte on the stack.
2) unwinder in the executable, not exported from it (slightly newer
version, already .hidden), links against shared libstdc++.so which
links against shared libgcc_s.so.  When an exception is thrown
(__cxa_throw), first the libgcc_s.so unwinder is used, but if there
is some destructor within the executable, we can then call _Unwind_Resume
hidden in the executable.  This again sets up a small _Unwind_Context,
then during unwinding calls __gxx_personality_v0.  This can call
_Unwind_SetGR to set the unwind arguments, in all cases passing around
pointer to the small _Unwind_Context.  But, as _Unwind_SetGR from the
executable's unwinder is hidden in the executable, it calls _Unwind_SetGR
in libgcc_s.so.  This looks at context->by_value, thus again depends
on uninitialized stack values.  If non-zero happens to be there,
it can set context->regs[i] directly to the value rather than
store the value to *context->regs[i].  When in uw_install_context_1
of the executable's unwinder the program will likely crash though,
because it expects context->regs[i] to be a valid pointer.

Unfortunately, the old _Unwind_Context did not contain any zero initialized
pad values nor any version field which would allow subsequent field
additions.
The following patch tries to stay backwards compatible with 99.999%
of old programs out there.  It will not work with functions on 32-bit
arches which push 1GB or more of arguments to one function, use
DW_CFA_GNU_args_size for that function and at least one other unwinder
is used in the application in addition to libgcc_s.so.1's unwinder.
I guess that is extremely rare to non-existent.
args_size field is always used only within the same unwinder
(set up in uw_frame_state_for and/or static functions it calls,
read in uw_install_context_1, again static function called from within
the same _Unwind_{RaiseException,ForcedUnwind,Resume}.

The reason I'm not just using the topmost args_size bit for
EXTENDED_CONTEXT_BIT is that we (Red Hat) unfortunately used the
topmost bit in RHEL 4.4 and RHEL 3.8 when we found the signal_frame
new field didn't fly.  I'm terribly sorry about that.
At that time we thought GCC 4.2+ doesn't need to worry about this,
after all apps that have their own unwinder in addition to libgcc_s's
are broken (e.g. when libc or some other library starts using new
DW_CFA_* opcodes, the old statically linked unwinders are out of luck),
but we didn't know about scenario 2) at that point and we didn't know
how many buggy apps are out there.

Perhaps to be even more compatible even with older GCC 4.2 (plus
all the 4.1 branches where this has been backported) unwinders,
_Unwind_Context could be:

struct _Unwind_Context
{
  void *reg[DWARF_FRAME_REGISTERS+1];
  void *cfa;
  void *ra;
  void *lsda;
  struct dwarf_eh_bases bases;
  /* Signal frame context.  */
#define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1)
  /* Context which has version/args_size/by_value fields.  */
#define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1)
  _Unwind_Word flags;
  char old_signal_frame; /* Unused now, always 0.  */
  char by_value[DWARF_FRAME_REGISTERS+1];
  /* 0 for now, can be increased when further fields are added to
     struct _Unwind_Context.  */
  _Unwind_Word version;
  _Unwind_Word args_size;
};

and keep the rest of the patch the same, or alternatively
drop SIGNAL_FRAME_BIT (but keep EXTENDED_CONTEXT_BIT the 2nd MSB)
and use signal_frame only if _Unwind_IsExtendedContext (context).

As args_size (see above) is only used within the same unwinder,
we can IMHO move it to a different field, therefore support even
extremely huge function arguments when only the new under is used.

The struct now includes also a version field which can be used
for further extensions of struct _Unwind_Context, so that we don't
have to fight this bit battle again.

Bootstrapped/regtested on 7 linux arches.

Thoughts?

2006-12-28  Jakub Jelinek  <jakub@redhat.com>

	* unwind-dw2.c (SIGNAL_FRAME_BIT, EXTENDED_CONTEXT_BIT): Define.
	(struct _Unwind_Context): Rename args_size to flags, remove
	signal_frame field, add a new args_size field and version field.
	(_Unwind_IsSignalFrame, _Unwind_SetSignalFrame,
	_Unwind_IsExtendedContext): New inline functions.
	(_Unwind_GetGR, _Unwind_SetGR, _Unwind_GetGRPtr, _Unwind_SetGRPtr):
	Assume by_value array is only present if _Unwind_IsExtendedContext.
	(_Unwind_GetIPInfo, execute_cfa_program, uw_frame_state_for): Use
	_Unwind_IsSignalFrame.
	(__frame_state_for): Initialize context.flags to EXTENDED_CONTEXT_BIT.
	(uw_update_context_1): Use _Unwind_SetSignalFrame.
	(uw_init_context_1): Initialize context->flags to
	EXTENDED_CONTEXT_BIT.
	* config/rs6000/linux-unwind.h (frob_update_context): Use
	_Unwind_SetSignalFrame.

--- gcc/unwind-dw2.c.jj	2006-12-08 15:57:44.000000000 +0100
+++ gcc/unwind-dw2.c	2006-12-27 17:16:12.000000000 +0100
@@ -70,8 +70,15 @@ struct _Unwind_Context
   void *ra;
   void *lsda;
   struct dwarf_eh_bases bases;
+  /* Signal frame context.  */
+#define SIGNAL_FRAME_BIT ((~(_Unwind_Word) 0 >> 1) + 1)
+  /* Context which has version/args_size/by_value fields.  */
+#define EXTENDED_CONTEXT_BIT ((~(_Unwind_Word) 0 >> 2) + 1)
+  _Unwind_Word flags;
+  /* 0 for now, can be increased when further fields are added to
+     struct _Unwind_Context.  */
+  _Unwind_Word version;
   _Unwind_Word args_size;
-  char signal_frame;
   char by_value[DWARF_FRAME_REGISTERS+1];
 };
 
@@ -123,6 +130,27 @@ read_8u (const void *p) { const union un
 static inline unsigned long
 read_8s (const void *p) { const union unaligned *up = p; return up->s8; }
 \f
+static inline _Unwind_Word
+_Unwind_IsSignalFrame (struct _Unwind_Context *context)
+{
+  return (context->flags & SIGNAL_FRAME_BIT) ? 1 : 0;
+}
+
+static inline void
+_Unwind_SetSignalFrame (struct _Unwind_Context *context, int val)
+{
+  if (val)
+    context->flags |= SIGNAL_FRAME_BIT;
+  else
+    context->flags &= ~SIGNAL_FRAME_BIT;
+}
+
+static inline _Unwind_Word
+_Unwind_IsExtendedContext (struct _Unwind_Context *context)
+{
+  return context->flags & EXTENDED_CONTEXT_BIT;
+}
+\f
 /* Get the value of register INDEX as saved in CONTEXT.  */
 
 inline _Unwind_Word
@@ -141,7 +169,7 @@ _Unwind_GetGR (struct _Unwind_Context *c
   size = dwarf_reg_size_table[index];
   ptr = context->reg[index];
 
-  if (context->by_value[index])
+  if (_Unwind_IsExtendedContext (context) && context->by_value[index])
     return (_Unwind_Word) (_Unwind_Internal_Ptr) ptr;
 
   /* This will segfault if the register hasn't been saved.  */
@@ -180,7 +208,7 @@ _Unwind_SetGR (struct _Unwind_Context *c
   gcc_assert (index < (int) sizeof(dwarf_reg_size_table));
   size = dwarf_reg_size_table[index];
 
-  if (context->by_value[index])
+  if (_Unwind_IsExtendedContext (context) && context->by_value[index])
     {
       context->reg[index] = (void *) (_Unwind_Internal_Ptr) val;
       return;
@@ -203,7 +231,7 @@ static inline void *
 _Unwind_GetGRPtr (struct _Unwind_Context *context, int index)
 {
   index = DWARF_REG_TO_UNWIND_COLUMN (index);
-  if (context->by_value[index])
+  if (_Unwind_IsExtendedContext (context) && context->by_value[index])
     return &context->reg[index];
   return context->reg[index];
 }
@@ -214,7 +242,8 @@ static inline void
 _Unwind_SetGRPtr (struct _Unwind_Context *context, int index, void *p)
 {
   index = DWARF_REG_TO_UNWIND_COLUMN (index);
-  context->by_value[index] = 0;
+  if (_Unwind_IsExtendedContext (context))
+    context->by_value[index] = 0;
   context->reg[index] = p;
 }
 
@@ -256,7 +285,7 @@ _Unwind_GetIP (struct _Unwind_Context *c
 inline _Unwind_Ptr
 _Unwind_GetIPInfo (struct _Unwind_Context *context, int *ip_before_insn)
 {
-  *ip_before_insn = context->signal_frame != 0;
+  *ip_before_insn = _Unwind_IsSignalFrame (context);
   return (_Unwind_Ptr) context->ra;
 }
 
@@ -824,7 +853,8 @@ execute_cfa_program (const unsigned char
      reflected at the point immediately before the call insn.
      In signal frames, return address is after last completed instruction,
      so we add 1 to return address to make the comparison <=.  */
-  while (insn_ptr < insn_end && fs->pc < context->ra + context->signal_frame)
+  while (insn_ptr < insn_end
+	 && fs->pc < context->ra + _Unwind_IsSignalFrame (context))
     {
       unsigned char insn = *insn_ptr++;
       _Unwind_Word reg, utmp;
@@ -1063,7 +1093,7 @@ uw_frame_state_for (struct _Unwind_Conte
   if (context->ra == 0)
     return _URC_END_OF_STACK;
 
-  fde = _Unwind_Find_FDE (context->ra + context->signal_frame - 1,
+  fde = _Unwind_Find_FDE (context->ra + _Unwind_IsSignalFrame (context) - 1,
 			  &context->bases);
   if (fde == NULL)
     {
@@ -1142,6 +1172,7 @@ __frame_state_for (void *pc_target, stru
   int reg;
 
   memset (&context, 0, sizeof (struct _Unwind_Context));
+  context.flags = EXTENDED_CONTEXT_BIT;
   context.ra = pc_target + 1;
 
   if (uw_frame_state_for (&context, &fs) != _URC_NO_REASON)
@@ -1306,7 +1337,7 @@ uw_update_context_1 (struct _Unwind_Cont
 	break;
       }
 
-  context->signal_frame = fs->signal_frame;
+  _Unwind_SetSignalFrame (context, fs->signal_frame);
 
 #ifdef MD_FROB_UPDATE_CONTEXT
   MD_FROB_UPDATE_CONTEXT (context, fs);
@@ -1366,6 +1397,7 @@ uw_init_context_1 (struct _Unwind_Contex
 
   memset (context, 0, sizeof (struct _Unwind_Context));
   context->ra = ra;
+  context->flags = EXTENDED_CONTEXT_BIT;
 
   code = uw_frame_state_for (context, &fs);
   gcc_assert (code == _URC_NO_REASON);
--- gcc/config/rs6000/linux-unwind.h.jj	2006-10-29 21:49:21.000000000 +0100
+++ gcc/config/rs6000/linux-unwind.h	2006-12-28 10:40:19.000000000 +0100
@@ -319,7 +319,7 @@ frob_update_context (struct _Unwind_Cont
   if (pc[0] == 0x38210000 + SIGNAL_FRAMESIZE
       && (pc[1] == 0x38000077 || pc[1] == 0x380000AC)
       && pc[2] == 0x44000002)
-    context->signal_frame = 1;
+    _Unwind_SetSignalFrame (context, 1);
 #else
   /* li r0, 0x7777; sc  (sigreturn old)  */
   /* li r0, 0x0077; sc  (sigreturn new)  */
@@ -328,7 +328,7 @@ frob_update_context (struct _Unwind_Cont
   if ((pc[0] == 0x38007777 || pc[0] == 0x38000077
        || pc[0] == 0x38006666 || pc[0] == 0x380000AC)
       && pc[1] == 0x44000002)
-    context->signal_frame = 1;
+    _Unwind_SetSignalFrame (context, 1);
 #endif
 
 #ifdef __powerpc64__

	Jakub

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-01-08  4:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-12-28 20:46 [PATCH] Fix _Unwind_Context backward compatibility issues Jakub Jelinek
2006-12-29  6:54 ` [PATCH]: for Score backend liqin
2007-01-04  1:30   ` liqin
2007-01-08  4:53     ` liqin
2007-01-03 18:07 ` [PATCH] Fix _Unwind_Context backward compatibility issues Richard Henderson

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).