[02/25] Adjust the order of 32bit-linux.xml and 32bit-sse.xml in i386/i386-linux.xml

  Exchange the order of 32bit-linux.xml and 32bit-sse.xml in
i386/i386-linux.xml, to align with other i386 linux .xml files.

gdb:

2017-04-27  Yao Qi  <yao.qi@linaro.org>

	* features/i386/i386-linux.xml: Exchange the order of including
	32bit-linux.xml and 32bit-sse.xml.
	* features/i386/i386-linux.c: Regenerated.
---
 gdb/features/i386/i386-linux.c   | 6 +++---
 gdb/features/i386/i386-linux.xml | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
  

On 2017-06-12 10:41, Yao Qi wrote:
> Exchange the order of 32bit-linux.xml and 32bit-sse.xml in
> i386/i386-linux.xml, to align with other i386 linux .xml files.
> 
> gdb:
> 
> 2017-04-27  Yao Qi  <yao.qi@linaro.org>
> 
> 	* features/i386/i386-linux.xml: Exchange the order of including
> 	32bit-linux.xml and 32bit-sse.xml.
> 	* features/i386/i386-linux.c: Regenerated.
> ---
>  gdb/features/i386/i386-linux.c   | 6 +++---
>  gdb/features/i386/i386-linux.xml | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/features/i386/i386-linux.c 
> b/gdb/features/i386/i386-linux.c
> index 42c406b..c7796c3 100644
> --- a/gdb/features/i386/i386-linux.c
> +++ b/gdb/features/i386/i386-linux.c
> @@ -71,9 +71,6 @@ initialize_tdesc_i386_linux (void)
>    tdesc_create_reg (feature, "fooff", 30, 1, "float", 32, "int");
>    tdesc_create_reg (feature, "fop", 31, 1, "float", 32, "int");
> 
> -  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux");
> -  tdesc_create_reg (feature, "orig_eax", 41, 1, NULL, 32, "int");
> -
>    feature = tdesc_create_feature (result, "org.gnu.gdb.i386.sse");
>    field_type = tdesc_named_type (feature, "ieee_single");
>    tdesc_create_vector (feature, "v4f", field_type, 4);
> @@ -135,5 +132,8 @@ initialize_tdesc_i386_linux (void)
>    tdesc_create_reg (feature, "xmm7", 39, 1, NULL, 128, "vec128");
>    tdesc_create_reg (feature, "mxcsr", 40, 1, "vector", 32, 
> "i386_mxcsr");
> 
> +  feature = tdesc_create_feature (result, "org.gnu.gdb.i386.linux");
> +  tdesc_create_reg (feature, "orig_eax", 41, 1, NULL, 32, "int");
> +
>    tdesc_i386_linux = result;
>  }
> diff --git a/gdb/features/i386/i386-linux.xml 
> b/gdb/features/i386/i386-linux.xml
> index f9aa311..17f9a1a 100644
> --- a/gdb/features/i386/i386-linux.xml
> +++ b/gdb/features/i386/i386-linux.xml
> @@ -12,6 +12,6 @@
>    <architecture>i386</architecture>
>    <osabi>GNU/Linux</osabi>
>    <xi:include href="32bit-core.xml"/>
> -  <xi:include href="32bit-linux.xml"/>
>    <xi:include href="32bit-sse.xml"/>
> +  <xi:include href="32bit-linux.xml"/>
>  </target>

I think you can push this one right away as a cosmetic cleanup.

Simon
  

On 06/19/2017 09:22 PM, Simon Marchi wrote:
> On 2017-06-12 10:41, Yao Qi wrote:
>> Exchange the order of 32bit-linux.xml and 32bit-sse.xml in
>> i386/i386-linux.xml, to align with other i386 linux .xml files.
>>


>> --- a/gdb/features/i386/i386-linux.xml
>> +++ b/gdb/features/i386/i386-linux.xml
>> @@ -12,6 +12,6 @@
>>    <architecture>i386</architecture>
>>    <osabi>GNU/Linux</osabi>
>>    <xi:include href="32bit-core.xml"/>
>> -  <xi:include href="32bit-linux.xml"/>
>>    <xi:include href="32bit-sse.xml"/>
>> +  <xi:include href="32bit-linux.xml"/>
>>  </target>
> 
> I think you can push this one right away as a cosmetic cleanup.

Unless this is a case of a default target description matching
the layout of targets that predated support for XML descriptions.

Could that be the case here?  From:

static void
i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
{
...
  if (! tdesc_has_registers (tdesc))
    tdesc = tdesc_i386_linux;
...

... it may well be.  So we need to tread carefully here.  The
order may be required for back compatibility.  A deeper audit
with that in mind is in order.

Thanks,
Pedro Alves
  

On 2017-06-19 23:24, Pedro Alves wrote:
> Unless this is a case of a default target description matching
> the layout of targets that predated support for XML descriptions.
> 
> Could that be the case here?  From:
> 
> static void
> i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> {
> ...
>   if (! tdesc_has_registers (tdesc))
>     tdesc = tdesc_i386_linux;
> ...
> 
> ... it may well be.  So we need to tread carefully here.  The
> order may be required for back compatibility.  A deeper audit
> with that in mind is in order.
> 
> Thanks,
> Pedro Alves

Do you mean that this might impact backward compatibility with older 
gdbservers (or other remotes) that don't send XML target descriptions 
and just assume a certain "well-known" register numbering?
  

On 06/19/2017 10:48 PM, Simon Marchi wrote:
> On 2017-06-19 23:24, Pedro Alves wrote:
>> Unless this is a case of a default target description matching
>> the layout of targets that predated support for XML descriptions.
>>
>> Could that be the case here?  From:
>>
>> static void
>> i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> {
>> ...
>>   if (! tdesc_has_registers (tdesc))
>>     tdesc = tdesc_i386_linux;
>> ...
>>
>> ... it may well be.  So we need to tread carefully here.  The
>> order may be required for back compatibility.  A deeper audit
>> with that in mind is in order.

> Do you mean that this might impact backward compatibility with older
> gdbservers (or other remotes) that don't send XML target descriptions
> and just assume a certain "well-known" register numbering?

Yes.

It may be easy to check against gdbserver.  You'd need to hack it to
disable the x86 xml tdesc support, which is a relatively
recent addition.  [It took a while for the x86 port to support xml
tdescs].  The xmlRegisters= qSupported feature had to invented to
keep backward compatibility back then.

E.g., hack gdb's remote.c:register_remote_support_xml, or
gdbserver's linux-x86-low.c:x86_linux_process_qsupported.
If, with x86 XML support disabled, before vs after patch the
layout of GDB's g/G packet buffer changes, then you have
a back compatibility break.

Thanks,
Pedro Alves