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
