[WINDOWS/RFC] Invalid segment resister value on x86_64-windows

[Joel Brobecker <brobecker@adacore.com>]

Hi Chris,

Would you mind telling us what you think on this issue?

Thanks!

> > > Hmm, it should be possible to do this without changing any of the
> > > -tdep.c code, but if you prefer to do it this way, can you rename the
> > > new field into cs_regnum, change the comment into "Register number for
> > > %cs", and move it somewhere around the other _regnum variables that
> > > are already part of the struct?  That makes things a bit more
> > > consistent.
> > 
> > If you prefer, I can do something similar to what we do to handle
> > the register mapping.  Attached is a patch that does that, and only
> > touches windows *-nat code. Untested for now; will test and add
> > a ChangeLog if this is your prefered approach.
> > 
> > This can be simplified a little further and have everything done
> > in windows-nat.c, but it would probably be considered uglier:
> > include "i386-tdep.h" and "amd64-tdep.h" in windows-nat.c, and
> > then have a quick check to determine which platform we are, and
> > then set then implement the segment_register_p function using either
> > a check on sizeof (void *), or a #ifdef [...] #else [...]...

commit ca26ab21b1b2d080e9002d86f23e3e41161af6de
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Wed May 2 14:19:23 2012 -0700

    Segment register reading on Windows targets.

diff --git a/gdb/amd64-windows-nat.c b/gdb/amd64-windows-nat.c
index bc2c047..e5fb0e0 100644
--- a/gdb/amd64-windows-nat.c
+++ b/gdb/amd64-windows-nat.c
@@ -18,6 +18,8 @@
 #include "defs.h"
 #include "windows-nat.h"
 #include "i386-nat.h"
+#include "amd64-tdep.h"
+
 #include <windows.h>
 
 #define context_offset(x) (offsetof (CONTEXT, x))
@@ -85,6 +87,14 @@ static const int mappings[] =
 };
 #undef context_offset
 
+/* segment_register_p_ftype implementation for amd64.  */
+
+static int
+amd64_windows_segment_register_p (int regnum)
+{
+  return regnum >= AMD64_CS_REGNUM && regnum <= AMD64_GS_REGNUM;
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_amd64_windows_nat;
 
@@ -92,5 +102,6 @@ void
 _initialize_amd64_windows_nat (void)
 {
   windows_set_context_register_offsets (mappings);
+  windows_set_segment_register_p (amd64_windows_segment_register_p);
   i386_set_debug_register_length (8);
 }
diff --git a/gdb/i386-windows-nat.c b/gdb/i386-windows-nat.c
index 5d93915..0928c9f 100644
--- a/gdb/i386-windows-nat.c
+++ b/gdb/i386-windows-nat.c
@@ -18,6 +18,7 @@
 #include "defs.h"
 #include "windows-nat.h"
 #include "i386-nat.h"
+#include "i386-tdep.h"
 
 #include <windows.h>
 
@@ -70,6 +71,14 @@ static const int mappings[] =
 };
 #undef context_offset
 
+/* segment_register_p_ftype implementation for x86.  */
+
+static int
+i386_windows_segment_register_p (int regnum)
+{
+  return regnum >= I386_CS_REGNUM && regnum <= I386_GS_REGNUM;
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_i386_windows_nat;
 
@@ -77,5 +86,6 @@ void
 _initialize_i386_windows_nat (void)
 {
   windows_set_context_register_offsets (mappings);
+  windows_set_segment_register_p (i386_windows_segment_register_p);
   i386_set_debug_register_length (4);
 }
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index f536ed1..000c86f 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -243,6 +243,10 @@ static int useshell = 0;		/* use shell for subprocesses */
 
 static const int *mappings;
 
+/* The function to use in order to determine whether a register is
+   a segment register or not.  */
+static segment_register_p_ftype *segment_register_p;
+
 /* This vector maps the target's idea of an exception (extracted
    from the DEBUG_EVENT structure) to GDB's idea.  */
 
@@ -272,6 +276,14 @@ windows_set_context_register_offsets (const int *offsets)
   mappings = offsets;
 }
 
+/* See windows-nat.h.  */
+
+void
+windows_set_segment_register_p (segment_register_p_ftype *fun)
+{
+  segment_register_p = fun;
+}
+
 static void
 check (BOOL ok, const char *file, int line)
 {
@@ -456,6 +468,14 @@ 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 (segment_register_p (r))
+    {
+      /* 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
diff --git a/gdb/windows-nat.h b/gdb/windows-nat.h
index 08200b9..a6cc5ec 100644
--- a/gdb/windows-nat.h
+++ b/gdb/windows-nat.h
@@ -20,5 +20,13 @@
 
 extern void windows_set_context_register_offsets (const int *offsets);
 
+/* A pointer to a function that should return non-zero iff REGNUM
+   corresponds to one of the segment registers.  */
+typedef int (segment_register_p_ftype) (int regnum);
+
+/* Set the function that should be used by this module to determine
+   whether a given register is a segment register or not.  */
+extern void windows_set_segment_register_p (segment_register_p_ftype *fun);
+
 #endif

-- 
Joel