* [PATCH 4/5]: Enhancements to "flags": i386 cleanup
@ 2016-02-29 23:09 Doug Evans
2016-07-20 18:18 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Doug Evans @ 2016-02-29 23:09 UTC (permalink / raw)
To: gdb-patches; +Cc: cole945
Hi.
This patch just simplifies things by removing the "end" spec in
i386 eflags definitions, and is otherwise a nop.
I removed them because they're redundant.
2016-02-29 Doug Evans <dje@google.com>
* features/i386/32bit-core.xml (i386_eflags): Remove "end" spec.
* features/i386/32bit-sse.xml (i386_eflags): Ditto.
* features/i386/64bit-core.xml (i386_eflags): Ditto.
* features/i386/64bit-sse.xml (i386_eflags): Ditto.
* features/i386/x32-core.xml (i386_eflags): Ditto.
diff --git a/gdb/features/i386/32bit-core.xml
b/gdb/features/i386/32bit-core.xml
index a27863f..b00d913 100644
--- a/gdb/features/i386/32bit-core.xml
+++ b/gdb/features/i386/32bit-core.xml
@@ -8,23 +8,23 @@
<!DOCTYPE feature SYSTEM "gdb-target.dtd">
<feature name="org.gnu.gdb.i386.core">
<flags id="i386_eflags" size="4">
- <field name="CF" start="0" end="0"/>
- <field name="" start="1" end="1"/>
- <field name="PF" start="2" end="2"/>
- <field name="AF" start="4" end="4"/>
- <field name="ZF" start="6" end="6"/>
- <field name="SF" start="7" end="7"/>
- <field name="TF" start="8" end="8"/>
- <field name="IF" start="9" end="9"/>
- <field name="DF" start="10" end="10"/>
- <field name="OF" start="11" end="11"/>
- <field name="NT" start="14" end="14"/>
- <field name="RF" start="16" end="16"/>
- <field name="VM" start="17" end="17"/>
- <field name="AC" start="18" end="18"/>
- <field name="VIF" start="19" end="19"/>
- <field name="VIP" start="20" end="20"/>
- <field name="ID" start="21" end="21"/>
+ <field name="CF" start="0"/>
+ <field name="" start="1"/>
+ <field name="PF" start="2"/>
+ <field name="AF" start="4"/>
+ <field name="ZF" start="6"/>
+ <field name="SF" start="7"/>
+ <field name="TF" start="8"/>
+ <field name="IF" start="9"/>
+ <field name="DF" start="10"/>
+ <field name="OF" start="11"/>
+ <field name="NT" start="14"/>
+ <field name="RF" start="16"/>
+ <field name="VM" start="17"/>
+ <field name="AC" start="18"/>
+ <field name="VIF" start="19"/>
+ <field name="VIP" start="20"/>
+ <field name="ID" start="21"/>
</flags>
<reg name="eax" bitsize="32" type="int32"/>
diff --git a/gdb/features/i386/32bit-sse.xml
b/gdb/features/i386/32bit-sse.xml
index 5a44d1e..4448a7e 100644
--- a/gdb/features/i386/32bit-sse.xml
+++ b/gdb/features/i386/32bit-sse.xml
@@ -23,20 +23,20 @@
<field name="uint128" type="uint128"/>
</union>
<flags id="i386_mxcsr" size="4">
- <field name="IE" start="0" end="0"/>
- <field name="DE" start="1" end="1"/>
- <field name="ZE" start="2" end="2"/>
- <field name="OE" start="3" end="3"/>
- <field name="UE" start="4" end="4"/>
- <field name="PE" start="5" end="5"/>
- <field name="DAZ" start="6" end="6"/>
- <field name="IM" start="7" end="7"/>
- <field name="DM" start="8" end="8"/>
- <field name="ZM" start="9" end="9"/>
- <field name="OM" start="10" end="10"/>
- <field name="UM" start="11" end="11"/>
- <field name="PM" start="12" end="12"/>
- <field name="FZ" start="15" end="15"/>
+ <field name="IE" start="0"/>
+ <field name="DE" start="1"/>
+ <field name="ZE" start="2"/>
+ <field name="OE" start="3"/>
+ <field name="UE" start="4"/>
+ <field name="PE" start="5"/>
+ <field name="DAZ" start="6"/>
+ <field name="IM" start="7"/>
+ <field name="DM" start="8"/>
+ <field name="ZM" start="9"/>
+ <field name="OM" start="10"/>
+ <field name="UM" start="11"/>
+ <field name="PM" start="12"/>
+ <field name="FZ" start="15"/>
</flags>
<reg name="xmm0" bitsize="128" type="vec128" regnum="32"/>
diff --git a/gdb/features/i386/64bit-core.xml
b/gdb/features/i386/64bit-core.xml
index 92f4e87..6e847c1 100644
--- a/gdb/features/i386/64bit-core.xml
+++ b/gdb/features/i386/64bit-core.xml
@@ -8,23 +8,23 @@
<!DOCTYPE feature SYSTEM "gdb-target.dtd">
<feature name="org.gnu.gdb.i386.core">
<flags id="i386_eflags" size="4">
- <field name="CF" start="0" end="0"/>
- <field name="" start="1" end="1"/>
- <field name="PF" start="2" end="2"/>
- <field name="AF" start="4" end="4"/>
- <field name="ZF" start="6" end="6"/>
- <field name="SF" start="7" end="7"/>
- <field name="TF" start="8" end="8"/>
- <field name="IF" start="9" end="9"/>
- <field name="DF" start="10" end="10"/>
- <field name="OF" start="11" end="11"/>
- <field name="NT" start="14" end="14"/>
- <field name="RF" start="16" end="16"/>
- <field name="VM" start="17" end="17"/>
- <field name="AC" start="18" end="18"/>
- <field name="VIF" start="19" end="19"/>
- <field name="VIP" start="20" end="20"/>
- <field name="ID" start="21" end="21"/>
+ <field name="CF" start="0"/>
+ <field name="" start="1"/>
+ <field name="PF" start="2"/>
+ <field name="AF" start="4"/>
+ <field name="ZF" start="6"/>
+ <field name="SF" start="7"/>
+ <field name="TF" start="8"/>
+ <field name="IF" start="9"/>
+ <field name="DF" start="10"/>
+ <field name="OF" start="11"/>
+ <field name="NT" start="14"/>
+ <field name="RF" start="16"/>
+ <field name="VM" start="17"/>
+ <field name="AC" start="18"/>
+ <field name="VIF" start="19"/>
+ <field name="VIP" start="20"/>
+ <field name="ID" start="21"/>
</flags>
<reg name="rax" bitsize="64" type="int64"/>
diff --git a/gdb/features/i386/64bit-sse.xml
b/gdb/features/i386/64bit-sse.xml
index 2a5271e..dd6a850 100644
--- a/gdb/features/i386/64bit-sse.xml
+++ b/gdb/features/i386/64bit-sse.xml
@@ -23,20 +23,20 @@
<field name="uint128" type="uint128"/>
</union>
<flags id="i386_mxcsr" size="4">
- <field name="IE" start="0" end="0"/>
- <field name="DE" start="1" end="1"/>
- <field name="ZE" start="2" end="2"/>
- <field name="OE" start="3" end="3"/>
- <field name="UE" start="4" end="4"/>
- <field name="PE" start="5" end="5"/>
- <field name="DAZ" start="6" end="6"/>
- <field name="IM" start="7" end="7"/>
- <field name="DM" start="8" end="8"/>
- <field name="ZM" start="9" end="9"/>
- <field name="OM" start="10" end="10"/>
- <field name="UM" start="11" end="11"/>
- <field name="PM" start="12" end="12"/>
- <field name="FZ" start="15" end="15"/>
+ <field name="IE" start="0"/>
+ <field name="DE" start="1"/>
+ <field name="ZE" start="2"/>
+ <field name="OE" start="3"/>
+ <field name="UE" start="4"/>
+ <field name="PE" start="5"/>
+ <field name="DAZ" start="6"/>
+ <field name="IM" start="7"/>
+ <field name="DM" start="8"/>
+ <field name="ZM" start="9"/>
+ <field name="OM" start="10"/>
+ <field name="UM" start="11"/>
+ <field name="PM" start="12"/>
+ <field name="FZ" start="15"/>
</flags>
<reg name="xmm0" bitsize="128" type="vec128" regnum="40"/>
diff --git a/gdb/features/i386/x32-core.xml b/gdb/features/i386/x32-core.xml
index ab51ffc..c03cdea 100644
--- a/gdb/features/i386/x32-core.xml
+++ b/gdb/features/i386/x32-core.xml
@@ -8,23 +8,23 @@
<!DOCTYPE feature SYSTEM "gdb-target.dtd">
<feature name="org.gnu.gdb.i386.core">
<flags id="i386_eflags" size="4">
- <field name="CF" start="0" end="0"/>
- <field name="" start="1" end="1"/>
- <field name="PF" start="2" end="2"/>
- <field name="AF" start="4" end="4"/>
- <field name="ZF" start="6" end="6"/>
- <field name="SF" start="7" end="7"/>
- <field name="TF" start="8" end="8"/>
- <field name="IF" start="9" end="9"/>
- <field name="DF" start="10" end="10"/>
- <field name="OF" start="11" end="11"/>
- <field name="NT" start="14" end="14"/>
- <field name="RF" start="16" end="16"/>
- <field name="VM" start="17" end="17"/>
- <field name="AC" start="18" end="18"/>
- <field name="VIF" start="19" end="19"/>
- <field name="VIP" start="20" end="20"/>
- <field name="ID" start="21" end="21"/>
+ <field name="CF" start="0"/>
+ <field name="" start="1"/>
+ <field name="PF" start="2"/>
+ <field name="AF" start="4"/>
+ <field name="ZF" start="6"/>
+ <field name="SF" start="7"/>
+ <field name="TF" start="8"/>
+ <field name="IF" start="9"/>
+ <field name="DF" start="10"/>
+ <field name="OF" start="11"/>
+ <field name="NT" start="14"/>
+ <field name="RF" start="16"/>
+ <field name="VM" start="17"/>
+ <field name="AC" start="18"/>
+ <field name="VIF" start="19"/>
+ <field name="VIP" start="20"/>
+ <field name="ID" start="21"/>
</flags>
<reg name="rax" bitsize="64" type="int64"/>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-02-29 23:09 [PATCH 4/5]: Enhancements to "flags": i386 cleanup Doug Evans
@ 2016-07-20 18:18 ` Pedro Alves
2016-07-22 19:16 ` Doug Evans
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-07-20 18:18 UTC (permalink / raw)
To: Doug Evans, gdb-patches; +Cc: cole945
Hi Doug,
On 02/29/2016 11:09 PM, Doug Evans wrote:
> Hi.
>
> This patch just simplifies things by removing the "end" spec in
> i386 eflags definitions, and is otherwise a nop.
>
> I removed them because they're redundant.
>
I noticed that this makes older gdbs reject the new target descriptions.
E.g., gdb 7.11.1 against master gdbserver:
Remote debugging using :9999
warning: while parsing target description (at line 24): Field "CF" has neither type nor bit position
warning: Could not load XML target description; ignoring
Reverting the patch makes old gdb grok the tdesc again (git revert 49b7ae7bb8f2).
Since it was meant as a cleanup, I think we should revert
it on grounds of avoiding a back compatibility break. WDYT?
Thanks,
Pedro Alves
> 2016-02-29 Doug Evans <dje@google.com>
>
> * features/i386/32bit-core.xml (i386_eflags): Remove "end" spec.
> * features/i386/32bit-sse.xml (i386_eflags): Ditto.
> * features/i386/64bit-core.xml (i386_eflags): Ditto.
> * features/i386/64bit-sse.xml (i386_eflags): Ditto.
> * features/i386/x32-core.xml (i386_eflags): Ditto.
>
> diff --git a/gdb/features/i386/32bit-core.xml
> b/gdb/features/i386/32bit-core.xml
> index a27863f..b00d913 100644
> --- a/gdb/features/i386/32bit-core.xml
> +++ b/gdb/features/i386/32bit-core.xml
> @@ -8,23 +8,23 @@
> <!DOCTYPE feature SYSTEM "gdb-target.dtd">
> <feature name="org.gnu.gdb.i386.core">
> <flags id="i386_eflags" size="4">
> - <field name="CF" start="0" end="0"/>
> - <field name="" start="1" end="1"/>
> - <field name="PF" start="2" end="2"/>
> - <field name="AF" start="4" end="4"/>
> - <field name="ZF" start="6" end="6"/>
> - <field name="SF" start="7" end="7"/>
> - <field name="TF" start="8" end="8"/>
> - <field name="IF" start="9" end="9"/>
> - <field name="DF" start="10" end="10"/>
> - <field name="OF" start="11" end="11"/>
> - <field name="NT" start="14" end="14"/>
> - <field name="RF" start="16" end="16"/>
> - <field name="VM" start="17" end="17"/>
> - <field name="AC" start="18" end="18"/>
> - <field name="VIF" start="19" end="19"/>
> - <field name="VIP" start="20" end="20"/>
> - <field name="ID" start="21" end="21"/>
> + <field name="CF" start="0"/>
> + <field name="" start="1"/>
> + <field name="PF" start="2"/>
> + <field name="AF" start="4"/>
> + <field name="ZF" start="6"/>
> + <field name="SF" start="7"/>
> + <field name="TF" start="8"/>
> + <field name="IF" start="9"/>
> + <field name="DF" start="10"/>
> + <field name="OF" start="11"/>
> + <field name="NT" start="14"/>
> + <field name="RF" start="16"/>
> + <field name="VM" start="17"/>
> + <field name="AC" start="18"/>
> + <field name="VIF" start="19"/>
> + <field name="VIP" start="20"/>
> + <field name="ID" start="21"/>
> </flags>
>
> <reg name="eax" bitsize="32" type="int32"/>
> diff --git a/gdb/features/i386/32bit-sse.xml
> b/gdb/features/i386/32bit-sse.xml
> index 5a44d1e..4448a7e 100644
> --- a/gdb/features/i386/32bit-sse.xml
> +++ b/gdb/features/i386/32bit-sse.xml
> @@ -23,20 +23,20 @@
> <field name="uint128" type="uint128"/>
> </union>
> <flags id="i386_mxcsr" size="4">
> - <field name="IE" start="0" end="0"/>
> - <field name="DE" start="1" end="1"/>
> - <field name="ZE" start="2" end="2"/>
> - <field name="OE" start="3" end="3"/>
> - <field name="UE" start="4" end="4"/>
> - <field name="PE" start="5" end="5"/>
> - <field name="DAZ" start="6" end="6"/>
> - <field name="IM" start="7" end="7"/>
> - <field name="DM" start="8" end="8"/>
> - <field name="ZM" start="9" end="9"/>
> - <field name="OM" start="10" end="10"/>
> - <field name="UM" start="11" end="11"/>
> - <field name="PM" start="12" end="12"/>
> - <field name="FZ" start="15" end="15"/>
> + <field name="IE" start="0"/>
> + <field name="DE" start="1"/>
> + <field name="ZE" start="2"/>
> + <field name="OE" start="3"/>
> + <field name="UE" start="4"/>
> + <field name="PE" start="5"/>
> + <field name="DAZ" start="6"/>
> + <field name="IM" start="7"/>
> + <field name="DM" start="8"/>
> + <field name="ZM" start="9"/>
> + <field name="OM" start="10"/>
> + <field name="UM" start="11"/>
> + <field name="PM" start="12"/>
> + <field name="FZ" start="15"/>
> </flags>
>
> <reg name="xmm0" bitsize="128" type="vec128" regnum="32"/>
> diff --git a/gdb/features/i386/64bit-core.xml
> b/gdb/features/i386/64bit-core.xml
> index 92f4e87..6e847c1 100644
> --- a/gdb/features/i386/64bit-core.xml
> +++ b/gdb/features/i386/64bit-core.xml
> @@ -8,23 +8,23 @@
> <!DOCTYPE feature SYSTEM "gdb-target.dtd">
> <feature name="org.gnu.gdb.i386.core">
> <flags id="i386_eflags" size="4">
> - <field name="CF" start="0" end="0"/>
> - <field name="" start="1" end="1"/>
> - <field name="PF" start="2" end="2"/>
> - <field name="AF" start="4" end="4"/>
> - <field name="ZF" start="6" end="6"/>
> - <field name="SF" start="7" end="7"/>
> - <field name="TF" start="8" end="8"/>
> - <field name="IF" start="9" end="9"/>
> - <field name="DF" start="10" end="10"/>
> - <field name="OF" start="11" end="11"/>
> - <field name="NT" start="14" end="14"/>
> - <field name="RF" start="16" end="16"/>
> - <field name="VM" start="17" end="17"/>
> - <field name="AC" start="18" end="18"/>
> - <field name="VIF" start="19" end="19"/>
> - <field name="VIP" start="20" end="20"/>
> - <field name="ID" start="21" end="21"/>
> + <field name="CF" start="0"/>
> + <field name="" start="1"/>
> + <field name="PF" start="2"/>
> + <field name="AF" start="4"/>
> + <field name="ZF" start="6"/>
> + <field name="SF" start="7"/>
> + <field name="TF" start="8"/>
> + <field name="IF" start="9"/>
> + <field name="DF" start="10"/>
> + <field name="OF" start="11"/>
> + <field name="NT" start="14"/>
> + <field name="RF" start="16"/>
> + <field name="VM" start="17"/>
> + <field name="AC" start="18"/>
> + <field name="VIF" start="19"/>
> + <field name="VIP" start="20"/>
> + <field name="ID" start="21"/>
> </flags>
>
> <reg name="rax" bitsize="64" type="int64"/>
> diff --git a/gdb/features/i386/64bit-sse.xml
> b/gdb/features/i386/64bit-sse.xml
> index 2a5271e..dd6a850 100644
> --- a/gdb/features/i386/64bit-sse.xml
> +++ b/gdb/features/i386/64bit-sse.xml
> @@ -23,20 +23,20 @@
> <field name="uint128" type="uint128"/>
> </union>
> <flags id="i386_mxcsr" size="4">
> - <field name="IE" start="0" end="0"/>
> - <field name="DE" start="1" end="1"/>
> - <field name="ZE" start="2" end="2"/>
> - <field name="OE" start="3" end="3"/>
> - <field name="UE" start="4" end="4"/>
> - <field name="PE" start="5" end="5"/>
> - <field name="DAZ" start="6" end="6"/>
> - <field name="IM" start="7" end="7"/>
> - <field name="DM" start="8" end="8"/>
> - <field name="ZM" start="9" end="9"/>
> - <field name="OM" start="10" end="10"/>
> - <field name="UM" start="11" end="11"/>
> - <field name="PM" start="12" end="12"/>
> - <field name="FZ" start="15" end="15"/>
> + <field name="IE" start="0"/>
> + <field name="DE" start="1"/>
> + <field name="ZE" start="2"/>
> + <field name="OE" start="3"/>
> + <field name="UE" start="4"/>
> + <field name="PE" start="5"/>
> + <field name="DAZ" start="6"/>
> + <field name="IM" start="7"/>
> + <field name="DM" start="8"/>
> + <field name="ZM" start="9"/>
> + <field name="OM" start="10"/>
> + <field name="UM" start="11"/>
> + <field name="PM" start="12"/>
> + <field name="FZ" start="15"/>
> </flags>
>
> <reg name="xmm0" bitsize="128" type="vec128" regnum="40"/>
> diff --git a/gdb/features/i386/x32-core.xml
> b/gdb/features/i386/x32-core.xml
> index ab51ffc..c03cdea 100644
> --- a/gdb/features/i386/x32-core.xml
> +++ b/gdb/features/i386/x32-core.xml
> @@ -8,23 +8,23 @@
> <!DOCTYPE feature SYSTEM "gdb-target.dtd">
> <feature name="org.gnu.gdb.i386.core">
> <flags id="i386_eflags" size="4">
> - <field name="CF" start="0" end="0"/>
> - <field name="" start="1" end="1"/>
> - <field name="PF" start="2" end="2"/>
> - <field name="AF" start="4" end="4"/>
> - <field name="ZF" start="6" end="6"/>
> - <field name="SF" start="7" end="7"/>
> - <field name="TF" start="8" end="8"/>
> - <field name="IF" start="9" end="9"/>
> - <field name="DF" start="10" end="10"/>
> - <field name="OF" start="11" end="11"/>
> - <field name="NT" start="14" end="14"/>
> - <field name="RF" start="16" end="16"/>
> - <field name="VM" start="17" end="17"/>
> - <field name="AC" start="18" end="18"/>
> - <field name="VIF" start="19" end="19"/>
> - <field name="VIP" start="20" end="20"/>
> - <field name="ID" start="21" end="21"/>
> + <field name="CF" start="0"/>
> + <field name="" start="1"/>
> + <field name="PF" start="2"/>
> + <field name="AF" start="4"/>
> + <field name="ZF" start="6"/>
> + <field name="SF" start="7"/>
> + <field name="TF" start="8"/>
> + <field name="IF" start="9"/>
> + <field name="DF" start="10"/>
> + <field name="OF" start="11"/>
> + <field name="NT" start="14"/>
> + <field name="RF" start="16"/>
> + <field name="VM" start="17"/>
> + <field name="AC" start="18"/>
> + <field name="VIF" start="19"/>
> + <field name="VIP" start="20"/>
> + <field name="ID" start="21"/>
> </flags>
>
> <reg name="rax" bitsize="64" type="int64"/>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-07-20 18:18 ` Pedro Alves
@ 2016-07-22 19:16 ` Doug Evans
2016-08-08 15:06 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Doug Evans @ 2016-07-22 19:16 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, cole945
On Wed, Jul 20, 2016 at 11:17 AM, Pedro Alves <palves@redhat.com> wrote:
> Hi Doug,
>
> On 02/29/2016 11:09 PM, Doug Evans wrote:
>> Hi.
>>
>> This patch just simplifies things by removing the "end" spec in
>> i386 eflags definitions, and is otherwise a nop.
>>
>> I removed them because they're redundant.
>>
>
> I noticed that this makes older gdbs reject the new target descriptions.
> E.g., gdb 7.11.1 against master gdbserver:
>
> Remote debugging using :9999
> warning: while parsing target description (at line 24): Field "CF" has neither type nor bit position
> warning: Could not load XML target description; ignoring
>
> Reverting the patch makes old gdb grok the tdesc again (git revert 49b7ae7bb8f2).
>
> Since it was meant as a cleanup, I think we should revert
> it on grounds of avoiding a back compatibility break. WDYT?
Fine by me.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-07-22 19:16 ` Doug Evans
@ 2016-08-08 15:06 ` Pedro Alves
2016-08-08 20:34 ` Doug Evans
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-08-08 15:06 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches, cole945
Hi Doug,
I was reverting this today, but the revert stumbles on something,
and I think this must be fixed before 7.12 is released.
See below.
On 07/22/2016 08:16 PM, Doug Evans wrote:
> On Wed, Jul 20, 2016 at 11:17 AM, Pedro Alves <palves@redhat.com> wrote:
>> Hi Doug,
>>
>> On 02/29/2016 11:09 PM, Doug Evans wrote:
>>> Hi.
>>>
>>> This patch just simplifies things by removing the "end" spec in
>>> i386 eflags definitions, and is otherwise a nop.
>>>
>>> I removed them because they're redundant.
>>>
>>
>> I noticed that this makes older gdbs reject the new target descriptions.
>> E.g., gdb 7.11.1 against master gdbserver:
>>
>> Remote debugging using :9999
>> warning: while parsing target description (at line 24): Field "CF" has neither type nor bit position
>> warning: Could not load XML target description; ignoring
>>
>> Reverting the patch makes old gdb grok the tdesc again (git revert 49b7ae7bb8f2).
>>
>> Since it was meant as a cleanup, I think we should revert
>> it on grounds of avoiding a back compatibility break. WDYT?
>
> Fine by me.
>
Testing the revert against gdbserver regresses caught gcore.exp:
Running /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/gcore.exp ...
FAIL: gdb.base/gcore.exp: corefile restored general registers
FAIL: gdb.base/gcore.exp: corefile restored all registers
Turns out that adding an "end" field back now makes gdb
consider the flags as bitfields. That is, with:
- <field name="CF" start="0"/>
+ <field name="CF" start="0" end="0"/>
etc., we now get:
rip 0x4005ea 0x4005ea <terminal_func+4>
-eflags 0x202 [ IF ]
+eflags 0x202 [ CF=0 PF=0 AF=0 ZF=0 SF=0 TF=0 IF=1 DF=0 OF=0 NT=0 RF=0 VM=0 AC=0 VIF=0 VIP=0 ID=0 ]
cs 0x33 51
And indeed, regenerating the features/*.c files gives us:
--- c/gdb/features/i386/amd64-avx-linux.c
+++ w/gdb/features/i386/amd64-avx-linux.c
@@ -20,23 +20,23 @@ initialize_tdesc_amd64_avx_linux (void)
feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core");
type = tdesc_create_flags (feature, "i386_eflags", 4);
- tdesc_add_flag (type, 0, "CF");
- tdesc_add_flag (type, 1, "");
- tdesc_add_flag (type, 2, "PF");
- tdesc_add_flag (type, 4, "AF");
- tdesc_add_flag (type, 6, "ZF");
- tdesc_add_flag (type, 7, "SF");
- tdesc_add_flag (type, 8, "TF");
- tdesc_add_flag (type, 9, "IF");
- tdesc_add_flag (type, 10, "DF");
- tdesc_add_flag (type, 11, "OF");
- tdesc_add_flag (type, 14, "NT");
- tdesc_add_flag (type, 16, "RF");
- tdesc_add_flag (type, 17, "VM");
- tdesc_add_flag (type, 18, "AC");
- tdesc_add_flag (type, 19, "VIF");
- tdesc_add_flag (type, 20, "VIP");
- tdesc_add_flag (type, 21, "ID");
+ tdesc_add_bitfield (type, "CF", 0, 0);
+ tdesc_add_bitfield (type, "", 1, 1);
+ tdesc_add_bitfield (type, "PF", 2, 2);
+ tdesc_add_bitfield (type, "AF", 4, 4);
+ tdesc_add_bitfield (type, "ZF", 6, 6);
+ tdesc_add_bitfield (type, "SF", 7, 7);
+ tdesc_add_bitfield (type, "TF", 8, 8);
+ tdesc_add_bitfield (type, "IF", 9, 9);
+ tdesc_add_bitfield (type, "DF", 10, 10);
+ tdesc_add_bitfield (type, "OF", 11, 11);
+ tdesc_add_bitfield (type, "NT", 14, 14);
+ tdesc_add_bitfield (type, "RF", 16, 16);
Etc.
However this is not what we want; we want these to continue to be
treated as flags. (I.e., the regeneration should have come out
empty.)
Seems like the original change is thus not only a backward compatibility
break, but a forward compatibility break as well, unfortunately.
I tried to make gdb treat "end" == "start" the same as not specifying
"end", with:
diff --git c/gdb/xml-tdesc.c w/gdb/xml-tdesc.c
index aa58385..34f2d18 100644
--- c/gdb/xml-tdesc.c
+++ w/gdb/xml-tdesc.c
@@ -414,7 +414,7 @@ tdesc_start_field (struct gdb_xml_parser *parser,
_("Bitfield \"%s\" does not fit in struct"));
}
- if (end == -1)
+ if (start == end || end == -1)
{
if (field_type != NULL)
tdesc_add_typed_bitfield (t, field_name, start, start, field_type);
Regenerating the .c files with that produces changes like these:
diff --git i/gdb/features/aarch64.c w/gdb/features/aarch64.c
index cec6956..e9eaed8 100644
--- i/gdb/features/aarch64.c
+++ w/gdb/features/aarch64.c
@@ -19,10 +19,10 @@ initialize_tdesc_aarch64 (void)
feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core");
type = tdesc_create_flags (feature, "cpsr_flags", 4);
tdesc_add_flag (type, 0, "SP");
- tdesc_add_bitfield (type, "", 1, 1);
+ tdesc_add_flag (type, 1, "");
tdesc_add_bitfield (type, "EL", 2, 3);
tdesc_add_flag (type, 4, "nRW");
- tdesc_add_bitfield (type, "", 5, 5);
+ tdesc_add_flag (type, 5, "");
tdesc_add_flag (type, 6, "F");
tdesc_add_flag (type, 7, "I");
tdesc_add_flag (type, 8, "A");
which kind of looks correct, actually, given the "cpsr_flags" name,
and the odd mix of bitfields and flags?
However, it also produces this:
diff --git c/gdb/features/i386/amd64-avx-mpx-linux.c w/gdb/features/i386/amd64-avx-mpx-linux.c
index 4605480..456f262 100644
--- c/gdb/features/i386/amd64-avx-mpx-linux.c
+++ w/gdb/features/i386/amd64-avx-mpx-linux.c
@@ -191,8 +191,8 @@ initialize_tdesc_amd64_avx_mpx_linux (void)
tdesc_set_struct_size (type, 8);
tdesc_add_bitfield (type, "base", 12, 63);
tdesc_add_bitfield (type, "reserved", 2, 11);
- tdesc_add_bitfield (type, "preserved", 1, 1);
- tdesc_add_bitfield (type, "enabled", 0, 0);
+ tdesc_add_flag (type, 1, "preserved");
+ tdesc_add_flag (type, 0, "enabled");
type = tdesc_create_union (feature, "cfgu");
field_type = tdesc_named_type (feature, "data_ptr");
which doesn't look so right.
Maybe the mpx descriptions are new enough that we could
change them, not sure. But I wouldn't know how best to
change them to avoid this.
Is there something else that could/should be used to
distinguish flags vs bitfields other than "end" being
present?
I put the reversion patch in the users/palves/revert-tdesc-remove-end-spec
branch, in case it helps.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-08-08 15:06 ` Pedro Alves
@ 2016-08-08 20:34 ` Doug Evans
2016-08-09 17:55 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Doug Evans @ 2016-08-08 20:34 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Wei-cheng Wang
On Mon, Aug 8, 2016 at 8:06 AM, Pedro Alves <palves@redhat.com> wrote:
> Hi Doug,
>
> I was reverting this today, but the revert stumbles on something,
> and I think this must be fixed before 7.12 is released.
>
> See below.
>
> On 07/22/2016 08:16 PM, Doug Evans wrote:
>> On Wed, Jul 20, 2016 at 11:17 AM, Pedro Alves <palves@redhat.com> wrote:
>>> Hi Doug,
>>>
>>> On 02/29/2016 11:09 PM, Doug Evans wrote:
>>>> Hi.
>>>>
>>>> This patch just simplifies things by removing the "end" spec in
>>>> i386 eflags definitions, and is otherwise a nop.
>>>>
>>>> I removed them because they're redundant.
>>>>
>>>
>>> I noticed that this makes older gdbs reject the new target descriptions.
>>> E.g., gdb 7.11.1 against master gdbserver:
>>>
>>> Remote debugging using :9999
>>> warning: while parsing target description (at line 24): Field "CF" has neither type nor bit position
>>> warning: Could not load XML target description; ignoring
>>>
>>> Reverting the patch makes old gdb grok the tdesc again (git revert 49b7ae7bb8f2).
>>>
>>> Since it was meant as a cleanup, I think we should revert
>>> it on grounds of avoiding a back compatibility break. WDYT?
>>
>> Fine by me.
>>
>
> Testing the revert against gdbserver regresses caught gcore.exp:
>
> Running /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.base/gcore.exp ...
> FAIL: gdb.base/gcore.exp: corefile restored general registers
> FAIL: gdb.base/gcore.exp: corefile restored all registers
>
> Turns out that adding an "end" field back now makes gdb
> consider the flags as bitfields. That is, with:
>
> - <field name="CF" start="0"/>
> + <field name="CF" start="0" end="0"/>
>
> etc., we now get:
>
> rip 0x4005ea 0x4005ea <terminal_func+4>
> -eflags 0x202 [ IF ]
> +eflags 0x202 [ CF=0 PF=0 AF=0 ZF=0 SF=0 TF=0 IF=1 DF=0 OF=0 NT=0 RF=0 VM=0 AC=0 VIF=0 VIP=0 ID=0 ]
> cs 0x33 51
>
> And indeed, regenerating the features/*.c files gives us:
>
> --- c/gdb/features/i386/amd64-avx-linux.c
> +++ w/gdb/features/i386/amd64-avx-linux.c
> @@ -20,23 +20,23 @@ initialize_tdesc_amd64_avx_linux (void)
>
> feature = tdesc_create_feature (result, "org.gnu.gdb.i386.core");
> type = tdesc_create_flags (feature, "i386_eflags", 4);
> - tdesc_add_flag (type, 0, "CF");
> - tdesc_add_flag (type, 1, "");
> - tdesc_add_flag (type, 2, "PF");
> - tdesc_add_flag (type, 4, "AF");
> - tdesc_add_flag (type, 6, "ZF");
> - tdesc_add_flag (type, 7, "SF");
> - tdesc_add_flag (type, 8, "TF");
> - tdesc_add_flag (type, 9, "IF");
> - tdesc_add_flag (type, 10, "DF");
> - tdesc_add_flag (type, 11, "OF");
> - tdesc_add_flag (type, 14, "NT");
> - tdesc_add_flag (type, 16, "RF");
> - tdesc_add_flag (type, 17, "VM");
> - tdesc_add_flag (type, 18, "AC");
> - tdesc_add_flag (type, 19, "VIF");
> - tdesc_add_flag (type, 20, "VIP");
> - tdesc_add_flag (type, 21, "ID");
> + tdesc_add_bitfield (type, "CF", 0, 0);
> + tdesc_add_bitfield (type, "", 1, 1);
> + tdesc_add_bitfield (type, "PF", 2, 2);
> + tdesc_add_bitfield (type, "AF", 4, 4);
> + tdesc_add_bitfield (type, "ZF", 6, 6);
> + tdesc_add_bitfield (type, "SF", 7, 7);
> + tdesc_add_bitfield (type, "TF", 8, 8);
> + tdesc_add_bitfield (type, "IF", 9, 9);
> + tdesc_add_bitfield (type, "DF", 10, 10);
> + tdesc_add_bitfield (type, "OF", 11, 11);
> + tdesc_add_bitfield (type, "NT", 14, 14);
> + tdesc_add_bitfield (type, "RF", 16, 16);
>
> Etc.
>
> However this is not what we want; we want these to continue to be
> treated as flags. (I.e., the regeneration should have come out
> empty.)
>
> Seems like the original change is thus not only a backward compatibility
> break, but a forward compatibility break as well, unfortunately.
>
> I tried to make gdb treat "end" == "start" the same as not specifying
> "end", with:
>
> diff --git c/gdb/xml-tdesc.c w/gdb/xml-tdesc.c
> index aa58385..34f2d18 100644
> --- c/gdb/xml-tdesc.c
> +++ w/gdb/xml-tdesc.c
> @@ -414,7 +414,7 @@ tdesc_start_field (struct gdb_xml_parser *parser,
> _("Bitfield \"%s\" does not fit in struct"));
> }
>
> - if (end == -1)
> + if (start == end || end == -1)
> {
> if (field_type != NULL)
> tdesc_add_typed_bitfield (t, field_name, start, start, field_type);
>
>
> Regenerating the .c files with that produces changes like these:
>
> diff --git i/gdb/features/aarch64.c w/gdb/features/aarch64.c
> index cec6956..e9eaed8 100644
> --- i/gdb/features/aarch64.c
> +++ w/gdb/features/aarch64.c
> @@ -19,10 +19,10 @@ initialize_tdesc_aarch64 (void)
> feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core");
> type = tdesc_create_flags (feature, "cpsr_flags", 4);
> tdesc_add_flag (type, 0, "SP");
> - tdesc_add_bitfield (type, "", 1, 1);
> + tdesc_add_flag (type, 1, "");
> tdesc_add_bitfield (type, "EL", 2, 3);
> tdesc_add_flag (type, 4, "nRW");
> - tdesc_add_bitfield (type, "", 5, 5);
> + tdesc_add_flag (type, 5, "");
> tdesc_add_flag (type, 6, "F");
> tdesc_add_flag (type, 7, "I");
> tdesc_add_flag (type, 8, "A");
>
>
> which kind of looks correct, actually, given the "cpsr_flags" name,
> and the odd mix of bitfields and flags?
>
> However, it also produces this:
>
> diff --git c/gdb/features/i386/amd64-avx-mpx-linux.c w/gdb/features/i386/amd64-avx-mpx-linux.c
> index 4605480..456f262 100644
> --- c/gdb/features/i386/amd64-avx-mpx-linux.c
> +++ w/gdb/features/i386/amd64-avx-mpx-linux.c
> @@ -191,8 +191,8 @@ initialize_tdesc_amd64_avx_mpx_linux (void)
> tdesc_set_struct_size (type, 8);
> tdesc_add_bitfield (type, "base", 12, 63);
> tdesc_add_bitfield (type, "reserved", 2, 11);
> - tdesc_add_bitfield (type, "preserved", 1, 1);
> - tdesc_add_bitfield (type, "enabled", 0, 0);
> + tdesc_add_flag (type, 1, "preserved");
> + tdesc_add_flag (type, 0, "enabled");
>
> type = tdesc_create_union (feature, "cfgu");
> field_type = tdesc_named_type (feature, "data_ptr");
>
> which doesn't look so right.
>
> Maybe the mpx descriptions are new enough that we could
> change them, not sure. But I wouldn't know how best to
> change them to avoid this.
>
> Is there something else that could/should be used to
> distinguish flags vs bitfields other than "end" being
> present?
>
> I put the reversion patch in the users/palves/revert-tdesc-remove-end-spec
> branch, in case it helps.
>
> Thanks,
> Pedro Alves
>
Hi.
Sorry for the trouble.
I think(!) these two patches fix things.
Basically, I made "end" required again, and made single bit fields
default to bool,
and then tweaked a couple of xml files to minimize changes.
https://sourceware.org/ml/gdb-patches/2016-08/msg00105.html
https://sourceware.org/ml/gdb-patches/2016-08/msg00106.html
It occurs to me I need to update the docs too.
Additional patch to follow.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-08-08 20:34 ` Doug Evans
@ 2016-08-09 17:55 ` Pedro Alves
2016-08-11 18:10 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-08-09 17:55 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches, Wei-cheng Wang
On 08/08/2016 09:33 PM, Doug Evans wrote:
> Hi.
> Sorry for the trouble.
>
> I think(!) these two patches fix things.
> Basically, I made "end" required again, and made single bit fields
> default to bool,
> and then tweaked a couple of xml files to minimize changes.
>
> https://sourceware.org/ml/gdb-patches/2016-08/msg00105.html
> https://sourceware.org/ml/gdb-patches/2016-08/msg00106.html
>
> It occurs to me I need to update the docs too.
> Additional patch to follow.
Thanks! All looks good to me.
--
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-08-09 17:55 ` Pedro Alves
@ 2016-08-11 18:10 ` Pedro Alves
2016-08-11 18:18 ` Doug Evans
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-08-11 18:10 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches, Wei-cheng Wang
On 08/09/2016 06:55 PM, Pedro Alves wrote:
> Thanks! All looks good to me.
A thought has been running through my mind though.
Is the "type" attribute used for anything in <flags> elements?
Does changing the type of a flag bitfield have any user-visible
effect at all? I.e., does a 1-bit uint32_t bitfield flag
print differently from a bool bitfield flag?
Wondering if we could remove the bool-special case, because
before 8151645076ce927e0ee866c598a19f192e68e103, we used to say,
for <struct>:
There are two forms of the @samp{<struct>}
element; a @samp{<struct>} element must either contain only bitfields
or contain no bitfields. If the structure contains only bitfields,
its total size in bytes must be specified, each bitfield must have an
explicit start and end, and bitfields are automatically assigned an
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
integer type. The field's @var{start} should be less than or
^^^^^^^^^^^^
equal to its @var{end}, and zero represents the least significant bit.
And for flags, we used to say:
@cindex <flags>
If a register's value is a series of single-bit flags, define it with
a flags type. The @samp{<flags>} element has an explicit @var{size}
and contains one or more @samp{<field>} elements. Each field has a
@var{name}, a @var{start}, and an @var{end}. Only single-bit flags
are supported.
I think I may be very confused by this all, though.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-08-11 18:10 ` Pedro Alves
@ 2016-08-11 18:18 ` Doug Evans
2016-10-06 11:33 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Doug Evans @ 2016-08-11 18:18 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Wei-cheng Wang
On Thu, Aug 11, 2016 at 11:10 AM, Pedro Alves <palves@redhat.com> wrote:
> On 08/09/2016 06:55 PM, Pedro Alves wrote:
>
>> Thanks! All looks good to me.
>
> A thought has been running through my mind though.
>
> Is the "type" attribute used for anything in <flags> elements?
> Does changing the type of a flag bitfield have any user-visible
> effect at all? I.e., does a 1-bit uint32_t bitfield flag
> print differently from a bool bitfield flag?
There may be some simplification possible, but note that bools do
print differently: if false we don't print the field at all.
[digression: I'm not sure it's possible today, but while I understand
the desire to avoid the extra verbosity if we always printed false
fields, there are times when I *do* want the field printed even if
false]
>
> Wondering if we could remove the bool-special case, because
> before 8151645076ce927e0ee866c598a19f192e68e103, we used to say,
> for <struct>:
>
> There are two forms of the @samp{<struct>}
> element; a @samp{<struct>} element must either contain only bitfields
> or contain no bitfields. If the structure contains only bitfields,
> its total size in bytes must be specified, each bitfield must have an
> explicit start and end, and bitfields are automatically assigned an
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> integer type. The field's @var{start} should be less than or
> ^^^^^^^^^^^^
> equal to its @var{end}, and zero represents the least significant bit.
>
> And for flags, we used to say:
>
> @cindex <flags>
> If a register's value is a series of single-bit flags, define it with
> a flags type. The @samp{<flags>} element has an explicit @var{size}
> and contains one or more @samp{<field>} elements. Each field has a
> @var{name}, a @var{start}, and an @var{end}. Only single-bit flags
> are supported.
>
> I think I may be very confused by this all, though.
>
> Thanks,
> Pedro Alves
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-08-11 18:18 ` Doug Evans
@ 2016-10-06 11:33 ` Pedro Alves
2016-10-06 13:44 ` Anton Kolesov
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-10-06 11:33 UTC (permalink / raw)
To: Doug Evans, Anton Kolesov; +Cc: gdb-patches, Wei-cheng Wang
[Adding Anton, this affects ARC]
On 08/11/2016 07:18 PM, Doug Evans wrote:
> On Thu, Aug 11, 2016 at 11:10 AM, Pedro Alves <palves@redhat.com> wrote:
>> Is the "type" attribute used for anything in <flags> elements?
>> Does changing the type of a flag bitfield have any user-visible
>> effect at all? I.e., does a 1-bit uint32_t bitfield flag
>> print differently from a bool bitfield flag?
>
> There may be some simplification possible, but note that bools do
> print differently: if false we don't print the field at all.
>
> [digression: I'm not sure it's possible today, but while I understand
> the desire to avoid the extra verbosity if we always printed false
> fields, there are times when I *do* want the field printed even if
> false]
>
So <https://sourceware.org/ml/gdb-patches/2016-10/msg00113.html> resulted
in this change:
diff --git a/gdb/features/arc-arcompact.c b/gdb/features/arc-arcompact.c
index d1fa4fe..a527cc2 100644
--- a/gdb/features/arc-arcompact.c
+++ b/gdb/features/arc-arcompact.c
@@ -54,19 +54,19 @@ initialize_tdesc_arc_arcompact (void)
feature = tdesc_create_feature (result, "org.gnu.gdb.arc.aux-minimal");
type = tdesc_create_flags (feature, "status32_type", 4);
- tdesc_add_bitfield (type, "H", 0, 0);
+ tdesc_add_flag (type, 0, "H");
tdesc_add_bitfield (type, "E", 1, 2);
tdesc_add_bitfield (type, "A", 3, 4);
- tdesc_add_bitfield (type, "AE", 5, 5);
- tdesc_add_bitfield (type, "DE", 6, 6);
- tdesc_add_bitfield (type, "U", 7, 7);
- tdesc_add_bitfield (type, "V", 8, 8);
- tdesc_add_bitfield (type, "C", 9, 9);
- tdesc_add_bitfield (type, "N", 10, 10);
- tdesc_add_bitfield (type, "Z", 11, 11);
- tdesc_add_bitfield (type, "L", 12, 12);
- tdesc_add_bitfield (type, "R", 13, 13);
- tdesc_add_bitfield (type, "SE", 14, 14);
+ tdesc_add_flag (type, 5, "AE");
+ tdesc_add_flag (type, 6, "DE");
+ tdesc_add_flag (type, 7, "U");
+ tdesc_add_flag (type, 8, "V");
+ tdesc_add_flag (type, 9, "C");
+ tdesc_add_flag (type, 10, "N");
+ tdesc_add_flag (type, 11, "Z");
+ tdesc_add_flag (type, 12, "L");
+ tdesc_add_flag (type, 13, "R");
+ tdesc_add_flag (type, 14, "SE");
diff --git a/gdb/features/arc-v2.c b/gdb/features/arc-v2.c
index c963410..b2bdfb5 100644
--- a/gdb/features/arc-v2.c
+++ b/gdb/features/arc-v2.c
@@ -54,23 +54,23 @@ initialize_tdesc_arc_v2 (void)
feature = tdesc_create_feature (result, "org.gnu.gdb.arc.aux-minimal");
type = tdesc_create_flags (feature, "status32_type", 4);
- tdesc_add_bitfield (type, "H", 0, 0);
+ tdesc_add_flag (type, 0, "H");
tdesc_add_bitfield (type, "E", 1, 4);
- tdesc_add_bitfield (type, "AE", 5, 5);
- tdesc_add_bitfield (type, "DE", 6, 6);
- tdesc_add_bitfield (type, "U", 7, 7);
- tdesc_add_bitfield (type, "V", 8, 8);
- tdesc_add_bitfield (type, "C", 9, 9);
- tdesc_add_bitfield (type, "N", 10, 10);
- tdesc_add_bitfield (type, "Z", 11, 11);
- tdesc_add_bitfield (type, "L", 12, 12);
- tdesc_add_bitfield (type, "DZ", 13, 13);
- tdesc_add_bitfield (type, "SC", 14, 14);
- tdesc_add_bitfield (type, "ES", 15, 15);
+ tdesc_add_flag (type, 5, "AE");
+ tdesc_add_flag (type, 6, "DE");
+ tdesc_add_flag (type, 7, "U");
+ tdesc_add_flag (type, 8, "V");
+ tdesc_add_flag (type, 9, "C");
+ tdesc_add_flag (type, 10, "N");
+ tdesc_add_flag (type, 11, "Z");
+ tdesc_add_flag (type, 12, "L");
+ tdesc_add_flag (type, 13, "DZ");
+ tdesc_add_flag (type, 14, "SC");
+ tdesc_add_flag (type, 15, "ES");
tdesc_add_bitfield (type, "RB", 16, 18);
- tdesc_add_bitfield (type, "AD", 19, 19);
- tdesc_add_bitfield (type, "US", 20, 20);
- tdesc_add_bitfield (type, "IE", 31, 31);
+ tdesc_add_flag (type, 19, "AD");
+ tdesc_add_flag (type, 20, "US");
+ tdesc_add_flag (type, 31, "IE");
Note how that left several flags with 2-bit and/or 4-bit
long bitfields:
tdesc_add_bitfield (type, "E", 1, 2);
tdesc_add_bitfield (type, "A", 3, 4);
...
tdesc_add_bitfield (type, "E", 1, 4);
which I understand means these two fields will
be given uint32_t type instead of bool? What does this
mean in practice? E.g,. for "A", what do we print when both
bits 3 and 4 are clear? What do we print if one
of the bits is set and the other is clear?
I see similar things on other archs though, it's
not just ARC. E.g., the patch resulted in:
--- a/gdb/features/aarch64.c
+++ b/gdb/features/aarch64.c
@@ -19,10 +19,10 @@ initialize_tdesc_aarch64 (void)
feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core");
type = tdesc_create_flags (feature, "cpsr_flags", 4);
tdesc_add_flag (type, 0, "SP");
- tdesc_add_bitfield (type, "", 1, 1);
+ tdesc_add_flag (type, 1, "");
tdesc_add_bitfield (type, "EL", 2, 3);
tdesc_add_flag (type, 4, "nRW");
- tdesc_add_bitfield (type, "", 5, 5);
+ tdesc_add_flag (type, 5, "");
tdesc_add_flag (type, 6, "F");
tdesc_add_flag (type, 7, "I");
tdesc_add_flag (type, 8, "A");
Which leaves "EL" as a 2-bit bitfield.
I'm still terribly confused. :-/
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-10-06 11:33 ` Pedro Alves
@ 2016-10-06 13:44 ` Anton Kolesov
2016-10-06 14:44 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Anton Kolesov @ 2016-10-06 13:44 UTC (permalink / raw)
To: Pedro Alves, Doug Evans; +Cc: gdb-patches, Wei-cheng Wang
Hi Pedro,
>
> Note how that left several flags with 2-bit and/or 4-bit
> long bitfields:
>
> tdesc_add_bitfield (type, "E", 1, 2);
> tdesc_add_bitfield (type, "A", 3, 4);
> ...
> tdesc_add_bitfield (type, "E", 1, 4);
>
> which I understand means these two fields will
> be given uint32_t type instead of bool? What does this
> mean in practice? E.g,. for "A", what do we print when both
> bits 3 and 4 are clear? What do we print if one
> of the bits is set and the other is clear?
With regards of ARC flags, if field is longer than one bit, then it should be
treated as an uint. For example, in arc-v2.c field H means "halt bit", so
it is a single bit, but E is a "Interrupt priority level", so bits are not
independent in this field - it is a 4-bit integer number, there is no idea
of independent "first bit" or "second bit" inside this field. If there would,
then I'd split it into separate fields bits. So it should be printed something
like "[ H E=1 AE ]" - bits printed only when they are set, uint fields are
printed as "name=value", though I'm not sure if it should be printed if value
is 0. At least that is what are my expectations of how "flags" register should
be presented.
Anton
>
> I see similar things on other archs though, it's
> not just ARC. E.g., the patch resulted in:
>
> --- a/gdb/features/aarch64.c
> +++ b/gdb/features/aarch64.c
> @@ -19,10 +19,10 @@ initialize_tdesc_aarch64 (void)
> feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core");
> type = tdesc_create_flags (feature, "cpsr_flags", 4);
> tdesc_add_flag (type, 0, "SP");
> - tdesc_add_bitfield (type, "", 1, 1);
> + tdesc_add_flag (type, 1, "");
> tdesc_add_bitfield (type, "EL", 2, 3);
> tdesc_add_flag (type, 4, "nRW");
> - tdesc_add_bitfield (type, "", 5, 5);
> + tdesc_add_flag (type, 5, "");
> tdesc_add_flag (type, 6, "F");
> tdesc_add_flag (type, 7, "I");
> tdesc_add_flag (type, 8, "A");
>
> Which leaves "EL" as a 2-bit bitfield.
>
> I'm still terribly confused. :-/
>
> Thanks,
> Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-10-06 13:44 ` Anton Kolesov
@ 2016-10-06 14:44 ` Pedro Alves
2016-10-06 18:43 ` Doug Evans
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2016-10-06 14:44 UTC (permalink / raw)
To: Anton Kolesov, Doug Evans; +Cc: gdb-patches, Wei-cheng Wang
On 10/06/2016 02:44 PM, Anton Kolesov wrote:
> Hi Pedro,
>
>>
>> Note how that left several flags with 2-bit and/or 4-bit
>> long bitfields:
>>
>> tdesc_add_bitfield (type, "E", 1, 2);
>> tdesc_add_bitfield (type, "A", 3, 4);
>> ...
>> tdesc_add_bitfield (type, "E", 1, 4);
>>
>> which I understand means these two fields will
>> be given uint32_t type instead of bool? What does this
>> mean in practice? E.g,. for "A", what do we print when both
>> bits 3 and 4 are clear? What do we print if one
>> of the bits is set and the other is clear?
>
> With regards of ARC flags, if field is longer than one bit, then it should be
> treated as an uint. For example, in arc-v2.c field H means "halt bit", so
> it is a single bit, but E is a "Interrupt priority level", so bits are not
> independent in this field - it is a 4-bit integer number, there is no idea
> of independent "first bit" or "second bit" inside this field. If there would,
> then I'd split it into separate fields bits. So it should be printed something
> like "[ H E=1 AE ]" - bits printed only when they are set, uint fields are
> printed as "name=value", though I'm not sure if it should be printed if value
> is 0. At least that is what are my expectations of how "flags" register should
> be presented.
Thanks. Looks like EL on aarch64 is similar. It's an exception
level, I believe.
To confirm what happens with uint bitfields within flags, I hacked
my local x86-64 GDB with:
--- c/gdb/features/i386/64bit-core.xml
+++ w/gdb/features/i386/64bit-core.xml
@@ -10,8 +10,7 @@
<flags id="i386_eflags" size="4">
<field name="CF" start="0" end="0"/>
<field name="" start="1" end="1"/>
- <field name="PF" start="2" end="2"/>
- <field name="AF" start="4" end="4"/>
+ <field name="PF" start="2" end="4"/>
<field name="ZF" start="6" end="6"/>
<field name="SF" start="7" end="7"/>
<field name="TF" start="8" end="8"/>
and (after regenerating the gdb/feature/ .c files.), I see:
(gdb) p $eflags = 0
$1 = [ PF=0 ]
(gdb) p $eflags = 0xffffffff
$2 = [ CF PF=7 ZF SF TF IF DF OF NT RF VM AC VIF VIP ID ]
So =0 is always shown for these. Debatable, but that seems
like just a presentation thing. Sorry for all my confusions.
I'll go close the PR, and unblock 7.12!
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-10-06 14:44 ` Pedro Alves
@ 2016-10-06 18:43 ` Doug Evans
0 siblings, 0 replies; 14+ messages in thread
From: Doug Evans @ 2016-10-06 18:43 UTC (permalink / raw)
To: Pedro Alves; +Cc: Anton Kolesov, gdb-patches, Wei-cheng Wang
On Thu, Oct 6, 2016 at 7:44 AM, Pedro Alves <palves@redhat.com> wrote:
>
> On 10/06/2016 02:44 PM, Anton Kolesov wrote:
> > Hi Pedro,
> >
> >>
> >> Note how that left several flags with 2-bit and/or 4-bit
> >> long bitfields:
> >>
> >> tdesc_add_bitfield (type, "E", 1, 2);
> >> tdesc_add_bitfield (type, "A", 3, 4);
> >> ...
> >> tdesc_add_bitfield (type, "E", 1, 4);
> >>
> >> which I understand means these two fields will
> >> be given uint32_t type instead of bool? What does this
> >> mean in practice? E.g,. for "A", what do we print when both
> >> bits 3 and 4 are clear? What do we print if one
> >> of the bits is set and the other is clear?
> >
> > With regards of ARC flags, if field is longer than one bit, then it should be
> > treated as an uint. For example, in arc-v2.c field H means "halt bit", so
> > it is a single bit, but E is a "Interrupt priority level", so bits are not
> > independent in this field - it is a 4-bit integer number, there is no idea
> > of independent "first bit" or "second bit" inside this field. If there would,
> > then I'd split it into separate fields bits. So it should be printed something
> > like "[ H E=1 AE ]" - bits printed only when they are set, uint fields are
> > printed as "name=value", though I'm not sure if it should be printed if value
> > is 0. At least that is what are my expectations of how "flags" register should
> > be presented.
>
> Thanks. Looks like EL on aarch64 is similar. It's an exception
> level, I believe.
Correct. This field is actually what prompted me to want to improve on
the status quo.
>
>
> To confirm what happens with uint bitfields within flags, I hacked
> my local x86-64 GDB with:
>
> --- c/gdb/features/i386/64bit-core.xml
> +++ w/gdb/features/i386/64bit-core.xml
> @@ -10,8 +10,7 @@
> <flags id="i386_eflags" size="4">
> <field name="CF" start="0" end="0"/>
> <field name="" start="1" end="1"/>
> - <field name="PF" start="2" end="2"/>
> - <field name="AF" start="4" end="4"/>
> + <field name="PF" start="2" end="4"/>
> <field name="ZF" start="6" end="6"/>
> <field name="SF" start="7" end="7"/>
> <field name="TF" start="8" end="8"/>
>
> and (after regenerating the gdb/feature/ .c files.), I see:
>
> (gdb) p $eflags = 0
> $1 = [ PF=0 ]
> (gdb) p $eflags = 0xffffffff
> $2 = [ CF PF=7 ZF SF TF IF DF OF NT RF VM AC VIF VIP ID ]
>
> So =0 is always shown for these. Debatable, but that seems
> like just a presentation thing.
It felt like if the value is a uint, then 0 should be treated no
differently so that's what I went with.
> Sorry for all my confusions.
> I'll go close the PR, and unblock 7.12!
Sorry for not getting the patch into master, and thanks for doing so.
Another consequence of a distracted summer.
Let me know if you need anything more from me.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
2016-08-15 19:28 Doug Evans
@ 2016-10-06 11:22 ` Pedro Alves
0 siblings, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2016-10-06 11:22 UTC (permalink / raw)
To: Doug Evans; +Cc: gdb-patches, Wei-cheng Wang
On 08/15/2016 08:27 PM, Doug Evans wrote:
> Here is what I committed (to main and gdb-7.12-branch).
> Same as before except I needed to update a testcase.
I was looking at closing PR 20448 [1], but then noticed that
the back compatibility was still broken on master. Turned out that the
problem was simply that the fix never actually made it to master. I've pushed
it to master now, after re-regenerating the features c files. That
regenerated a couple newer files that have gone in meanwhile:
* features/arc-arcompact.c: Regenerate.
* features/arc-v2.c: Regenerate.
That raises a point that I'll follow up on in the other subthread.
[1] - https://sourceware.org/bugzilla/show_bug.cgi?id=20448
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/5]: Enhancements to "flags": i386 cleanup
@ 2016-08-15 19:28 Doug Evans
2016-10-06 11:22 ` Pedro Alves
0 siblings, 1 reply; 14+ messages in thread
From: Doug Evans @ 2016-08-15 19:28 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Wei-cheng Wang
Pedro Alves writes:
> On 08/08/2016 09:33 PM, Doug Evans wrote:
>
> > Hi.
> > Sorry for the trouble.
> >
> > I think(!) these two patches fix things.
> > Basically, I made "end" required again, and made single bit fields
> > default to bool,
> > and then tweaked a couple of xml files to minimize changes.
> >
> > https://sourceware.org/ml/gdb-patches/2016-08/msg00105.html
> > https://sourceware.org/ml/gdb-patches/2016-08/msg00106.html
> >
> > It occurs to me I need to update the docs too.
> > Additional patch to follow.
>
> Thanks! All looks good to me.
Here is what I committed (to main and gdb-7.12-branch).
Same as before except I needed to update a testcase.
2016-08-15 Doug Evans <dje@google.com>
* features/aarch64-core.xml (cpsr_flags): Elide "type" and specify
"end" in all fields.
* features/aarch64.c: Regenerate.
* features/i386/32bit-mpx.xml (_bndcfgu): Specify type of "preserved"
and "enabled" fields. Correct size of "enabled" field.
* features/i386/64bit-mpx.xml (_bndcfgu): Specify type of "preserved"
and "enabled" fields.
* features/i386/i386-avx-mpx-linux.c: Regenerate.
* features/i386/i386-avx-mpx.c: Regenerate.
* features/i386/i386-avx512-linux.c: Regenerate.
* features/i386/i386-avx512.c: Regenerate.
* features/i386/i386-mpx-linux.c: Regenerate.
* features/i386/i386-mpx.c: Regenerate.
* xml-tdesc.c (tdesc_start_field): Require "end" spec. Single bit
fields default to "bool" type.
Revert 2016-03-15 Doug Evans <dje@google.com>
* features/i386/32bit-core.xml (i386_eflags): Remove "end" spec.
* features/i386/32bit-sse.xml (i386_eflags): Ditto.
* features/i386/64bit-core.xml (i386_eflags): Ditto.
* features/i386/64bit-sse.xml (i386_eflags): Ditto.
* features/i386/x32-core.xml (i386_eflags): Ditto.
doc/
* gdb.texinfo (Target Description Format): Update docs on "end"
field spec and field default type.
testsuite/
* gdb.xml/extra-regs.xml: Update, end field now required, default type
for single bitfields is bool.
* gdb.xml/tdesc-regs.exp: Ditto.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f5dde61..0b5ec39 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -40681,16 +40681,13 @@ Bitfield values may be named with the empty
string, @samp{""},
in which case the field is ``filler'' and its value is not printed.
Not all bits need to be specified, so ``filler'' fields are optional.
-The @var{start} value is required, and @var{end} and @var{type}
-are optional.
+The @var{start} and @var{end} values are required, and @var{type}
+is optional.
The field's @var{start} must be less than or equal to its @var{end},
and zero represents the least significant bit.
-The default value of @var{end} is @var{start}, a single bit field.
-The default value of @var{type} depends on whether the
-@var{end} was specified. If @var{end} is specified then the default
-value of @var{type} is an unsigned integer. If @var{end} is unspecified
-then the default value of @var{type} is @code{bool}.
+The default value of @var{type} is @code{bool} for single bit fields,
+and an unsigned integer otherwise.
Which to choose? Structures or flags?
diff --git a/gdb/features/aarch64-core.xml b/gdb/features/aarch64-core.xml
index 8f96296..7ff064d 100644
--- a/gdb/features/aarch64-core.xml
+++ b/gdb/features/aarch64-core.xml
@@ -44,23 +44,23 @@
<reg name="pc" bitsize="64" type="code_ptr"/>
<flags id="cpsr_flags" size="4">
- <field name="SP" start="0" type="bool"/>
+ <field name="SP" start="0" end="0"/>
<field name="" start="1" end="1"/>
<field name="EL" start="2" end="3"/>
- <field name="nRW" start="4" type="bool"/>
+ <field name="nRW" start="4" end="4"/>
<field name="" start="5" end="5"/>
- <field name="F" start="6" type="bool"/>
- <field name="I" start="7" type="bool"/>
- <field name="A" start="8" type="bool"/>
- <field name="D" start="9" type="bool"/>
+ <field name="F" start="6" end="6"/>
+ <field name="I" start="7" end="7"/>
+ <field name="A" start="8" end="8"/>
+ <field name="D" start="9" end="9"/>
- <field name="IL" start="20" type="bool"/>
- <field name="SS" start="21" type="bool"/>
+ <field name="IL" start="20" end="20"/>
+ <field name="SS" start="21" end="21"/>
- <field name="V" start="28" type="bool"/>
- <field name="C" start="29" type="bool"/>
- <field name="Z" start="30" type="bool"/>
- <field name="N" start="31" type="bool"/>
+ <field name="V" start="28" end="28"/>
+ <field name="C" start="29" end="29"/>
+ <field name="Z" start="30" end="30"/>
+ <field name="N" start="31" end="31"/>
</flags>
<reg name="cpsr" bitsize="32" type="cpsr_flags"/>
diff --git a/gdb/features/aarch64.c b/gdb/features/aarch64.c
index cec6956..e9eaed8 100644
--- a/gdb/features/aarch64.c
+++ b/gdb/features/aarch64.c
@@ -19,10 +19,10 @@ initialize_tdesc_aarch64 (void)
feature = tdesc_create_feature (result, "org.gnu.gdb.aarch64.core");
type = tdesc_create_flags (feature, "cpsr_flags", 4);
tdesc_add_flag (type, 0, "SP");
- tdesc_add_bitfield (type, "", 1, 1);
+ tdesc_add_flag (type, 1, "");
tdesc_add_bitfield (type, "EL", 2, 3);
tdesc_add_flag (type, 4, "nRW");
- tdesc_add_bitfield (type, "", 5, 5);
+ tdesc_add_flag (type, 5, "");
tdesc_add_flag (type, 6, "F");
tdesc_add_flag (type, 7, "I");
tdesc_add_flag (type, 8, "A");
diff --git a/gdb/features/i386/32bit-core.xml
b/gdb/features/i386/32bit-core.xml
index b00d913..a27863f 100644
--- a/gdb/features/i386/32bit-core.xml
+++ b/gdb/features/i386/32bit-core.xml
@@ -8,23 +8,23 @@
<!DOCTYPE feature SYSTEM "gdb-target.dtd">
<feature name="org.gnu.gdb.i386.core">
<flags id="i386_eflags" size="4">
- <field name="CF" start="0"/>
- <field name="" start="1"/>
- <field name="PF" start="2"/>
- <field name="AF" start="4"/>
- <field name="ZF" start="6"/>
- <field name="SF" start="7"/>
- <field name="TF" start="8"/>
- <field name="IF" start="9"/>
- <field name="DF" start="10"/>
- <field name="OF" start="11"/>
- <field name="NT" start="14"/>
- <field name="RF" start="16"/>
- <field name="VM" start="17"/>
- <field name="AC" start="18"/>
- <field name="VIF" start="19"/>
- <field name="VIP" start="20"/>
- <field name="ID" start="21"/>
+ <field name="CF" start="0" end="0"/>
+ <field name="" start="1" end="1"/>
+ <field name="PF" start="2" end="2"/>
+ <field name="AF" start="4" end="4"/>
+ <field name="ZF" start="6" end="6"/>
+ <field name="SF" start="7" end="7"/>
+ <field name="TF" start="8" end="8"/>
+ <field name="IF" start="9" end="9"/>
+ <field name="DF" start="10" end="10"/>
+ <field name="OF" start="11" end="11"/>
+ <field name="NT" start="14" end="14"/>
+ <field name="RF" start="16" end="16"/>
+ <field name="VM" start="17" end="17"/>
+ <field name="AC" start="18" end="18"/>
+ <field name="VIF" start="19" end="19"/>
+ <field name="VIP" start="20" end="20"/>
+ <field name="ID" start="21" end="21"/>
</flags>
<reg name="eax" bitsize="32" type="int32"/>
diff --git a/gdb/features/i386/32bit-mpx.xml
b/gdb/features/i386/32bit-mpx.xml
index b1c0615..8d319cf 100644
--- a/gdb/features/i386/32bit-mpx.xml
+++ b/gdb/features/i386/32bit-mpx.xml
@@ -25,8 +25,10 @@
<struct id="_bndcfgu" size="8">
<field name="base" start="12" end="31" />
<field name="reserved" start="2" end="11"/>
- <field name="preserved" start="1" end="1"/>
- <field name="enabled" start="0" end="1"/>
+ <!-- Explicitly set the type here, otherwise it defaults to bool.
+ Perhaps this should be uint32, but the container type has size
8. -->
+ <field name="preserved" start="1" end="1" type="uint64"/>
+ <field name="enabled" start="0" end="0" type="uint64"/>
</struct>
<union id="cfgu">
diff --git a/gdb/features/i386/32bit-sse.xml
b/gdb/features/i386/32bit-sse.xml
index 4448a7e..5a44d1e 100644
--- a/gdb/features/i386/32bit-sse.xml
+++ b/gdb/features/i386/32bit-sse.xml
@@ -23,20 +23,20 @@
<field name="uint128" type="uint128"/>
</union>
<flags id="i386_mxcsr" size="4">
- <field name="IE" start="0"/>
- <field name="DE" start="1"/>
- <field name="ZE" start="2"/>
- <field name="OE" start="3"/>
- <field name="UE" start="4"/>
- <field name="PE" start="5"/>
- <field name="DAZ" start="6"/>
- <field name="IM" start="7"/>
- <field name="DM" start="8"/>
- <field name="ZM" start="9"/>
- <field name="OM" start="10"/>
- <field name="UM" start="11"/>
- <field name="PM" start="12"/>
- <field name="FZ" start="15"/>
+ <field name="IE" start="0" end="0"/>
+ <field name="DE" start="1" end="1"/>
+ <field name="ZE" start="2" end="2"/>
+ <field name="OE" start="3" end="3"/>
+ <field name="UE" start="4" end="4"/>
+ <field name="PE" start="5" end="5"/>
+ <field name="DAZ" start="6" end="6"/>
+ <field name="IM" start="7" end="7"/>
+ <field name="DM" start="8" end="8"/>
+ <field name="ZM" start="9" end="9"/>
+ <field name="OM" start="10" end="10"/>
+ <field name="UM" start="11" end="11"/>
+ <field name="PM" start="12" end="12"/>
+ <field name="FZ" start="15" end="15"/>
</flags>
<reg name="xmm0" bitsize="128" type="vec128" regnum="32"/>
diff --git a/gdb/features/i386/64bit-core.xml
b/gdb/features/i386/64bit-core.xml
index 6e847c1..92f4e87 100644
--- a/gdb/features/i386/64bit-core.xml
+++ b/gdb/features/i386/64bit-core.xml
@@ -8,23 +8,23 @@
<!DOCTYPE feature SYSTEM "gdb-target.dtd">
<feature name="org.gnu.gdb.i386.core">
<flags id="i386_eflags" size="4">
- <field name="CF" start="0"/>
- <field name="" start="1"/>
- <field name="PF" start="2"/>
- <field name="AF" start="4"/>
- <field name="ZF" start="6"/>
- <field name="SF" start="7"/>
- <field name="TF" start="8"/>
- <field name="IF" start="9"/>
- <field name="DF" start="10"/>
- <field name="OF" start="11"/>
- <field name="NT" start="14"/>
- <field name="RF" start="16"/>
- <field name="VM" start="17"/>
- <field name="AC" start="18"/>
- <field name="VIF" start="19"/>
- <field name="VIP" start="20"/>
- <field name="ID" start="21"/>
+ <field name="CF" start="0" end="0"/>
+ <field name="" start="1" end="1"/>
+ <field name="PF" start="2" end="2"/>
+ <field name="AF" start="4" end="4"/>
+ <field name="ZF" start="6" end="6"/>
+ <field name="SF" start="7" end="7"/>
+ <field name="TF" start="8" end="8"/>
+ <field name="IF" start="9" end="9"/>
+ <field name="DF" start="10" end="10"/>
+ <field name="OF" start="11" end="11"/>
+ <field name="NT" start="14" end="14"/>
+ <field name="RF" start="16" end="16"/>
+ <field name="VM" start="17" end="17"/>
+ <field name="AC" start="18" end="18"/>
+ <field name="VIF" start="19" end="19"/>
+ <field name="VIP" start="20" end="20"/>
+ <field name="ID" start="21" end="21"/>
</flags>
<reg name="rax" bitsize="64" type="int64"/>
diff --git a/gdb/features/i386/64bit-mpx.xml
b/gdb/features/i386/64bit-mpx.xml
index 279b537..877cc0f 100644
--- a/gdb/features/i386/64bit-mpx.xml
+++ b/gdb/features/i386/64bit-mpx.xml
@@ -25,8 +25,9 @@
<struct id="_bndcfgu" size="8">
<field name="base" start="12" end="63"/>
<field name="reserved" start="2" end="11"/>
- <field name="preserved" start="1" end="1"/>
- <field name="enabled" start="0" end="0"/>
+ <!-- Explicitly set the type here, otherwise it defaults to bool. -->
+ <field name="preserved" start="1" end="1" type="uint64"/>
+ <field name="enabled" start="0" end="0" type="uint64"/>
</struct>
<union id="cfgu">
diff --git a/gdb/features/i386/64bit-sse.xml
b/gdb/features/i386/64bit-sse.xml
index dd6a850..2a5271e 100644
--- a/gdb/features/i386/64bit-sse.xml
+++ b/gdb/features/i386/64bit-sse.xml
@@ -23,20 +23,20 @@
<field name="uint128" type="uint128"/>
</union>
<flags id="i386_mxcsr" size="4">
- <field name="IE" start="0"/>
- <field name="DE" start="1"/>
- <field name="ZE" start="2"/>
- <field name="OE" start="3"/>
- <field name="UE" start="4"/>
- <field name="PE" start="5"/>
- <field name="DAZ" start="6"/>
- <field name="IM" start="7"/>
- <field name="DM" start="8"/>
- <field name="ZM" start="9"/>
- <field name="OM" start="10"/>
- <field name="UM" start="11"/>
- <field name="PM" start="12"/>
- <field name="FZ" start="15"/>
+ <field name="IE" start="0" end="0"/>
+ <field name="DE" start="1" end="1"/>
+ <field name="ZE" start="2" end="2"/>
+ <field name="OE" start="3" end="3"/>
+ <field name="UE" start="4" end="4"/>
+ <field name="PE" start="5" end="5"/>
+ <field name="DAZ" start="6" end="6"/>
+ <field name="IM" start="7" end="7"/>
+ <field name="DM" start="8" end="8"/>
+ <field name="ZM" start="9" end="9"/>
+ <field name="OM" start="10" end="10"/>
+ <field name="UM" start="11" end="11"/>
+ <field name="PM" start="12" end="12"/>
+ <field name="FZ" start="15" end="15"/>
</flags>
<reg name="xmm0" bitsize="128" type="vec128" regnum="40"/>
diff --git a/gdb/features/i386/i386-avx-mpx-linux.c
b/gdb/features/i386/i386-avx-mpx-linux.c
index 941f2b3..4b27bfc 100644
--- a/gdb/features/i386/i386-avx-mpx-linux.c
+++ b/gdb/features/i386/i386-avx-mpx-linux.c
@@ -168,7 +168,7 @@ initialize_tdesc_i386_avx_mpx_linux (void)
tdesc_add_bitfield (type, "base", 12, 31);
tdesc_add_bitfield (type, "reserved", 2, 11);
tdesc_add_bitfield (type, "preserved", 1, 1);
- tdesc_add_bitfield (type, "enabled", 0, 1);
+ tdesc_add_bitfield (type, "enabled", 0, 0);
type = tdesc_create_union (feature, "cfgu");
field_type = tdesc_named_type (feature, "data_ptr");
diff --git a/gdb/features/i386/i386-avx-mpx.c
b/gdb/features/i386/i386-avx-mpx.c
index d822aac..b27b40a 100644
--- a/gdb/features/i386/i386-avx-mpx.c
+++ b/gdb/features/i386/i386-avx-mpx.c
@@ -163,7 +163,7 @@ initialize_tdesc_i386_avx_mpx (void)
tdesc_add_bitfield (type, "base", 12, 31);
tdesc_add_bitfield (type, "reserved", 2, 11);
tdesc_add_bitfield (type, "preserved", 1, 1);
- tdesc_add_bitfield (type, "enabled", 0, 1);
+ tdesc_add_bitfield (type, "enabled", 0, 0);
type = tdesc_create_union (feature, "cfgu");
field_type = tdesc_named_type (feature, "data_ptr");
diff --git a/gdb/features/i386/i386-avx512-linux.c
b/gdb/features/i386/i386-avx512-linux.c
index 47a3319..0d3ab22 100644
--- a/gdb/features/i386/i386-avx512-linux.c
+++ b/gdb/features/i386/i386-avx512-linux.c
@@ -168,7 +168,7 @@ initialize_tdesc_i386_avx512_linux (void)
tdesc_add_bitfield (type, "base", 12, 31);
tdesc_add_bitfield (type, "reserved", 2, 11);
tdesc_add_bitfield (type, "preserved", 1, 1);
- tdesc_add_bitfield (type, "enabled", 0, 1);
+ tdesc_add_bitfield (type, "enabled", 0, 0);
type = tdesc_create_union (feature, "cfgu");
field_type = tdesc_named_type (feature, "data_ptr");
diff --git a/gdb/features/i386/i386-avx512.c
b/gdb/features/i386/i386-avx512.c
index 6e8cb55..1cb68a1 100644
--- a/gdb/features/i386/i386-avx512.c
+++ b/gdb/features/i386/i386-avx512.c
@@ -163,7 +163,7 @@ initialize_tdesc_i386_avx512 (void)
tdesc_add_bitfield (type, "base", 12, 31);
tdesc_add_bitfield (type, "reserved", 2, 11);
tdesc_add_bitfield (type, "preserved", 1, 1);
- tdesc_add_bitfield (type, "enabled", 0, 1);
+ tdesc_add_bitfield (type, "enabled", 0, 0);
type = tdesc_create_union (feature, "cfgu");
field_type = tdesc_named_type (feature, "data_ptr");
diff --git a/gdb/features/i386/i386-mpx-linux.c
b/gdb/features/i386/i386-mpx-linux.c
index 298b7ff..43ea192 100644
--- a/gdb/features/i386/i386-mpx-linux.c
+++ b/gdb/features/i386/i386-mpx-linux.c
@@ -158,7 +158,7 @@ initialize_tdesc_i386_mpx_linux (void)
tdesc_add_bitfield (type, "base", 12, 31);
tdesc_add_bitfield (type, "reserved", 2, 11);
tdesc_add_bitfield (type, "preserved", 1, 1);
- tdesc_add_bitfield (type, "enabled", 0, 1);
+ tdesc_add_bitfield (type, "enabled", 0, 0);
type = tdesc_create_union (feature, "cfgu");
field_type = tdesc_named_type (feature, "data_ptr");
diff --git a/gdb/features/i386/i386-mpx.c b/gdb/features/i386/i386-mpx.c
index c19af55..e832d2e 100644
--- a/gdb/features/i386/i386-mpx.c
+++ b/gdb/features/i386/i386-mpx.c
@@ -153,7 +153,7 @@ initialize_tdesc_i386_mpx (void)
tdesc_add_bitfield (type, "base", 12, 31);
tdesc_add_bitfield (type, "reserved", 2, 11);
tdesc_add_bitfield (type, "preserved", 1, 1);
- tdesc_add_bitfield (type, "enabled", 0, 1);
+ tdesc_add_bitfield (type, "enabled", 0, 0);
type = tdesc_create_union (feature, "cfgu");
field_type = tdesc_named_type (feature, "data_ptr");
diff --git a/gdb/features/i386/x32-core.xml b/gdb/features/i386/x32-core.xml
index c03cdea..ab51ffc 100644
--- a/gdb/features/i386/x32-core.xml
+++ b/gdb/features/i386/x32-core.xml
@@ -8,23 +8,23 @@
<!DOCTYPE feature SYSTEM "gdb-target.dtd">
<feature name="org.gnu.gdb.i386.core">
<flags id="i386_eflags" size="4">
- <field name="CF" start="0"/>
- <field name="" start="1"/>
- <field name="PF" start="2"/>
- <field name="AF" start="4"/>
- <field name="ZF" start="6"/>
- <field name="SF" start="7"/>
- <field name="TF" start="8"/>
- <field name="IF" start="9"/>
- <field name="DF" start="10"/>
- <field name="OF" start="11"/>
- <field name="NT" start="14"/>
- <field name="RF" start="16"/>
- <field name="VM" start="17"/>
- <field name="AC" start="18"/>
- <field name="VIF" start="19"/>
- <field name="VIP" start="20"/>
- <field name="ID" start="21"/>
+ <field name="CF" start="0" end="0"/>
+ <field name="" start="1" end="1"/>
+ <field name="PF" start="2" end="2"/>
+ <field name="AF" start="4" end="4"/>
+ <field name="ZF" start="6" end="6"/>
+ <field name="SF" start="7" end="7"/>
+ <field name="TF" start="8" end="8"/>
+ <field name="IF" start="9" end="9"/>
+ <field name="DF" start="10" end="10"/>
+ <field name="OF" start="11" end="11"/>
+ <field name="NT" start="14" end="14"/>
+ <field name="RF" start="16" end="16"/>
+ <field name="VM" start="17" end="17"/>
+ <field name="AC" start="18" end="18"/>
+ <field name="VIF" start="19" end="19"/>
+ <field name="VIP" start="20" end="20"/>
+ <field name="ID" start="21" end="21"/>
</flags>
<reg name="rax" bitsize="64" type="int64"/>
diff --git a/gdb/testsuite/gdb.xml/extra-regs.xml
b/gdb/testsuite/gdb.xml/extra-regs.xml
index cbbaf76..997d659 100644
--- a/gdb/testsuite/gdb.xml/extra-regs.xml
+++ b/gdb/testsuite/gdb.xml/extra-regs.xml
@@ -15,12 +15,12 @@
<struct id="struct2" size="8">
<field name="f1" start="0" end="34"/>
- <field name="f2" start="63" end="63"/>
+ <field name="f2" start="63" end="63" type="uint64"/>
</struct>
<flags id="flags" size="4">
<field name="X" start="0" end="0"/>
- <field name="Y" start="2" end="2"/>
+ <field name="Y" start="2" end="2" type="uint32"/>
</flags>
<enum id="Z_values" size="4">
@@ -31,18 +31,16 @@
</enum>
<flags id="mixed_flags" size="4">
- <!-- Elided end and type. -->
- <field name="A" start="0"/>
- <!-- Elided end, unsigned int. -->
- <field name="B" start="1" type="uint32"/>
- <!-- Elided end, bool. -->
- <field name="C" start="2" type="bool"/>
- <!-- Elided type, single bitfield. -->
- <field name="D" start="3" end="3"/>
+ <!-- Elided type. -->
+ <field name="A" start="0" end="0"/>
+ <!-- Elided type, multiple bits. -->
+ <field name="B" start="1" end="3"/>
+ <!-- Bool. -->
+ <field name="C" start="4" end="4" type="bool"/>
+ <!-- Unsigned int. -->
+ <field name="D" start="5" end="5" type="uint32"/>
<!-- Anonymous field. -->
- <field name="" start="4" end="5"/>
- <!-- Multi-bit bitfield, elided type. -->
- <field name="E" start="6" end="7"/>
+ <field name="" start="6" end="7"/>
<!-- Enum bitfield. -->
<field name="Z" start="8" end="9" type="Z_values"/>
</flags>
diff --git a/gdb/testsuite/gdb.xml/tdesc-regs.exp
b/gdb/testsuite/gdb.xml/tdesc-regs.exp
index c197e28..e7d8c74 100644
--- a/gdb/testsuite/gdb.xml/tdesc-regs.exp
+++ b/gdb/testsuite/gdb.xml/tdesc-regs.exp
@@ -170,9 +170,9 @@ gdb_test "ptype \$structreg.v4" "type = int8_t
__attribute__ \\(\\(vector_size\\
gdb_test "ptype \$bitfields" \
"type = struct struct2 {\r\n *uint64_t f1 : 35;\r\n *uint64_t f2 :
1;\r\n}"
gdb_test "ptype \$flags" \
- "type = flag flags {\r\n *uint32_t X @0;\r\n *uint32_t Y @2;\r\n}"
+ "type = flag flags {\r\n *bool X @0;\r\n *uint32_t Y @2;\r\n}"
gdb_test "ptype \$mixed_flags" \
- "type = flag mixed_flags {\r\n *bool A @0;\r\n *uint32_t B @1;\r\n
*bool C @2;\r\n *uint32_t D @3;\r\n *uint32_t @4-5;\r\n *uint32_t E
@6-7;\r\n *enum {yes = 1, no = 0, maybe = 2, so} Z @8-9;\r\n}"
+ "type = flag mixed_flags {\r\n *bool A @0;\r\n *uint32_t B @1-3;\r\n
*bool C @4;\r\n *uint32_t D @5;\r\n *uint32_t @6-7;\r\n *enum {yes = 1, no
= 0, maybe = 2, so} Z @8-9;\r\n}"
load_description "core-only.xml" "" "test-regs.xml"
# The extra register from the previous description should be gone.
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index aa58385..eeaf79b 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -382,13 +382,19 @@ tdesc_start_field (struct gdb_xml_parser *parser,
{
struct tdesc_type *t = data->current_type;
+ /* Older versions of gdb can't handle elided end values.
+ Stick with that for now, to help ensure backward compatibility.
+ E.g., If a newer gdbserver is talking to an older gdb. */
+ if (end == -1)
+ gdb_xml_error (parser, _("Missing end value"));
+
if (data->current_type_size == 0)
gdb_xml_error (parser,
_("Bitfields must live in explicitly sized types"));
if (field_type_id != NULL
&& strcmp (field_type_id, "bool") == 0
- && !(start == end || end == -1))
+ && start != end)
{
gdb_xml_error (parser,
_("Boolean fields must be one bit in size"));
@@ -400,29 +406,20 @@ tdesc_start_field (struct gdb_xml_parser *parser,
"64 bits (unsupported)"),
field_name);
- if (end != -1)
- {
- /* Assume that the bit numbering in XML is "lsb-zero". Most
- architectures other than PowerPC use this ordering. In the
- future, we can add an XML tag to indicate "msb-zero"
- numbering. */
- if (start > end)
- gdb_xml_error (parser, _("Bitfield \"%s\" has start after end"),
- field_name);
- if (end >= data->current_type_size * TARGET_CHAR_BIT)
- gdb_xml_error (parser,
- _("Bitfield \"%s\" does not fit in struct"));
- }
+ /* Assume that the bit numbering in XML is "lsb-zero". Most
+ architectures other than PowerPC use this ordering. In the
+ future, we can add an XML tag to indicate "msb-zero" numbering. */
+ if (start > end)
+ gdb_xml_error (parser, _("Bitfield \"%s\" has start after end"),
+ field_name);
+ if (end >= data->current_type_size * TARGET_CHAR_BIT)
+ gdb_xml_error (parser,
+ _("Bitfield \"%s\" does not fit in struct"));
- if (end == -1)
- {
- if (field_type != NULL)
- tdesc_add_typed_bitfield (t, field_name, start, start, field_type);
- else
- tdesc_add_flag (t, start, field_name);
- }
- else if (field_type != NULL)
+ if (field_type != NULL)
tdesc_add_typed_bitfield (t, field_name, start, end, field_type);
+ else if (start == end)
+ tdesc_add_flag (t, start, field_name);
else
tdesc_add_bitfield (t, field_name, start, end);
}
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-10-06 18:43 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-29 23:09 [PATCH 4/5]: Enhancements to "flags": i386 cleanup Doug Evans
2016-07-20 18:18 ` Pedro Alves
2016-07-22 19:16 ` Doug Evans
2016-08-08 15:06 ` Pedro Alves
2016-08-08 20:34 ` Doug Evans
2016-08-09 17:55 ` Pedro Alves
2016-08-11 18:10 ` Pedro Alves
2016-08-11 18:18 ` Doug Evans
2016-10-06 11:33 ` Pedro Alves
2016-10-06 13:44 ` Anton Kolesov
2016-10-06 14:44 ` Pedro Alves
2016-10-06 18:43 ` Doug Evans
2016-08-15 19:28 Doug Evans
2016-10-06 11:22 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).