This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: i386.record.floating.point.patch : with more testing and assurity
- From: Michael Snyder <msnyder at vmware dot com>
- To: paawan oza <paawan1982 at yahoo dot com>
- Cc: Hui Zhu <teawater at gmail dot com>, Mark Kettenis <mark dot kettenis at xs4all dot nl>, "pedro at codesourcery dot com" <pedro at codesourcery dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Sun, 26 Jul 2009 12:04:56 -0700
- Subject: Re: i386.record.floating.point.patch : with more testing and assurity
- References: <223451.13359.qm@web112517.mail.gq1.yahoo.com>
paawan oza wrote:
(please try take difference with attached files and see the 'diff' result, it is strange, if you take the patch on windows it gets even more worse with formatting)
OK, I've done as you suggested. Here are my comments
(search for "msnyder):
--- i386-tdep.0726.c 2009-07-26 10:55:37.000013000 -0700
+++ i386-tdep.c 2009-07-26 11:00:19.001044000 -0700
@@ -3139,6 +3139,66 @@
return 0;
}
+
+/* Defines contents to record. */
+#define I386_SAVE_FPU_REGS 0xfffd
+#define I386_SAVE_FPU_ENV 0xfffe
+#define I386_SAVE_FPU_ENV_REG_STACK 0xffff
+
+/* Record the value of floating point registers which will be changed by the
+ current instruction to "record_arch_list". Return -1 if something is wrong.
+*/
+
+static int i386_record_floats (struct gdbarch *gdbarch,
+ struct i386_record_s *ir,
+ uint32_t iregnum)
+{
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ int i;
+
+ /* Oza:Because of floating point insn push/pop of fpu stack is going to
msnyder: "Oza: Because"
+ happen. Currently we store st0-st7 registers, but we need not store all
+ registers all the time, in future we use ftag register and record only
+ those who are not marked as an empty.
+ */
+ if (I386_SAVE_FPU_REGS == iregnum)
+ {
+ for (i = I387_ST0_REGNUM (tdep);i <= I387_ST0_REGNUM (tdep) + 7;i++)
msynder:
for (i = I387_ST0_REGNUM (tdep); i <= I387_ST0_REGNUM (tdep) + 7; i++)
+ {
+ if (record_arch_list_add_reg (ir->regcache, i))
+ return -1;
+ }
+ }
+ else if (I386_SAVE_FPU_ENV == iregnum)
+ {
+ for (i = I387_FCTRL_REGNUM (tdep);i <= I387_FOP_REGNUM (tdep);i++)
msnyder:
for (i = I387_FCTRL_REGNUM (tdep); i <= I387_FOP_REGNUM (tdep); i++)
+ {
msnyder: indent { to the right by two spaces after "for".
+ if (record_arch_list_add_reg (ir->regcache, i))
+ return -1;
+ }
+ }
+ else if (I386_SAVE_FPU_ENV_REG_STACK == iregnum)
+ {
+ for (i = I387_ST0_REGNUM (tdep);i <= I387_FOP_REGNUM (tdep);i++)
msnyder:
for (i = I387_ST0_REGNUM (tdep); i <= I387_FOP_REGNUM (tdep); i++)
+ {
+ if (record_arch_list_add_reg (ir->regcache, i))
+ return -1;
+ }
+ }
+ else if ((iregnum >= I387_ST0_REGNUM (tdep)) &&
+ (iregnum <= I387_FOP_REGNUM (tdep)))
+ {
+ if (record_arch_list_add_reg (ir->regcache,iregnum))
+ return -1;
+ }
+ else
+ {
+ /* Parameter error. */
+ return -1;
+ }
+ return 0;
+}
+
/* Parse the current instruction and record the values of the registers and
memory that will be changed in current instruction to "record_arch_list".
Return -1 if something wrong. */
@@ -3153,6 +3213,7 @@
uint32_t tmpu32;
uint32_t opcode;
struct i386_record_s ir;
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
memset (&ir, 0, sizeof (struct i386_record_s));
ir.regcache = regcache;
@@ -4188,8 +4249,7 @@
}
break;
- /* floats */
- /* It just record the memory change of instrcution. */
+ /* Floats. */
case 0xd8:
case 0xd9:
case 0xda:
@@ -4203,45 +4263,56 @@
ir.reg |= ((opcode & 7) << 3);
if (ir.mod != 3)
{
- /* memory */
+ /* Memory. */
uint32_t addr;
if (i386_record_lea_modrm_addr (&ir, &addr))
return -1;
switch (ir.reg)
{
- case 0x00:
- case 0x01:
case 0x02:
+ case 0x12:
+ case 0x22:
+ case 0x32:
+ /* For fcom, ficom nothing to do. */
+ break;
case 0x03:
+ case 0x13:
+ case 0x23:
+ case 0x33:
+ /* For fcomp, ficomp pop FPU stack, store all. */
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
+ return -1;
+ break;
+ case 0x00:
+ case 0x01:
case 0x04:
case 0x05:
case 0x06:
case 0x07:
case 0x10:
case 0x11:
- case 0x12:
- case 0x13:
case 0x14:
case 0x15:
case 0x16:
case 0x17:
case 0x20:
case 0x21:
- case 0x22:
- case 0x23:
case 0x24:
case 0x25:
case 0x26:
case 0x27:
case 0x30:
case 0x31:
- case 0x32:
- case 0x33:
case 0x34:
case 0x35:
case 0x36:
case 0x37:
+ /* For fadd, fmul, fsub, fsubr, fdiv, fdivr, fiadd, fimul,
+ fisub, fisubr, fidiv, fidivr, modR/M.reg is an extension of code,
+ always affects st(0) register. */
+ if (i386_record_floats (gdbarch, &ir, I387_ST0_REGNUM (tdep)))
+ return -1;
break;
case 0x08:
case 0x0a:
@@ -4250,6 +4321,7 @@
case 0x19:
case 0x1a:
case 0x1b:
+ case 0x1d:
case 0x28:
case 0x29:
case 0x2a:
@@ -4258,9 +4330,14 @@
case 0x39:
case 0x3a:
case 0x3b:
+ case 0x3c:
+ case 0x3d:
switch (ir.reg & 7)
{
case 0:
+ /* Handling fld, fild. */
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
+ return -1;
msnyder: indented too far (two spaces extra):
/* Handling fld, fild. */
if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
return -1;
break;
case 1:
switch (ir.reg >> 4)
@@ -4274,6 +4351,7 @@
return -1;
break;
case 3:
+ break;
msnyder: indented too far:
break;
default:
if (record_arch_list_add_mem (addr, 2))
return -1;
@@ -4284,15 +4362,49 @@
switch (ir.reg >> 4)
{
case 0:
+ if (record_arch_list_add_mem (addr, 4))
+ return -1;
+ if (3 == (ir.reg & 7))
+ {
+ /* For fstp m32fp. */
+ if (i386_record_floats (gdbarch, &ir,
+ I386_SAVE_FPU_REGS))
+ return -1;
+ }
+ break;
msnyder: indented four extra spaces too far.
case 1:
if (record_arch_list_add_mem (addr, 4))
return -1;
+ if ((3 == (ir.reg & 7)) \
+ || (5 == (ir.reg & 7)) \
+ || (7 == (ir.reg & 7)))
msnyder: remove '\' from end of lines:
if ((3 == (ir.reg & 7))
|| (5 == (ir.reg & 7))
|| (7 == (ir.reg & 7)))
+ {
+ /* For fstp insn. */
+ if (i386_record_floats (gdbarch, &ir,
+ I386_SAVE_FPU_REGS))
+ return -1;
+ }
break;
case 2:
if (record_arch_list_add_mem (addr, 8))
return -1;
+ if (3 == (ir.reg & 7))
+ {
+ /* For fstp m64fp. */
+ if (i386_record_floats (gdbarch, &ir,
+ I386_SAVE_FPU_REGS))
+ return -1;
+ }
msnyder: indented too far.
break;
case 3:
+ if ((3 <= (ir.reg & 7)) && (6 <= (ir.reg & 7)))
+ {
+ /* For fistp, fbld, fild, fbstp. */
+ if (i386_record_floats (gdbarch, &ir,
+ I386_SAVE_FPU_REGS))
+ return -1;
+ }
+ /*Fall through */
msnyder: indented too far.
default:
if (record_arch_list_add_mem (addr, 2))
return -1;
@@ -4302,11 +4414,21 @@
}
break;
case 0x0c:
+ /* Insn fldenv. */
+ if (i386_record_floats (gdbarch, &ir,
+ I386_SAVE_FPU_ENV_REG_STACK))
+ return -1;
+ break;
case 0x0d:
- case 0x1d:
+ /* Insn fldcw. */
+ if (i386_record_floats (gdbarch, &ir, I387_FCTRL_REGNUM (tdep)))
+ return -1;
+ break;
case 0x2c:
- case 0x3c:
- case 0x3d:
+ /* Insn frstor. */
+ if (i386_record_floats (gdbarch, &ir,
+ I386_SAVE_FPU_ENV_REG_STACK))
+ return -1;
break;
case 0x0e:
if (ir.dflag)
@@ -4329,6 +4451,9 @@
case 0x3e:
if (record_arch_list_add_mem (addr, 10))
return -1;
+ /* Insn fstp, fbstp. */
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
+ return -1;
break;
case 0x2e:
if (ir.dflag)
@@ -4345,10 +4470,17 @@
}
if (record_arch_list_add_mem (addr, 80))
return -1;
+ /* Insn fsave. */
+ if (i386_record_floats (gdbarch, &ir,
+ I386_SAVE_FPU_ENV_REG_STACK))
+ return -1;
break;
case 0x3f:
if (record_arch_list_add_mem (addr, 8))
return -1;
+ /* Ins fistp. */
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
+ return -1;
break;
default:
ir.addr -= 2;
@@ -4357,8 +4489,205 @@
break;
}
}
+ /* Opcode is an extension of modR/M byte. */
+ else
msnyder: above, indentation level minus two after }
/* Opcode is an extension of modR/M byte. */
else
+ {
+ switch (opcode)
msnyder: above, indentation level plus two after {
switch (opcode)
And then everything below this point needs to be re-indented to the right.
+ {
+ case 0xd8:
+ if (i386_record_floats (gdbarch, &ir, I387_ST0_REGNUM (tdep)))
+ return -1;
+ break;
+ case 0xd9:
+ if (0x0c == (ir.modrm >> 4))
+ {
+ if ((ir.modrm & 0x0f) <= 7)
+ {
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
+ return -1;
msnyder: missing a right-indent.
+ }
+ else
+ {
+ if (i386_record_floats (gdbarch, &ir,
+ I387_ST0_REGNUM (tdep)))
msnyder: missing a right-indent.
+ return -1;
+ /* If only st(0) is changing, then we have already recorded. */
+ if ((ir.modrm & 0x0f) - 0x08)
+ {
+ if (i386_record_floats (gdbarch, &ir,
msnyder: missing a right-indent.
+ I387_ST0_REGNUM (tdep) + \
msnyder: no \ at end of line.
I387_ST0_REGNUM (tdep) +
+ ((ir.modrm & 0x0f) - 0x08)))
+ return -1;
+ }
+ }
+ }
+ else
+ {
msnyder: indentation.
+ switch(ir.modrm)
msnyder: "switch ("
+ {
+ case 0xe0:
+ case 0xe1:
+ case 0xf0:
+ case 0xf5:
+ case 0xf8:
+ case 0xfa:
+ case 0xfc:
+ case 0xfe:
+ case 0xff:
+ if (i386_record_floats (gdbarch, &ir,
+ I387_ST0_REGNUM (tdep)))
+ return -1;
+ break;
+ case 0xf1:
+ case 0xf2:
+ case 0xf3:
+ case 0xf4:
+ case 0xf6:
+ case 0xf7:
+ case 0xe8:
+ case 0xe9:
+ case 0xea:
+ case 0xeb:
+ case 0xec:
+ case 0xed:
+ case 0xee:
+ case 0xf9:
+ case 0xfb:
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
+ return -1;
+ break;
+ case 0xfd:
+ if (i386_record_floats (gdbarch, &ir,
+ I387_ST0_REGNUM (tdep)))
+ return -1;
+ if (i386_record_floats (gdbarch, &ir,
+ I387_ST0_REGNUM (tdep) + 1))
+ return -1;
+ break;
+ }
+ }
+ break;
+ case 0xda:
+ if (0xe9 == ir.modrm)
+ {
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
+ return -1;
msnyder: indentation after {
+ }
+ else if ((0x0c == ir.modrm >> 4) || (0x0d == ir.modrm >> 4))
+ {
+ if (i386_record_floats (gdbarch, &ir, I387_ST0_REGNUM (tdep)))
msnyder: indentation after {
+ return -1;
+ if (((ir.modrm & 0x0f) > 0) && ((ir.modrm & 0x0f) <= 7))
+ {
+ if (i386_record_floats (gdbarch, &ir,
msnyder: indentation after {
+ I387_ST0_REGNUM (tdep) +
+ (ir.modrm & 0x0f)))
+ return -1;
+ }
+ else if ((ir.modrm & 0x0f) - 0x08)
+ {
+ if (i386_record_floats (gdbarch, &ir,
+ I387_ST0_REGNUM (tdep) + \
msnyder: no \ at end of line.
I387_ST0_REGNUM (tdep) +
+ ((ir.modrm & 0x0f) - 0x08)))
+ return -1;
+ }
+ }
+ break;
+ case 0xdb:
+ if (0xe3 == ir.modrm)
+ {
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_ENV))
msnyder: indentation after {
+ return -1;
+ }
+ else if ((0x0c == ir.modrm >> 4) || (0x0d == ir.modrm >> 4))
+ {
+ if (i386_record_floats (gdbarch, &ir, I387_ST0_REGNUM (tdep)))
msnyder: indentation after {
+ return -1;
+ if (((ir.modrm & 0x0f) > 0) && ((ir.modrm & 0x0f) <= 7))
+ {
+ if (i386_record_floats (gdbarch, &ir,
msnyder: indentation after {
+ I387_ST0_REGNUM (tdep) + \
msnyder: no \ at end of line.
I387_ST0_REGNUM (tdep) +
+ (ir.modrm & 0x0f)))
+ return -1;
+ }
+ else if ((ir.modrm & 0x0f) - 0x08)
+ {
+ if (i386_record_floats (gdbarch, &ir,
msnyder: indentation after {
+ I387_ST0_REGNUM (tdep) + \
msnyder: no \ at end of line.
I387_ST0_REGNUM (tdep) +
+ ((ir.modrm & 0x0f) - 0x08)))
+ return -1;
+ }
+ }
+ break;
+ case 0xdc:
+ if ((0x0c == ir.modrm >> 4) \
+ || (0x0d == ir.modrm >> 4) \
+ || (0x0f == ir.modrm >> 4))
msnyder: no \ at end of line.
if ((0x0c == ir.modrm >> 4)
|| (0x0d == ir.modrm >> 4)
|| (0x0f == ir.modrm >> 4))
+ {
+ if ((ir.modrm & 0x0f) <= 7)
+ {
+ if (i386_record_floats (gdbarch, &ir,
+ I387_ST0_REGNUM (tdep) + \
msnyder: no \ at end of line.
I387_ST0_REGNUM (tdep) +
+ (ir.modrm & 0x0f)))
+ return -1;
+ }
+ else
+ {
+ if (i386_record_floats (gdbarch, &ir,
+ I387_ST0_REGNUM (tdep) + \
msnyder: no \ at end of line.
I387_ST0_REGNUM (tdep) +
+ ((ir.modrm & 0x0f) - 0x08)))
+ return -1;
+ }
+ }
+ break;
+ case 0xdd:
+ if (0x0c == ir.modrm >> 4)
+ {
+ if (i386_record_floats (gdbarch, &ir,
+ I387_FTAG_REGNUM (tdep)))
+ return -1;
+ }
+ else if ((0x0d == ir.modrm >> 4) || (0x0e == ir.modrm >> 4))
+ {
+ if ((ir.modrm & 0x0f) <= 7)
+ {
+ if (i386_record_floats (gdbarch, &ir,
+ I387_ST0_REGNUM (tdep) + \
msnyder: no \ at end of line.
I387_ST0_REGNUM (tdep) +
+ (ir.modrm & 0x0f)))
+ return -1;
+ }
+ else
+ {
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
+ return -1;
+ }
+ }
+ break;
+ case 0xde:
+ if ((0x0c == ir.modrm >> 4) \
+ || (0x0e == ir.modrm >> 4) \
+ || (0x0f == ir.modrm >> 4) \
+ || (0xd9 == ir.modrm))
msnyder: no \ at end of line.
if ((0x0c == ir.modrm >> 4)
|| (0x0e == ir.modrm >> 4)
|| (0x0f == ir.modrm >> 4)
|| (0xd9 == ir.modrm))
+ {
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
+ return -1;
+ }
+ break;
+ case 0xdf:
+ if (0xe0 == ir.modrm)
+ {
+ if (record_arch_list_add_reg (ir.regcache, I386_EAX_REGNUM))
+ return -1;
+ }
+ else if ((0x0f == ir.modrm >> 4) || (0x0e == ir.modrm >> 4))
+ {
+ if (i386_record_floats (gdbarch, &ir, I386_SAVE_FPU_REGS))
+ return -1;
+ }
+ break;
+ }
+ }
break;
-
/* string ops */
/* movsS */
case 0xa4:
@@ -4777,10 +5106,17 @@
/* fwait */
/* XXX */
case 0x9b:
- printf_unfiltered (_("Process record doesn't support instruction "
- "fwait.\n"));
- ir.addr -= 1;
- goto no_support;
+ if (target_read_memory (ir.addr, &tmpu8, 1))
+ {
+ if (record_debug)
+ printf_unfiltered (_("Process record: error reading memory at "
+ "addr 0x%s len = 1.\n"),
+ paddress (gdbarch, ir.addr));
msnyder: indentation
printf_unfiltered (_("Process record: error reading memory at "
"addr 0x%s len = 1.\n"),
paddress (gdbarch, ir.addr));
+ return -1;
+ }
+ opcode = (uint32_t) tmpu8;
+ ir.addr++;
+ goto reswitch;
break;
/* int3 */