diff mbox

Fix sizes and types of x86 segment and x87 registers

Message ID 20200201184318.25049-1-b7.10110111@gmail.com
State New, archived
Headers

Commit Message

Ruslan Kabatsayev Feb. 1, 2020, 6:43 p.m. UTC 
  Segment registers are actually 16-bit, and their upper bit doesn't have
the meaning of sign. Currently GDB instead thinks they are signed
32-bit, which makes various debugger front-ends (e.g. QtCreator) display
them in unnatural format like 00000023.

Similar consideration applies to various x87 non-data registers. In
addition, fioff and fooff on IA32 are conceptually pointers, so the
command like "p $fioff" should show them as such, not as decimal
integers. On x86-64 fioff and fooff are not as straightforward, being
only the lower parts of the 48-bit offsets, upper part of which is in
fiseg and foseg, respectively, so this easy type assignment can't be
done.

This patch fixes types and sizes of these 16-bit registers to unsigned
16-bit, and makes types of fioff and fooff on IA32 respectively code_ptr
and data_ptr (on x86_64 both are made uint32).

gdb/ChangeLog:

2020-02-01  Ruslan Kabatsayev  <b7.10110111@gmail.com>

	* features/i386/32bit-core.xml: Fix segment and x87 register sizes and
	types.
	* features/i386/64bit-core.xml: Ditto.
	* features/i386/32bit-core.c: Regenerate.
	* features/i386/64bit-core.c: Regenerate.
---
 gdb/features/i386/32bit-core.c   | 28 ++++++++++++++--------------
 gdb/features/i386/32bit-core.xml | 28 ++++++++++++++--------------
 gdb/features/i386/64bit-core.c   | 28 ++++++++++++++--------------
 gdb/features/i386/64bit-core.xml | 28 ++++++++++++++--------------
 4 files changed, 56 insertions(+), 56 deletions(-)

Comments

John Baldwin March 5, 2020, 9:32 p.m. UTC | #1 
On 2/1/20 10:43 AM, Ruslan Kabatsayev wrote:
> Segment registers are actually 16-bit, and their upper bit doesn't have
> the meaning of sign. Currently GDB instead thinks they are signed
> 32-bit, which makes various debugger front-ends (e.g. QtCreator) display
> them in unnatural format like 00000023.
> 
> Similar consideration applies to various x87 non-data registers. In
> addition, fioff and fooff on IA32 are conceptually pointers, so the
> command like "p $fioff" should show them as such, not as decimal
> integers. On x86-64 fioff and fooff are not as straightforward, being
> only the lower parts of the 48-bit offsets, upper part of which is in
> fiseg and foseg, respectively, so this easy type assignment can't be
> done.
> 
> This patch fixes types and sizes of these 16-bit registers to unsigned
> 16-bit, and makes types of fioff and fooff on IA32 respectively code_ptr
> and data_ptr (on x86_64 both are made uint32).

I'd be happy to see these fixed (segment regs in particular), but I had
worried that this might break any debug stubs that aren't using XML target
descriptions to describe the layout of 'g'?
Ruslan Kabatsayev March 5, 2020, 9:50 p.m. UTC | #2 
On Fri, 6 Mar 2020 at 00:32, John Baldwin <jhb@freebsd.org> wrote:
>
> On 2/1/20 10:43 AM, Ruslan Kabatsayev wrote:
> > Segment registers are actually 16-bit, and their upper bit doesn't have
> > the meaning of sign. Currently GDB instead thinks they are signed
> > 32-bit, which makes various debugger front-ends (e.g. QtCreator) display
> > them in unnatural format like 00000023.
> >
> > Similar consideration applies to various x87 non-data registers. In
> > addition, fioff and fooff on IA32 are conceptually pointers, so the
> > command like "p $fioff" should show them as such, not as decimal
> > integers. On x86-64 fioff and fooff are not as straightforward, being
> > only the lower parts of the 48-bit offsets, upper part of which is in
> > fiseg and foseg, respectively, so this easy type assignment can't be
> > done.
> >
> > This patch fixes types and sizes of these 16-bit registers to unsigned
> > 16-bit, and makes types of fioff and fooff on IA32 respectively code_ptr
> > and data_ptr (on x86_64 both are made uint32).
>
> I'd be happy to see these fixed (segment regs in particular), but I had
> worried that this might break any debug stubs that aren't using XML target
> descriptions to describe the layout of 'g'?

I'm not sure what exactly debug stubs you mean and what "layout of
'g'" means (I guess it's about remote debugging?). But since sending
this patch I've discovered that it for some reason breaks debugging of
some threaded(?) 32-bit apps, giving the following output:

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/libthread_db.so.1".
Cannot find user-level thread for LWP 9577: generic error

The culprit appears to be the size of GS register that this patch
changes—the register which is used as TLS base. If I revert the change
only to it, this breakage goes away.
Do you have any idea where to look for the reason?

>
> --
> John Baldwin
John Baldwin March 5, 2020, 11:57 p.m. UTC | #3 
On 3/5/20 1:50 PM, Ruslan Kabatsayev wrote:
> On Fri, 6 Mar 2020 at 00:32, John Baldwin <jhb@freebsd.org> wrote:
>>
>> On 2/1/20 10:43 AM, Ruslan Kabatsayev wrote:
>>> Segment registers are actually 16-bit, and their upper bit doesn't have
>>> the meaning of sign. Currently GDB instead thinks they are signed
>>> 32-bit, which makes various debugger front-ends (e.g. QtCreator) display
>>> them in unnatural format like 00000023.
>>>
>>> Similar consideration applies to various x87 non-data registers. In
>>> addition, fioff and fooff on IA32 are conceptually pointers, so the
>>> command like "p $fioff" should show them as such, not as decimal
>>> integers. On x86-64 fioff and fooff are not as straightforward, being
>>> only the lower parts of the 48-bit offsets, upper part of which is in
>>> fiseg and foseg, respectively, so this easy type assignment can't be
>>> done.
>>>
>>> This patch fixes types and sizes of these 16-bit registers to unsigned
>>> 16-bit, and makes types of fioff and fooff on IA32 respectively code_ptr
>>> and data_ptr (on x86_64 both are made uint32).
>>
>> I'd be happy to see these fixed (segment regs in particular), but I had
>> worried that this might break any debug stubs that aren't using XML target
>> descriptions to describe the layout of 'g'?
> 
> I'm not sure what exactly debug stubs you mean and what "layout of
> 'g'" means (I guess it's about remote debugging?).

Yes the 'g' packet is the remote protocol request to fetch the base set of
general registers for a given architecture and has a fixed layout per
architecture.

> But since sending
> this patch I've discovered that it for some reason breaks debugging of
> some threaded(?) 32-bit apps, giving the following output:
> 
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib/libthread_db.so.1".
> Cannot find user-level thread for LWP 9577: generic error
> 
> The culprit appears to be the size of GS register that this patch
> changes—the register which is used as TLS base. If I revert the change
> only to it, this breakage goes away.
> Do you have any idea where to look for the reason?

Hmm, not off the top of my head.  I think the libthread_db bits for
Linux are in linux-thread-db.c and you can single step gdb itself to
see why the call into td_ta_map_lwp2thr_p (which points into a function
in libthread_db.so) fails.
diff mbox

Patch

diff --git a/gdb/features/i386/32bit-core.c b/gdb/features/i386/32bit-core.c
index 8f9fbad717b..b1b5d5ad673 100644
--- a/gdb/features/i386/32bit-core.c
+++ b/gdb/features/i386/32bit-core.c
@@ -39,12 +39,12 @@  create_feature_i386_32bit_core (struct target_desc *result, long regnum)
   tdesc_create_reg (feature, "edi", regnum++, 1, NULL, 32, "int32");
   tdesc_create_reg (feature, "eip", regnum++, 1, NULL, 32, "code_ptr");
   tdesc_create_reg (feature, "eflags", regnum++, 1, NULL, 32, "i386_eflags");
-  tdesc_create_reg (feature, "cs", regnum++, 1, NULL, 32, "int32");
-  tdesc_create_reg (feature, "ss", regnum++, 1, NULL, 32, "int32");
-  tdesc_create_reg (feature, "ds", regnum++, 1, NULL, 32, "int32");
-  tdesc_create_reg (feature, "es", regnum++, 1, NULL, 32, "int32");
-  tdesc_create_reg (feature, "fs", regnum++, 1, NULL, 32, "int32");
-  tdesc_create_reg (feature, "gs", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "cs", regnum++, 1, NULL, 16, "uint16");
+  tdesc_create_reg (feature, "ss", regnum++, 1, NULL, 16, "uint16");
+  tdesc_create_reg (feature, "ds", regnum++, 1, NULL, 16, "uint16");
+  tdesc_create_reg (feature, "es", regnum++, 1, NULL, 16, "uint16");
+  tdesc_create_reg (feature, "fs", regnum++, 1, NULL, 16, "uint16");
+  tdesc_create_reg (feature, "gs", regnum++, 1, NULL, 16, "uint16");
   tdesc_create_reg (feature, "st0", regnum++, 1, NULL, 80, "i387_ext");
   tdesc_create_reg (feature, "st1", regnum++, 1, NULL, 80, "i387_ext");
   tdesc_create_reg (feature, "st2", regnum++, 1, NULL, 80, "i387_ext");
@@ -53,13 +53,13 @@  create_feature_i386_32bit_core (struct target_desc *result, long regnum)
   tdesc_create_reg (feature, "st5", regnum++, 1, NULL, 80, "i387_ext");
   tdesc_create_reg (feature, "st6", regnum++, 1, NULL, 80, "i387_ext");
   tdesc_create_reg (feature, "st7", regnum++, 1, NULL, 80, "i387_ext");
-  tdesc_create_reg (feature, "fctrl", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "fstat", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "ftag", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "fiseg", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "fioff", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "foseg", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "fooff", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "fop", regnum++, 1, "float", 32, "int");
+  tdesc_create_reg (feature, "fctrl", regnum++, 1, "float", 16, "uint16");
+  tdesc_create_reg (feature, "fstat", regnum++, 1, "float", 16, "uint16");
+  tdesc_create_reg (feature, "ftag", regnum++, 1, "float", 16, "uint16");
+  tdesc_create_reg (feature, "fiseg", regnum++, 1, "float", 16, "uint16");
+  tdesc_create_reg (feature, "fioff", regnum++, 1, "float", 32, "code_ptr");
+  tdesc_create_reg (feature, "foseg", regnum++, 1, "float", 16, "uint16");
+  tdesc_create_reg (feature, "fooff", regnum++, 1, "float", 32, "data_ptr");
+  tdesc_create_reg (feature, "fop", regnum++, 1, "float", 16, "uint16");
   return regnum;
 }
diff --git a/gdb/features/i386/32bit-core.xml b/gdb/features/i386/32bit-core.xml
index 0958032a805..70e5a1d6c66 100644
--- a/gdb/features/i386/32bit-core.xml
+++ b/gdb/features/i386/32bit-core.xml
@@ -38,12 +38,12 @@ 
 
   <reg name="eip" bitsize="32" type="code_ptr"/>
   <reg name="eflags" bitsize="32" type="i386_eflags"/>
-  <reg name="cs" bitsize="32" type="int32"/>
-  <reg name="ss" bitsize="32" type="int32"/>
-  <reg name="ds" bitsize="32" type="int32"/>
-  <reg name="es" bitsize="32" type="int32"/>
-  <reg name="fs" bitsize="32" type="int32"/>
-  <reg name="gs" bitsize="32" type="int32"/>
+  <reg name="cs" bitsize="16" type="uint16"/>
+  <reg name="ss" bitsize="16" type="uint16"/>
+  <reg name="ds" bitsize="16" type="uint16"/>
+  <reg name="es" bitsize="16" type="uint16"/>
+  <reg name="fs" bitsize="16" type="uint16"/>
+  <reg name="gs" bitsize="16" type="uint16"/>
 
   <reg name="st0" bitsize="80" type="i387_ext"/>
   <reg name="st1" bitsize="80" type="i387_ext"/>
@@ -54,12 +54,12 @@ 
   <reg name="st6" bitsize="80" type="i387_ext"/>
   <reg name="st7" bitsize="80" type="i387_ext"/>
 
-  <reg name="fctrl" bitsize="32" type="int" group="float"/>
-  <reg name="fstat" bitsize="32" type="int" group="float"/>
-  <reg name="ftag" bitsize="32" type="int" group="float"/>
-  <reg name="fiseg" bitsize="32" type="int" group="float"/>
-  <reg name="fioff" bitsize="32" type="int" group="float"/>
-  <reg name="foseg" bitsize="32" type="int" group="float"/>
-  <reg name="fooff" bitsize="32" type="int" group="float"/>
-  <reg name="fop" bitsize="32" type="int" group="float"/>
+  <reg name="fctrl" bitsize="16" type="uint16" group="float"/>
+  <reg name="fstat" bitsize="16" type="uint16" group="float"/>
+  <reg name="ftag" bitsize="16" type="uint16" group="float"/>
+  <reg name="fiseg" bitsize="16" type="uint16" group="float"/>
+  <reg name="fioff" bitsize="32" type="code_ptr" group="float"/>
+  <reg name="foseg" bitsize="16" type="uint16" group="float"/>
+  <reg name="fooff" bitsize="32" type="data_ptr" group="float"/>
+  <reg name="fop" bitsize="16" type="uint16" group="float"/>
 </feature>
diff --git a/gdb/features/i386/64bit-core.c b/gdb/features/i386/64bit-core.c
index 2418fe54e80..9945ce361d6 100644
--- a/gdb/features/i386/64bit-core.c
+++ b/gdb/features/i386/64bit-core.c
@@ -47,12 +47,12 @@  create_feature_i386_64bit_core (struct target_desc *result, long regnum)
   tdesc_create_reg (feature, "r15", regnum++, 1, NULL, 64, "int64");
   tdesc_create_reg (feature, "rip", regnum++, 1, NULL, 64, "code_ptr");
   tdesc_create_reg (feature, "eflags", regnum++, 1, NULL, 32, "i386_eflags");
-  tdesc_create_reg (feature, "cs", regnum++, 1, NULL, 32, "int32");
-  tdesc_create_reg (feature, "ss", regnum++, 1, NULL, 32, "int32");
-  tdesc_create_reg (feature, "ds", regnum++, 1, NULL, 32, "int32");
-  tdesc_create_reg (feature, "es", regnum++, 1, NULL, 32, "int32");
-  tdesc_create_reg (feature, "fs", regnum++, 1, NULL, 32, "int32");
-  tdesc_create_reg (feature, "gs", regnum++, 1, NULL, 32, "int32");
+  tdesc_create_reg (feature, "cs", regnum++, 1, NULL, 16, "uint32");
+  tdesc_create_reg (feature, "ss", regnum++, 1, NULL, 16, "uint32");
+  tdesc_create_reg (feature, "ds", regnum++, 1, NULL, 16, "uint32");
+  tdesc_create_reg (feature, "es", regnum++, 1, NULL, 16, "uint32");
+  tdesc_create_reg (feature, "fs", regnum++, 1, NULL, 16, "uint32");
+  tdesc_create_reg (feature, "gs", regnum++, 1, NULL, 16, "uint32");
   tdesc_create_reg (feature, "st0", regnum++, 1, NULL, 80, "i387_ext");
   tdesc_create_reg (feature, "st1", regnum++, 1, NULL, 80, "i387_ext");
   tdesc_create_reg (feature, "st2", regnum++, 1, NULL, 80, "i387_ext");
@@ -61,13 +61,13 @@  create_feature_i386_64bit_core (struct target_desc *result, long regnum)
   tdesc_create_reg (feature, "st5", regnum++, 1, NULL, 80, "i387_ext");
   tdesc_create_reg (feature, "st6", regnum++, 1, NULL, 80, "i387_ext");
   tdesc_create_reg (feature, "st7", regnum++, 1, NULL, 80, "i387_ext");
-  tdesc_create_reg (feature, "fctrl", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "fstat", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "ftag", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "fiseg", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "fioff", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "foseg", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "fooff", regnum++, 1, "float", 32, "int");
-  tdesc_create_reg (feature, "fop", regnum++, 1, "float", 32, "int");
+  tdesc_create_reg (feature, "fctrl", regnum++, 1, "float", 16, "uint16");
+  tdesc_create_reg (feature, "fstat", regnum++, 1, "float", 16, "uint16");
+  tdesc_create_reg (feature, "ftag", regnum++, 1, "float", 16, "uint16");
+  tdesc_create_reg (feature, "fiseg", regnum++, 1, "float", 16, "uint16");
+  tdesc_create_reg (feature, "fioff", regnum++, 1, "float", 32, "uint32");
+  tdesc_create_reg (feature, "foseg", regnum++, 1, "float", 16, "uint16");
+  tdesc_create_reg (feature, "fooff", regnum++, 1, "float", 32, "uint32");
+  tdesc_create_reg (feature, "fop", regnum++, 1, "float", 16, "uint16");
   return regnum;
 }
diff --git a/gdb/features/i386/64bit-core.xml b/gdb/features/i386/64bit-core.xml
index 47a631323dd..4e674367730 100644
--- a/gdb/features/i386/64bit-core.xml
+++ b/gdb/features/i386/64bit-core.xml
@@ -46,12 +46,12 @@ 
 
   <reg name="rip" bitsize="64" type="code_ptr"/>
   <reg name="eflags" bitsize="32" type="i386_eflags"/>
-  <reg name="cs" bitsize="32" type="int32"/>
-  <reg name="ss" bitsize="32" type="int32"/>
-  <reg name="ds" bitsize="32" type="int32"/>
-  <reg name="es" bitsize="32" type="int32"/>
-  <reg name="fs" bitsize="32" type="int32"/>
-  <reg name="gs" bitsize="32" type="int32"/>
+  <reg name="cs" bitsize="16" type="uint32"/>
+  <reg name="ss" bitsize="16" type="uint32"/>
+  <reg name="ds" bitsize="16" type="uint32"/>
+  <reg name="es" bitsize="16" type="uint32"/>
+  <reg name="fs" bitsize="16" type="uint32"/>
+  <reg name="gs" bitsize="16" type="uint32"/>
 
   <reg name="st0" bitsize="80" type="i387_ext"/>
   <reg name="st1" bitsize="80" type="i387_ext"/>
@@ -62,12 +62,12 @@ 
   <reg name="st6" bitsize="80" type="i387_ext"/>
   <reg name="st7" bitsize="80" type="i387_ext"/>
 
-  <reg name="fctrl" bitsize="32" type="int" group="float"/>
-  <reg name="fstat" bitsize="32" type="int" group="float"/>
-  <reg name="ftag" bitsize="32" type="int" group="float"/>
-  <reg name="fiseg" bitsize="32" type="int" group="float"/>
-  <reg name="fioff" bitsize="32" type="int" group="float"/>
-  <reg name="foseg" bitsize="32" type="int" group="float"/>
-  <reg name="fooff" bitsize="32" type="int" group="float"/>
-  <reg name="fop" bitsize="32" type="int" group="float"/>
+  <reg name="fctrl" bitsize="16" type="uint16" group="float"/>
+  <reg name="fstat" bitsize="16" type="uint16" group="float"/>
+  <reg name="ftag" bitsize="16" type="uint16" group="float"/>
+  <reg name="fiseg" bitsize="16" type="uint16" group="float"/>
+  <reg name="fioff" bitsize="32" type="uint32" group="float"/>
+  <reg name="foseg" bitsize="16" type="uint16" group="float"/>
+  <reg name="fooff" bitsize="32" type="uint32" group="float"/>
+  <reg name="fop" bitsize="16" type="uint16" group="float"/>
 </feature>