From patchwork Thu Jun 30 20:47:30 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 102834 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 2350CB6F53 for ; Fri, 1 Jul 2011 06:47:49 +1000 (EST) Received: (qmail 4160 invoked by alias); 30 Jun 2011 20:47:48 -0000 Received: (qmail 4151 invoked by uid 22791); 30 Jun 2011 20:47:46 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, TW_GM, TW_PM X-Spam-Check-By: sourceware.org Received: from mail-qy0-f182.google.com (HELO mail-qy0-f182.google.com) (209.85.216.182) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 30 Jun 2011 20:47:31 +0000 Received: by qyk38 with SMTP id 38so1596641qyk.20 for ; Thu, 30 Jun 2011 13:47:30 -0700 (PDT) MIME-Version: 1.0 Received: by 10.229.63.69 with SMTP id a5mr1977476qci.94.1309466850466; Thu, 30 Jun 2011 13:47:30 -0700 (PDT) Received: by 10.229.214.83 with HTTP; Thu, 30 Jun 2011 13:47:30 -0700 (PDT) In-Reply-To: <4E0CC85E.3090401@redhat.com> References: <20110625161357.GA5401@intel.com> <4E078E58.20903@redhat.com> <4E07A879.70509@redhat.com> <4E089A88.60401@redhat.com> <4E0C8361.5050403@redhat.com> <4E0C904E.8090504@redhat.com> <4E0CC85E.3090401@redhat.com> Date: Thu, 30 Jun 2011 13:47:30 -0700 Message-ID: Subject: Re: PATCH [8/n]: Prepare x32: PR other/48007: Unwind library doesn't work with UNITS_PER_WORD > sizeof (void *) From: "H.J. Lu" To: Richard Henderson Cc: Rainer Orth , Jason Merrill , gcc-patches@gcc.gnu.org, Jakub Jelinek X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Thu, Jun 30, 2011 at 12:02 PM, Richard Henderson wrote: > On 06/30/2011 11:23 AM, H.J. Lu wrote: >> +#ifdef REG_VALUE_IN_UNWIND_CONTEXT >> +typedef _Unwind_Word _Unwind_Context_Reg_Val; >> +/* Signal frame context.  */ >> +#define SIGNAL_FRAME_BIT ((_Unwind_Word) 1 >> 0) > > There's absolutely no reason to re-define this. > So what if the value is most-significant-bit set? > > Nor do I see any reason not to continue setting E_C_B. Done. >> +#define _Unwind_IsExtendedContext(c) 1 > > Why is this not still an inline function? It is defined before _Unwind_Context is declared. I used macros so that there can be one less "#ifdef". >> + >> +static inline _Unwind_Word >> +_Unwind_Get_Unwind_Word (_Unwind_Context_Reg_Val val) >> +{ >> +  return val; >> +} >> + >> +static inline _Unwind_Context_Reg_Val >> +_Unwind_Get_Unwind_Context_Reg_Val (_Unwind_Word val) >> +{ >> +  return val; >> +} > > I cannot believe this actually works.  I see nowhere that > you copy the by-address slot out of the stack frame and > place it into the by-value slot in the unwind context. I changed the implantation based on the feedback from Jason. Now I use the same reg field for both value and address. >>    /* This will segfault if the register hasn't been saved.  */ >>    if (size == sizeof(_Unwind_Ptr)) >> -    return * (_Unwind_Ptr *) ptr; >> +    return * (_Unwind_Ptr *) (_Unwind_Internal_Ptr) val; >>    else >>      { >>        gcc_assert (size == sizeof(_Unwind_Word)); >> -      return * (_Unwind_Word *) ptr; >> +      return * (_Unwind_Word *) (_Unwind_Internal_Ptr) val; >>      } > > Indeed, this section is both wrong and belies the change > you purport to make. > > You didn't even test this, did you? > Here is the updated patch. It works on simple tests. I am running full tests. I kept config/i386/value-unwind.h since libgcc/md-unwind-support.h is included too late in unwind-dw2.c and I don't want to move it to be on the safe side. OK for trunk? Thanks. diff --git a/gcc/config.gcc b/gcc/config.gcc index 73c47d7..2702df2 100644 --- a/gcc/config.gcc +++ b/gcc/config.gcc @@ -2630,6 +2651,7 @@ esac case ${target} in i[34567]86-*-linux* | x86_64-*-linux*) tmake_file="${tmake_file} i386/t-pmm_malloc i386/t-i386" + libgcc_tm_file="${libgcc_tm_file} i386/value-unwind.h" ;; i[34567]86-*-* | x86_64-*-*) tmake_file="${tmake_file} i386/t-gmm_malloc i386/t-i386" diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 341628b..2666716 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -3701,6 +3701,14 @@ return @code{@var{regno}}. @end defmac +@defmac REG_VALUE_IN_UNWIND_CONTEXT + +Define this macro if the target stores register values as +@code{_Unwind_Word} type in unwind context. The default is to +store register values as @code{void *} type. + +@end defmac + @node Elimination @subsection Eliminating Frame Pointer and Arg Pointer diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index f7c16e9..690fa52 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -3687,6 +3687,14 @@ return @code{@var{regno}}. @end defmac +@defmac REG_VALUE_IN_UNWIND_CONTEXT + +Define this macro if the target stores register values as +@code{_Unwind_Word} type in unwind context. The default is to +store register values as @code{void *} type. + +@end defmac + @node Elimination @subsection Eliminating Frame Pointer and Arg Pointer diff --git a/gcc/system.h b/gcc/system.h index e02cbcd..ed39d93 100644 --- a/gcc/system.h +++ b/gcc/system.h @@ -764,7 +764,7 @@ extern void fancy_abort (const char *, int, const char *) ATTRIBUTE_NORETURN; /* Target macros only used for code built for the target, that have moved to libgcc-tm.h or have never been present elsewhere. */ #pragma GCC poison DECLARE_LIBRARY_RENAMES LIBGCC2_GNU_PREFIX \ - MD_UNWIND_SUPPORT ENABLE_EXECUTE_STACK + MD_UNWIND_SUPPORT ENABLE_EXECUTE_STACK REG_VALUE_IN_UNWIND_CONTEXT /* Other obsolete target macros, or macros that used to be in target headers and were not used, and may be obsolete or may never have diff --git a/gcc/unwind-dw2.c b/gcc/unwind-dw2.c index 19da299..2114b8b 100644 --- a/gcc/unwind-dw2.c +++ b/gcc/unwind-dw2.c @@ -59,12 +59,46 @@ #define DWARF_REG_TO_UNWIND_COLUMN(REGNO) (REGNO) #endif +#ifdef REG_VALUE_IN_UNWIND_CONTEXT +typedef _Unwind_Word _Unwind_Context_Reg_Val; + +#define _Unwind_IsExtendedContext(c) 1 + +static inline _Unwind_Word +_Unwind_Get_Unwind_Word (_Unwind_Context_Reg_Val val) +{ + return val; +} + +static inline _Unwind_Context_Reg_Val +_Unwind_Get_Unwind_Context_Reg_Val (_Unwind_Word val) +{ + return val; +} +#else +typedef void *_Unwind_Context_Reg_Val; + +#define _Unwind_IsExtendedContext(c) ((c)->flags & EXTENDED_CONTEXT_BIT) + +static inline _Unwind_Word +_Unwind_Get_Unwind_Word (_Unwind_Context_Reg_Val val) +{ + return (_Unwind_Word) (_Unwind_Internal_Ptr) val; +} + +static inline _Unwind_Context_Reg_Val +_Unwind_Get_Unwind_Context_Reg_Val (_Unwind_Word val) +{ + return (_Unwind_Context_Reg_Val) (_Unwind_Internal_Ptr) val; +} +#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. */ struct _Unwind_Context { - void *reg[DWARF_FRAME_REGISTERS+1]; + _Unwind_Context_Reg_Val reg[DWARF_FRAME_REGISTERS+1]; void *cfa; void *ra; void *lsda; @@ -143,12 +177,6 @@ _Unwind_SetSignalFrame (struct _Unwind_Context *context, int val) else context->flags &= ~SIGNAL_FRAME_BIT; } - -static inline _Unwind_Word -_Unwind_IsExtendedContext (struct _Unwind_Context *context) -{ - return context->flags & EXTENDED_CONTEXT_BIT; -} /* Get the value of register INDEX as saved in CONTEXT. */ @@ -156,7 +184,7 @@ inline _Unwind_Word _Unwind_GetGR (struct _Unwind_Context *context, int index) { int size; - void *ptr; + _Unwind_Context_Reg_Val val; #ifdef DWARF_ZERO_REG if (index == DWARF_ZERO_REG) @@ -166,18 +194,18 @@ _Unwind_GetGR (struct _Unwind_Context *context, int index) index = DWARF_REG_TO_UNWIND_COLUMN (index); gcc_assert (index < (int) sizeof(dwarf_reg_size_table)); size = dwarf_reg_size_table[index]; - ptr = context->reg[index]; + val = context->reg[index]; if (_Unwind_IsExtendedContext (context) && context->by_value[index]) - return (_Unwind_Word) (_Unwind_Internal_Ptr) ptr; + return _Unwind_Get_Unwind_Word (val); /* This will segfault if the register hasn't been saved. */ if (size == sizeof(_Unwind_Ptr)) - return * (_Unwind_Ptr *) ptr; + return * (_Unwind_Ptr *) (_Unwind_Internal_Ptr) val; else { gcc_assert (size == sizeof(_Unwind_Word)); - return * (_Unwind_Word *) ptr; + return * (_Unwind_Word *) (_Unwind_Internal_Ptr) val; } } @@ -209,11 +237,11 @@ _Unwind_SetGR (struct _Unwind_Context *context, int index, _Unwind_Word val) if (_Unwind_IsExtendedContext (context) && context->by_value[index]) { - context->reg[index] = (void *) (_Unwind_Internal_Ptr) val; + context->reg[index] = _Unwind_Get_Unwind_Context_Reg_Val (val); return; } - ptr = context->reg[index]; + ptr = (void *) (_Unwind_Internal_Ptr) context->reg[index]; if (size == sizeof(_Unwind_Ptr)) * (_Unwind_Ptr *) ptr = val; @@ -232,7 +260,7 @@ _Unwind_GetGRPtr (struct _Unwind_Context *context, int index) index = DWARF_REG_TO_UNWIND_COLUMN (index); if (_Unwind_IsExtendedContext (context) && context->by_value[index]) return &context->reg[index]; - return context->reg[index]; + return (void *) (_Unwind_Internal_Ptr) context->reg[index]; } /* Set the pointer to a register INDEX as saved in CONTEXT. */ @@ -243,7 +271,7 @@ _Unwind_SetGRPtr (struct _Unwind_Context *context, int index, void *p) index = DWARF_REG_TO_UNWIND_COLUMN (index); if (_Unwind_IsExtendedContext (context)) context->by_value[index] = 0; - context->reg[index] = p; + context->reg[index] = (_Unwind_Context_Reg_Val) (_Unwind_Internal_Ptr) p; } /* Overwrite the saved value for register INDEX in CONTEXT with VAL. */ @@ -254,10 +282,10 @@ _Unwind_SetGRValue (struct _Unwind_Context *context, int index, { index = DWARF_REG_TO_UNWIND_COLUMN (index); gcc_assert (index < (int) sizeof(dwarf_reg_size_table)); - gcc_assert (dwarf_reg_size_table[index] == sizeof (_Unwind_Ptr)); + gcc_assert (dwarf_reg_size_table[index] == sizeof (_Unwind_Context_Reg_Val)); context->by_value[index] = 1; - context->reg[index] = (void *) (_Unwind_Internal_Ptr) val; + context->reg[index] = _Unwind_Get_Unwind_Context_Reg_Val (val); } /* Return nonzero if register INDEX is stored by value rather than @@ -1215,7 +1243,9 @@ __frame_state_for (void *pc_target, struct frame_state *state_in) int reg; memset (&context, 0, sizeof (struct _Unwind_Context)); +#ifndef REG_VALUE_IN_UNWIND_CONTEXT context.flags = EXTENDED_CONTEXT_BIT; +#endif context.ra = pc_target + 1; if (uw_frame_state_for (&context, &fs) != _URC_NO_REASON) @@ -1453,7 +1483,9 @@ uw_init_context_1 (struct _Unwind_Context *context, memset (context, 0, sizeof (struct _Unwind_Context)); context->ra = ra; +#ifndef REG_VALUE_IN_UNWIND_CONTEXT context->flags = EXTENDED_CONTEXT_BIT; +#endif code = uw_frame_state_for (context, &fs); gcc_assert (code == _URC_NO_REASON); @@ -1532,8 +1564,8 @@ uw_install_context_1 (struct _Unwind_Context *current, for (i = 0; i < DWARF_FRAME_REGISTERS; ++i) { - void *c = current->reg[i]; - void *t = target->reg[i]; + void *c = (void *) (_Unwind_Internal_Ptr) current->reg[i]; + void *t = (void *) (_Unwind_Internal_Ptr)target->reg[i]; gcc_assert (current->by_value[i] == 0); if (target->by_value[i] && c) diff --git a/libgcc/config/i386/value-unwind.h b/libgcc/config/i386/value-unwind.h new file mode 100644 index 0000000..0dceb5c --- /dev/null +++ b/libgcc/config/i386/value-unwind.h @@ -0,0 +1,26 @@ +/* Store register values as _Unwind_Word type in DWARF2 EH unwind context. + Copyright (C) 2011 + Free Software Foundation, Inc. + + This file is part of GCC. + + GCC is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published + by the Free Software Foundation; either version 3, or (at your + option) any later version. + + GCC is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public + License for more details. + + You should have received a copy of the GNU General Public License and + a copy of the GCC Runtime Library Exception along with this program; + see the files COPYING3 and COPYING.RUNTIME respectively. If not, see + . */ + +/* Define this macro if the target stores register values as _Unwind_Word + type in unwind context. Only enable it for x32. */ +#if defined __x86_64 && !defined __LP64__ +# define REG_VALUE_IN_UNWIND_CONTEXT +#endif