Re: Invalid segment resister value on x86_64-windows

[Joel Brobecker <brobecker@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]

> I'm not sure we can make those changes.  The default layout for the
> registers in the target description is chosen such that it is
> compatible with the "old" register cache layout used for stubs that
> didn't provide a target description.  That layout is still extensively
> used by kernel stubs such as the ones in the Linux and NetBSD kernels.
> I don't think breaking those would be acceptable, as kernel debugging
> is where the segment registers actually matter!

That's something I was concerned about. Here is another approach, which
adds special handling for those registers for Windows. The issue was
that the same code was used for both 32bit and 64bit Windows, so
I needed to extend the tdep structure to be able to determine whether
register N was a segment register or not.

Does the attached patch look good to you?

gdb/ChangeLog:

        * i386-tdep.h (struct gdbarch_tdep): New field
        `first_segment_regnum'.
        * amd64-tdep.c (amd64_init_abi): Set tdep->first_segment_regnum.
        * i386-tdep.c (i386_gdbarch_init): Likewise.
        * windows-nat.c (do_windows_fetch_inferior_registers): Only
        read the first 16 bits of segment register values.

Tested on x86-windows and x86_64-windows using AdaCore's testsuite.
Tested on x86_64-linux with the official testsuite (JIC).

Thank you,
-- 
Joel

[-- Attachment #2: segment-registers-windows.diff --]
[-- Type: text/x-diff, Size: 2931 bytes --]

commit 657e5f87523009cab319c427829543b0231923af
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Wed May 2 09:00:11 2012 -0700

    Segment register reading on Windows targets.
    
    gdb/ChangeLog:
    
            * i386-tdep.h (struct gdbarch_tdep): New field
            `first_segment_regnum'.
            * amd64-tdep.c (amd64_init_abi): Set tdep->first_segment_regnum.
            * i386-tdep.c (i386_gdbarch_init): Likewise.
            * windows-nat.c (do_windows_fetch_inferior_registers): Only
            read the first 16 bits of segment register values.

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 685fa48..27988bf 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -2585,6 +2585,8 @@ amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   const struct target_desc *tdesc = info.target_desc;
 
+  tdep->first_segment_regnum = AMD64_CS_REGNUM;
+
   /* AMD64 generally uses `fxsave' instead of `fsave' for saving its
      floating-point registers.  */
   tdep->sizeof_fpregset = I387_SIZEOF_FXSAVE;
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 769ef42..e6457f6 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -7613,6 +7613,8 @@ i386_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   tdep->gregset_num_regs = I386_NUM_GREGS;
   tdep->sizeof_gregset = 0;
 
+  tdep->first_segment_regnum = I386_CS_REGNUM;
+
   /* Floating-point registers.  */
   tdep->fpregset = NULL;
   tdep->sizeof_fpregset = I387_SIZEOF_FSAVE;
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index f297ae7..e2acbbc 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -75,6 +75,10 @@ struct gdbarch_tdep
   int gregset_num_regs;
   size_t sizeof_gregset;
 
+  /* Register number for the first segment register.  Its value
+     depends on whether we're debugging on an x86 or amd64 target.  */
+  int first_segment_regnum;
+
   /* The general-purpose registers used to pass integers when making
      function calls.  This only applies to amd64, as all parameters
      are passed through the stack on x86.  */
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index f536ed1..a265462 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -456,6 +456,15 @@ do_windows_fetch_inferior_registers (struct regcache *regcache, int r)
       l = (*((long *) context_offset) >> 16) & ((1 << 11) - 1);
       regcache_raw_supply (regcache, r, (char *) &l);
     }
+  else if (r >= tdep->first_segment_regnum
+	   && r < tdep->first_segment_regnum + 6)
+    {
+      /* GDB treats segment registers as 32bit registers, but they are
+	 in fact only 16 bits long.  Make sure we do not read extra
+	 bits from our source buffer.  */
+      l = *((long *) context_offset) & 0xffff;
+      regcache_raw_supply (regcache, r, (char *) &l);
+    }
   else if (r >= 0)
     regcache_raw_supply (regcache, r, context_offset);
   else