I tested
https://github.com/djwatson/libunwind/commit/5acd7c9a2c6dadc778a78c37bbd4fbf4625361fe
with O32 ABI (be/le) and it seems it more correct and supersedes my fixes,
but I am not sure that _MIPS_SIM is defined on all systems supported by the
library.

2017-02-27 21:30 GMT+03:00 Dave Watson <davejwat...@fb.com>:

> <ca+qjrnl7sew-nqelids2unz8w3ukwqae2qvpli4+bmjss8m...@mail.gmail.com>
>
> 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!
>
_______________________________________________
Libunwind-devel mailing list
Libunwind-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/libunwind-devel

Reply via email to