Message ID | BANLkTimt9V4kyKE2jmixeMOW=k7ghWrw+A@mail.gmail.com |
---|---|
State | New |
Headers | show |
"H.J. Lu" <hjl.tools@gmail.com> writes: > 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. Oh please, don't pile hack upon hack to avoid proper testing. Rainer
On Fri, Jul 1, 2011 at 2:02 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: > >> 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. > > Oh please, don't pile hack upon hack to avoid proper testing. > What is your suggestion?
"H.J. Lu" <hjl.tools@gmail.com> writes: > On Fri, Jul 1, 2011 at 2:02 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote: >> "H.J. Lu" <hjl.tools@gmail.com> writes: >> >>> 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. >> >> Oh please, don't pile hack upon hack to avoid proper testing. >> > > What is your suggestion? How about moving the md-unwind-support.h include up to the rest of the includes? The headers usually only define MD_FALLBACK_FRAME_STATE_FOR and perhaps MD_FROB_UPDATE_CONTEXT, everything else is an internal helper macro, so order shouldn't matter. Rainer
On Fri, Jul 1, 2011 at 6:37 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: > >> On Fri, Jul 1, 2011 at 2:02 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote: >>> "H.J. Lu" <hjl.tools@gmail.com> writes: >>> >>>> 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. >>> >>> Oh please, don't pile hack upon hack to avoid proper testing. >>> >> >> What is your suggestion? > > How about moving the md-unwind-support.h include up to the rest of the > includes? The headers usually only define MD_FALLBACK_FRAME_STATE_FOR > and perhaps MD_FROB_UPDATE_CONTEXT, everything else is an internal > helper macro, so order shouldn't matter. > It doesn't work on Linux/x86-64: In file included from /export/gnu/import/git/gcc/libgcc/../gcc/unwind-dw2.c:39:0: ./md-unwind-support.h: In function \u2018x86_fallback_frame_state\u2019: ./md-unwind-support.h:121:30: error: dereferencing pointer to incomplete type ./md-unwind-support.h:129:17: error: dereferencing pointer to incomplete type ./md-unwind-support.h:141:23: error: dereferencing pointer to incomplete type ./md-unwind-support.h:153:49: error: dereferencing pointer to incomplete type ./md-unwind-support.h: In function \u2018x86_frob_update_context\u2019: ./md-unwind-support.h:185:30: error: dereferencing pointer to incomplete type ./md-unwind-support.h:192:5: warning: implicit declaration of function \u2018_Unwind_SetSignalFrame\u2019 [-Wimplicit-function-declaration] /export/gnu/import/git/gcc/libgcc/../gcc/unwind-dw2.c: At top level: /export/gnu/import/git/gcc/libgcc/../gcc/unwind-dw2.c:140:1: warning: conflicting types for \u2018_Unwind_SetSignalFrame\u2019 [enabled by default] /export/gnu/import/git/gcc/libgcc/../gcc/unwind-dw2.c:140:1: error: static declaration of \u2018_Unwind_SetSignalFrame\u2019 follows non-static declaration ./md-unwind-support.h:192:5: note: previous implicit declaration of \u2018_Unwind_SetSignalFrame\u2019 was here
"H.J. Lu" <hjl.tools@gmail.com> writes: >>> What is your suggestion? >> >> How about moving the md-unwind-support.h include up to the rest of the >> includes? The headers usually only define MD_FALLBACK_FRAME_STATE_FOR >> and perhaps MD_FROB_UPDATE_CONTEXT, everything else is an internal >> helper macro, so order shouldn't matter. >> > > It doesn't work on Linux/x86-64: > > In file included from > /export/gnu/import/git/gcc/libgcc/../gcc/unwind-dw2.c:39:0: > ./md-unwind-support.h: In function \u2018x86_fallback_frame_state\u2019: > ./md-unwind-support.h:121:30: error: dereferencing pointer to incomplete type Then move it below the definition of struct _Unwind_Context with a comment explaining why it has to be there. I thought you aspired to become Linux maintainer? Rainer
On Fri, Jul 1, 2011 at 7:02 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: > >>>> What is your suggestion? >>> >>> How about moving the md-unwind-support.h include up to the rest of the >>> includes? The headers usually only define MD_FALLBACK_FRAME_STATE_FOR >>> and perhaps MD_FROB_UPDATE_CONTEXT, everything else is an internal >>> helper macro, so order shouldn't matter. >>> >> >> It doesn't work on Linux/x86-64: >> >> In file included from >> /export/gnu/import/git/gcc/libgcc/../gcc/unwind-dw2.c:39:0: >> ./md-unwind-support.h: In function \u2018x86_fallback_frame_state\u2019: >> ./md-unwind-support.h:121:30: error: dereferencing pointer to incomplete type > > Then move it below the definition of struct _Unwind_Context with a It won't work since I need to define a macro before struct _Unwind_Context. > comment explaining why it has to be there. I thought you aspired to > become Linux maintainer? > Yes, not by breaking working codes.
"H.J. Lu" <hjl.tools@gmail.com> writes: >> Then move it below the definition of struct _Unwind_Context with a > > It won't work since I need to define a macro before struct _Unwind_Context. Then this does seem to be a case for libgcc_tm_file indeed. Ugly that the unwinder configuration has to be split between two different files this way, but unavoidable, it seems. At least when libgcc_tm_file moves to libgcc/config.host, it won't any longer be split between gcc and libgcc. >> comment explaining why it has to be there. I thought you aspired to >> become Linux maintainer? > > Yes, not by breaking working codes. Anyway, thanks for trying. I'd really have liked to avoid this split. Rainer
On Fri, Jul 1, 2011 at 7:25 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: > >>> Then move it below the definition of struct _Unwind_Context with a >> >> It won't work since I need to define a macro before struct _Unwind_Context. > > Then this does seem to be a case for libgcc_tm_file indeed. Ugly that > the unwinder configuration has to be split between two different files > this way, but unavoidable, it seems. At least when libgcc_tm_file moves > to libgcc/config.host, it won't any longer be split between gcc and libgcc. > We need target support for unwind: 1. Target configuration. 2. Target implementation. Unfortunately, they can't be put in the same file. If we really want to use the same file, we can add some macros like: UNWIND_CONFIGURE UNWIND_IMPLEMENT and we can include the same file twice.
On 06/30/2011 04:47 PM, H.J. Lu wrote: > +@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 This ought to suggest why a port would need to do this, namely if registers can be larger than void*. > +#ifdef REG_VALUE_IN_UNWIND_CONTEXT > +typedef _Unwind_Word _Unwind_Context_Reg_Val; > + > +#define _Unwind_IsExtendedContext(c) 1 I still think that assuming extended context should be a separate target macro which is implied by REG_VALUE_IN_UNWIND_CONTEXT, but can also be defined separately by new ports. Otherwise it looks good to me. Jason
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 + <http://www.gnu.org/licenses/>. */ + +/* 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