From patchwork Tue Mar 22 04:18:56 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: 87857 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 0D06FB6F74 for ; Tue, 22 Mar 2011 15:19:35 +1100 (EST) Received: (qmail 30363 invoked by alias); 22 Mar 2011 04:19:27 -0000 Received: (qmail 25560 invoked by uid 22791); 22 Mar 2011 04:19:02 -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 X-Spam-Check-By: sourceware.org Received: from mail-iy0-f175.google.com (HELO mail-iy0-f175.google.com) (209.85.210.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 22 Mar 2011 04:18:58 +0000 Received: by iyb26 with SMTP id 26so8748010iyb.20 for ; Mon, 21 Mar 2011 21:18:56 -0700 (PDT) MIME-Version: 1.0 Received: by 10.42.156.67 with SMTP id y3mr8026683icw.381.1300767536305; Mon, 21 Mar 2011 21:18:56 -0700 (PDT) Received: by 10.43.135.9 with HTTP; Mon, 21 Mar 2011 21:18:56 -0700 (PDT) In-Reply-To: References: <20110306171830.GA18591@intel.com> Date: Mon, 21 Mar 2011 21:18:56 -0700 Message-ID: Subject: Re: PATCH: PR other/48007: Unwind library doesn't work with UNITS_PER_WORD > sizeof (void *) From: "H.J. Lu" To: Ian Lance Taylor Cc: GCC Patches 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 Mon, Mar 21, 2011 at 2:55 PM, Ian Lance Taylor wrote: > On Sun, Mar 6, 2011 at 9:18 AM, H.J. Lu wrote: > >> We shouldn't save call frame hard registers as "void *".  This patch >> changes the unwind library to save call frame hard registers as >> _Unwind_Word.  OK for 4.7? > >> +       PR other/48007 >> +       * unwind-dw2.c (_Unwind_Context): Save call frame hard registers >> +       as _Unwind_Word. >> +       (_Unwind_GetGR): Get GR value as _Unwind_Word. >> +       (_Unwind_SetGR): Set GR value as _Unwind_Word. >> +       (_Unwind_SetGRValue): Likewise. >> +       (_Unwind_GetGRPtr): Cast return to "void *". >> +       (_Unwind_SetGRPtr): Cast pointer to _Unwind_Word. >> +       (uw_install_context_1): Cast pointer to "void *". > > The approach you are using here looks wrong to me.  When looking at > the DWARF2 _Unwind_Context, you have to look at the by_value field for > the register.  If by_value is true, then the reg field for that > register holds an _Unwind_Word which is the value of the register.  If > by_value is false, then the reg field holds a pointer to the place > where the actual value is stored.  The size of the actual value is > determined by dwarf_reg_size_table.  In other words, the reg field is > a really a union of _Unwind_Word and void*. > > You have correctly observed that the current code fails for the case > where sizeof(_Unwind_Word) > sizeof(void*).  However, the answer is > not to change the type from void* to _Unwind_Word.  That will fail > when sizeof(void*) > sizeof(_Unwind_Word). > > I think you have to make the reg field a union. > > By the way, don't use intptr_t in the unwind code.  Use > _Unwind_Internal_Ptr.  But I think if you make the field a union, you > won't need to use either type. > > Ian > Here is the updated patch. It has @@ -243,7 +250,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].ref = p; } Is that OK to take the address of a union member? diff --git a/gcc/unwind-dw2.c b/gcc/unwind-dw2.c index 25990b4..1cb4b47 100644 --- a/gcc/unwind-dw2.c +++ b/gcc/unwind-dw2.c @@ -59,12 +59,19 @@ #define DWARF_REG_TO_UNWIND_COLUMN(REGNO) (REGNO) #endif +/* A reister is stored either by reference or by value. */ +union _Unwind_Register +{ + void *ref; + _Unwind_Word val; +}; + /* 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]; + union _Unwind_Register reg[DWARF_FRAME_REGISTERS+1]; void *cfa; void *ra; void *lsda; @@ -156,7 +163,7 @@ inline _Unwind_Word _Unwind_GetGR (struct _Unwind_Context *context, int index) { int size; - void *ptr; + union _Unwind_Register reg; #ifdef DWARF_ZERO_REG if (index == DWARF_ZERO_REG) @@ -166,18 +173,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]; + reg = context->reg[index]; if (_Unwind_IsExtendedContext (context) && context->by_value[index]) - return (_Unwind_Word) (_Unwind_Internal_Ptr) ptr; + return reg.val; /* This will segfault if the register hasn't been saved. */ if (size == sizeof(_Unwind_Ptr)) - return * (_Unwind_Ptr *) ptr; + return * (_Unwind_Ptr *) reg.ref; else { gcc_assert (size == sizeof(_Unwind_Word)); - return * (_Unwind_Word *) ptr; + return * (_Unwind_Word *) reg.ref; } } @@ -209,11 +216,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].val = val; return; } - ptr = context->reg[index]; + ptr = context->reg[index].ref; if (size == sizeof(_Unwind_Ptr)) * (_Unwind_Ptr *) ptr = val; @@ -231,8 +238,8 @@ _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 *) &context->reg[index].val; + return context->reg[index].ref; } /* Set the pointer to a register INDEX as saved in CONTEXT. */ @@ -243,7 +250,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].ref = p; } /* Overwrite the saved value for register INDEX in CONTEXT with VAL. */ @@ -254,10 +261,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_Word)); context->by_value[index] = 1; - context->reg[index] = (void *) (_Unwind_Internal_Ptr) val; + context->reg[index].val = val; } /* Return nonzero if register INDEX is stored by value rather than @@ -1534,8 +1541,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 = current->reg[i].ref; + void *t = target->reg[i].ref; gcc_assert (current->by_value[i] == 0); if (target->by_value[i] && c)