This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v3 2/5] Change xstate_bv handling to use 8 bytes of data.


On 12/06/2016 10:58 AM, Michael Sturm wrote:
> The size of the state-component bitmap as specified in
> Intel(R) 64 and IA-32 Architectures Software Developer's Manual,
> Chapter 13.4.2 is 8 bytes.
> So far, the data types used for xstate_bv_p (gdb_byte*),
> clear_bv (unsigned int) and tdep->xcr0 (uint64_t) were
> inconsistent. But, since the xstate components were still
> fitting into a single byte, the code still worked
> as expected.
> However, with the addition of the PKU feature (bit 9),
> using one byte for the bitmap will no longer be sufficient.
> 
> This patch changes related code to use 64 bit data types
> consistently and changes read/write acces of the XSAVE
> header in the xsave buffer to use the endianess-aware
> functions extract_unsigned_integer and store_unsigned_integer.

The typing is a bit inconsistent, with some places using
unsigned long long, while others ULONGEST.  It'd be nice to
use uint64_t if we exactly mean 64-bit.

But that's for another day.  Meanwhile, this LGTM.

>      * i387-tdep.c (i387_supply_xsave): Change type
>      of clear_bv to ULONGEST. Replace gdb_byte *xstate_bv_p
>      with ULONGEST xstate_bv and use extract_unsigned_integer
>      and store_unsigned_integer to read/write its value from
>      the xsave buffer. This is required to make sure that
>      eventual differences in endianess between host and
>      target are taken care off.

The "This is required ..." part belongs in the commit log.
The ChangeLogs only mention the "what", not the "why".

Thanks,
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]