Re: PATCH [8/n]: Prepare x32: PR other/48007: Unwind library doesn't work with UNITS_PER_WORD > sizeof (void *)

[Richard Henderson <rth@redhat.com>]

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.

> +#define _Unwind_IsExtendedContext(c) 1

Why is this not still an inline function?

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

>    /* 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?


r~