libunwind-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Libunwind-devel] [PATCH] mips/Ginit.c: fix access_mem accessor for


From: Dave Watson
Subject: Re: [Libunwind-devel] [PATCH] mips/Ginit.c: fix access_mem accessor for O32 ABI
Date: Mon, 27 Feb 2017 10:30:54 -0800
User-agent: Mutt/1.6.0 (2016-04-01)

<address@hidden>

Comments inline.

On 02/26/17 12:12 PM, Sergey Korolev wrote:
> The patch prevents undesirable access to 8 bytes instead of 4
> on MIPS (be/el) O32 that potentially may cause a segmentation fault.
> 
> This also fixes wrong readings on MIPS (be) O32 in mips/Gis_signal_frame.c.
> Specificaly non-patched version reads w0 and w1 64 bit values with swapped
> lower and higher 32 bits on big endian platforms.
> ---
>  src/mips/Ginit.c | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mips/Ginit.c b/src/mips/Ginit.c
> index 83b100f..856df15 100644
> --- a/src/mips/Ginit.c
> +++ b/src/mips/Ginit.c
> @@ -90,18 +90,52 @@ get_dyn_info_list_addr (unw_addr_space_t as, unw_word_t
> *dyn_info_list_addr,
>    return 0;
>  }
> 
> +static inline int
> +is_32_bit (unw_word_t w)
> +{
> +  return ((uint32_t) (w >> 32)) == 0;
> +}
> +

This doesn't look quite right, is it not possible to get an address
with the upper 32 bits unset on 64bit?  It looks like these checks
could just be removed entirely.

>  static int
>  access_mem (unw_addr_space_t as, unw_word_t addr, unw_word_t *val, int
> write,
>              void *arg)
>  {
> +  if (as->abi == UNW_MIPS_ABI_O32 && !is_32_bit (addr))
> +    {
> +      Debug (1, "bad O32 ABI address %llx\n", (long long) addr);
> +      return -UNW_EUNSPEC;
> +    }
> +
>    if (write)
>      {
>        Debug (16, "mem[%llx] <- %llx\n", (long long) addr, (long long)
> *val);
> -      *(unw_word_t *) (intptr_t) addr = *val;
> +
> +      if (as->abi == UNW_MIPS_ABI_O32)
> +        {
> +          if (!is_32_bit (*val))
> +            {
> +              Debug (1, "bad O32 ABI value %llx\n", (long long) *val);
> +              return -UNW_EUNSPEC;
> +            }
> +
> +          *(uint32_t *) addr = (uint32_t) *val;
> +        }
> +      else
> +        {
> +          *(unw_word_t *) (intptr_t) addr = *val;
> +        }
>      }
>    else
>      {
> -      *val = *(unw_word_t *) (intptr_t) addr;
> +      if (as->abi == UNW_MIPS_ABI_O32)
> +        {
> +          *val = *(uint32_t *) addr;
> +        }
> +      else
> +        {
> +          *val = *(unw_word_t *) (intptr_t) addr;

Hmm so this means for UNW_MIPS_ABI_O32 intptr_t is actually 8 bytes?
Interesting.

> +        }
> +
>        Debug (16, "mem[%llx] -> %llx\n", (long long) addr, (long long)
> *val);
>      }
>    return 0;
> -- 
> 2.7.4

I still have a couple untested mips patches if you're willing to test
them at

https://github.com/djwatson/libunwind/commits/patch-queue-mips

Thanks!



reply via email to

[Prev in Thread] Current Thread [Next in Thread]