PING.

On Thu, Jun 30, 2011 at 1:47 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Thu, Jun 30, 2011 at 12:02 PM, Richard Henderson <r...@redhat.com> 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.
>
> --
> H.J.
> ---
> gcc/
>
> 2011-06-30  H.J. Lu  <hongjiu...@intel.com>
>
>        * config.gcc (libgcc_tm_file): Add i386/value-unwind.h for
>        Linux/x86.
>
>        * system.h (REG_VALUE_IN_UNWIND_CONTEXT): Poisoned.
>
>        * unwind-dw2.c (_Unwind_Context_Reg_Val): New.
>        (_Unwind_Get_Unwind_Word): Likewise.
>        (_Unwind_Get_Unwind_Context_Reg_Val): Likewise.
>        (_Unwind_Context): Use _Unwind_Context_Reg_Val on the reg field.
>        (_Unwind_IsExtendedContext): Defined as macro.
>        (_Unwind_GetGR): Updated.
>        (_Unwind_SetGR): Likewise.
>        (_Unwind_GetGRPtr): Likewise.
>        (_Unwind_SetGRPtr): Likewise.
>        (_Unwind_SetGRValue): Likewise.
>        (_Unwind_GRByValue): Likewise.
>        (__frame_state_for): Likewise.
>        (uw_install_context_1): Likewise.
>
>        * doc/tm.texi.in: Document REG_VALUE_IN_UNWIND_CONTEXT.
>        * doc/tm.texi: Regenerated.
>
> libgcc/
>
> 2011-06-30  H.J. Lu  <hongjiu...@intel.com>
>
>        * config/i386/value-unwind.h: New.
>



-- 
H.J.

Reply via email to