This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA/prec] Make i386 handle segment register better


Hui Zhu wrote:
On Sun, Aug 30, 2009 at 05:21, Michael Snyder<msnyder@vmware.com> wrote:

And this one is also an if/else. So I guess my questions are:

1) Should you use an "else" in the "String ops" case?

OK.


2) Should we go ahead and record the register changes,
even though we can't record the memory change?

I think even if we cannot record the memory change. Keep record the change of reg is better.

3) Should this be a warning, rather than just a debug message?
I think yes, because if this happens, it actually means that the
record log will be inaccurate.

OK.


I make a new patch for it. Please help me review it.

I think I like this version. Want to check it in?

Michael



2009-08-30 Hui Zhu <teawater@gmail.com>


        * i386-tdep.c (i386_record_s): Add orig_addr.
        (i386_record_check_override): New function.
        (i386_record_lea_modrm): Call i386_record_check_override.
        (i386_process_record): Ditto.

---
 i386-tdep.c |  103 ++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 59 insertions(+), 44 deletions(-)

--- a/i386-tdep.c
+++ b/i386-tdep.c
@@ -2867,6 +2867,7 @@ struct i386_record_s
 {
   struct gdbarch *gdbarch;
   struct regcache *regcache;
+  CORE_ADDR orig_addr;
   CORE_ADDR addr;
   int aflag;
   int dflag;
@@ -3147,6 +3148,26 @@ no_rm:
   return 0;
 }

+static int
+i386_record_check_override (struct i386_record_s *irp)
+{
+  if (irp->override >= 0 && irp->override != X86_RECORD_DS_REGNUM)
+    {
+      ULONGEST tmp, ds;
+
+      regcache_raw_read_unsigned (irp->regcache,
+                                  irp->regmap[irp->override],
+                                  &tmp);
+      regcache_raw_read_unsigned (irp->regcache,
+                                  irp->regmap[X86_RECORD_DS_REGNUM],
+                                  &ds);
+      if (tmp != ds)
+        return 1;
+    }
+
+  return 0;
+}
+
 /* Record the value of the memory that willbe changed in current instruction
    to "record_arch_list".
    Return -1 if something wrong. */
@@ -3157,13 +3178,12 @@ i386_record_lea_modrm (struct i386_recor
   struct gdbarch *gdbarch = irp->gdbarch;
   uint64_t addr;

-  if (irp->override >= 0)
+  if (i386_record_check_override (irp))
     {
-      if (record_debug)
-       printf_unfiltered (_("Process record ignores the memory change "
-                            "of instruction at address %s because it "
-                            "can't get the value of the segment register.\n"),
-                          paddress (gdbarch, irp->addr));
+      warning (_("Process record ignores the memory change "
+                 "of instruction at address %s because it "
+                 "can't get the value of the segment register."),
+               paddress (gdbarch, irp->orig_addr));
       return 0;
     }

@@ -3221,6 +3241,7 @@ i386_process_record (struct gdbarch *gdb
   memset (&ir, 0, sizeof (struct i386_record_s));
   ir.regcache = regcache;
   ir.addr = addr;
+  ir.orig_addr = addr;
   ir.aflag = 1;
   ir.dflag = 1;
   ir.override = -1;
@@ -4039,14 +4060,13 @@ reswitch:
       /* mov EAX */
     case 0xa2:
     case 0xa3:
-      if (ir.override >= 0)
+      if (i386_record_check_override (&ir))
         {
-         if (record_debug)
-           printf_unfiltered (_("Process record ignores the memory change "
-                                "of instruction at address 0x%s because "
-                                "it can't get the value of the segment "
-                                "register.\n"),
-                              paddress (gdbarch, ir.addr));
+         warning (_("Process record ignores the memory change "
+                     "of instruction at address 0x%s because "
+                     "it can't get the value of the segment "
+                     "register."),
+                   paddress (gdbarch, ir.orig_addr));
        }
       else
        {
@@ -4458,27 +4478,24 @@ reswitch:
                                       ir.regmap[X86_RECORD_REDI_REGNUM],
                                       &tmpulongest);

-          regcache_raw_read_unsigned (ir.regcache,
-                                      ir.regmap[X86_RECORD_ES_REGNUM],
-                                      &es);
-          regcache_raw_read_unsigned (ir.regcache,
-                                      ir.regmap[X86_RECORD_DS_REGNUM],
-                                      &ds);
-          if (ir.aflag && (es != ds))
+          ir.override = X86_RECORD_ES_REGNUM;
+          if (ir.aflag && i386_record_check_override (&ir))
             {
               /* addr += ((uint32_t) read_register (I386_ES_REGNUM)) << 4; */
-              if (record_debug)
-                printf_unfiltered (_("Process record ignores the memory "
-                                    "change of instruction at address 0x%s "
-                                    "because it can't get the value of the "
-                                    "ES segment register.\n"),
-                                   paddress (gdbarch, ir.addr));
+              warning (_("Process record ignores the memory "
+                         "change of instruction at address 0x%s "
+                         "because it can't get the value of the "
+                         "ES segment register."),
+                       paddress (gdbarch, ir.orig_addr));
+            }
+          else
+            {
+              if (record_arch_list_add_mem (tmpulongest, 1 << ir.ot))
+                return -1;
             }

           if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ))
             I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM);
-          if (record_arch_list_add_mem (tmpulongest, 1 << ir.ot))
-            return -1;
           if (opcode == 0xa4 || opcode == 0xa5)
             I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RESI_REGNUM);
           I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDI_REGNUM);
@@ -5086,15 +5103,14 @@ reswitch:
                opcode = opcode << 8 | ir.modrm;
                goto no_support;
              }
-           if (ir.override >= 0)
+           if (i386_record_check_override (&ir))
              {
-               if (record_debug)
-                 printf_unfiltered (_("Process record ignores the memory "
-                                      "change of instruction at "
-                                      "address %s because it can't get "
-                                      "the value of the segment "
-                                      "register.\n"),
-                                    paddress (gdbarch, ir.addr));
+               warning (_("Process record ignores the memory "
+                           "change of instruction at "
+                           "address %s because it can't get "
+                           "the value of the segment "
+                           "register."),
+                         paddress (gdbarch, ir.orig_addr));
              }
            else
              {
@@ -5138,15 +5154,14 @@ reswitch:
          else
            {
              /* sidt */
-             if (ir.override >= 0)
+             if (i386_record_check_override (&ir))
                {
-                 if (record_debug)
-                   printf_unfiltered (_("Process record ignores the memory "
-                                        "change of instruction at "
-                                        "address %s because it can't get "
-                                        "the value of the segment "
-                                        "register.\n"),
-                                      paddress (gdbarch, ir.addr));
+                 warning (_("Process record ignores the memory "
+                             "change of instruction at "
+                             "address %s because it can't get "
+                             "the value of the segment "
+                             "register."),
+                           paddress (gdbarch, ir.orig_addr));
                }
              else
                {


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]