* [RFA/prec] Make i386 handle segment register better
@ 2009-08-29 16:12 Hui Zhu
2009-08-29 21:34 ` Michael Snyder
0 siblings, 1 reply; 10+ messages in thread
From: Hui Zhu @ 2009-08-29 16:12 UTC (permalink / raw)
To: gdb-patches ml; +Cc: Michael Snyder
[-- Attachment #1: Type: text/plain, Size: 3167 bytes --]
Hi guys,
In prec-fix-x86-strinsn.txt patch, I add code the compare the ES and
DS to make sure if es if same with ds or not.
I think it works not bad, so I make a patch to check other segment
regiser like it.
Please help me with it.
Thanks,
Hui
2009-08-29 Hui Zhu <teawater@gmail.com>
* i386-tdep.c (i386_record_check_override): New function.
(i386_record_lea_modrm): Call i386_record_check_override.
(i386_process_record): Ditto.
---
i386-tdep.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
--- a/i386-tdep.c
+++ b/i386-tdep.c
@@ -3147,6 +3147,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,7 +3177,7 @@ 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 "
@@ -4039,7 +4059,7 @@ 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 "
@@ -4458,13 +4478,8 @@ 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)
@@ -5086,7 +5101,7 @@ 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 "
@@ -5138,7 +5153,7 @@ reswitch:
else
{
/* sidt */
- if (ir.override >= 0)
+ if (i386_record_check_override (&ir))
{
if (record_debug)
printf_unfiltered (_("Process record ignores the memory "
[-- Attachment #2: prec-i386-override.txt --]
[-- Type: text/plain, Size: 2729 bytes --]
---
i386-tdep.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)
--- a/i386-tdep.c
+++ b/i386-tdep.c
@@ -3147,6 +3147,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,7 +3177,7 @@ 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 "
@@ -4039,7 +4059,7 @@ 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 "
@@ -4458,13 +4478,8 @@ 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)
@@ -5086,7 +5101,7 @@ 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 "
@@ -5138,7 +5153,7 @@ reswitch:
else
{
/* sidt */
- if (ir.override >= 0)
+ if (i386_record_check_override (&ir))
{
if (record_debug)
printf_unfiltered (_("Process record ignores the memory "
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA/prec] Make i386 handle segment register better
2009-08-29 16:12 [RFA/prec] Make i386 handle segment register better Hui Zhu
@ 2009-08-29 21:34 ` Michael Snyder
2009-08-30 3:21 ` Hui Zhu
0 siblings, 1 reply; 10+ messages in thread
From: Michael Snyder @ 2009-08-29 21:34 UTC (permalink / raw)
To: Hui Zhu; +Cc: gdb-patches ml
Hui Zhu wrote:
> Hi guys,
>
> In prec-fix-x86-strinsn.txt patch, I add code the compare the ES and
> DS to make sure if es if same with ds or not.
> I think it works not bad, so I make a patch to check other segment
> regiser like it.
>
> Please help me with it.
Thanks for doing this!
I think it looks good, but I have a couple of questions:
> 2009-08-29 Hui Zhu <teawater@gmail.com>
>
> * i386-tdep.c (i386_record_check_override): New function.
> (i386_record_lea_modrm): Call i386_record_check_override.
> (i386_process_record): Ditto.
>
> ---
> i386-tdep.c | 37 ++++++++++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 11 deletions(-)
>
> --- a/i386-tdep.c
> +++ b/i386-tdep.c
> @@ -3147,6 +3147,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,7 +3177,7 @@ 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 "
In this case, you "return 0", so it is true that we
"ignore the memory change".
In some cases below, you use an "if/else", so it is also
true that we "ignore the memory change".
But in the "String ops" case, there is no "else", so we
really do *not* ignore the memory change.
Should we be consistant, and add an "else" to the string ops case?
See further comments at end.
> @@ -4039,7 +4059,7 @@ 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 "
OK, this one is an "if/else", so you don't record the memory.
> @@ -4458,13 +4478,8 @@ 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)
But in this case, there is no "else", so you still record
the memory even if i386_record_check_override returns true.
> @@ -5086,7 +5101,7 @@ 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 "
This is an "if/else" so you don't record the memory.
> @@ -5138,7 +5153,7 @@ reswitch:
> else
> {
> /* sidt */
> - if (ir.override >= 0)
> + if (i386_record_check_override (&ir))
> {
> if (record_debug)
> printf_unfiltered (_("Process record ignores the memory "
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?
2) Should we go ahead and record the register changes,
even though we can't record the memory change?
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.
That's all for now,
Michael
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA/prec] Make i386 handle segment register better
2009-08-29 21:34 ` Michael Snyder
@ 2009-08-30 3:21 ` Hui Zhu
2009-09-05 2:42 ` Michael Snyder
0 siblings, 1 reply; 10+ messages in thread
From: Hui Zhu @ 2009-08-30 3:21 UTC (permalink / raw)
To: Michael Snyder; +Cc: gdb-patches ml
[-- Attachment #1: Type: text/plain, Size: 11866 bytes --]
On Sun, Aug 30, 2009 at 05:21, Michael Snyder<msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>>
>> Hi guys,
>>
>> In prec-fix-x86-strinsn.txt patch, I add code the compare the ES and
>> DS to make sure if es if same with ds or not.
>> I think it works not bad, so I make a patch to check other segment
>> regiser like it.
>>
>> Please help me with it.
>
> Thanks for doing this!
> I think it looks good, but I have a couple of questions:
>
>> 2009-08-29 Hui Zhu <teawater@gmail.com>
>>
>> * i386-tdep.c (i386_record_check_override): New function.
>> (i386_record_lea_modrm): Call i386_record_check_override.
>> (i386_process_record): Ditto.
>>
>> ---
>> i386-tdep.c | 37 ++++++++++++++++++++++++++-----------
>> 1 file changed, 26 insertions(+), 11 deletions(-)
>>
>> --- a/i386-tdep.c
>> +++ b/i386-tdep.c
>> @@ -3147,6 +3147,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,7 +3177,7 @@ 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 "
>
> In this case, you "return 0", so it is true that we
> "ignore the memory change".
>
> In some cases below, you use an "if/else", so it is also
> true that we "ignore the memory change".
>
> But in the "String ops" case, there is no "else", so we
> really do *not* ignore the memory change.
>
> Should we be consistant, and add an "else" to the string ops case?
>
> See further comments at end.
>
>> @@ -4039,7 +4059,7 @@ 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
>> "
>
> OK, this one is an "if/else", so you don't record the memory.
>
>> @@ -4458,13 +4478,8 @@ 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)
>
> But in this case, there is no "else", so you still record
> the memory even if i386_record_check_override returns true.
>
>
>
>> @@ -5086,7 +5101,7 @@ 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 "
>
> This is an "if/else" so you don't record the memory.
>
>> @@ -5138,7 +5153,7 @@ reswitch:
>> else
>> {
>> /* sidt */
>> - if (ir.override >= 0)
>> + if (i386_record_check_override (&ir))
>> {
>> if (record_debug)
>> printf_unfiltered (_("Process record ignores the memory
>> "
>
> 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.
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
{
[-- Attachment #2: prec-i386-override.txt --]
[-- Type: text/plain, Size: 6296 bytes --]
---
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
{
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA/prec] Make i386 handle segment register better
2009-08-30 3:21 ` Hui Zhu
@ 2009-09-05 2:42 ` Michael Snyder
2009-09-05 8:15 ` Mark Kettenis
0 siblings, 1 reply; 10+ messages in thread
From: Michael Snyder @ 2009-09-05 2:42 UTC (permalink / raw)
To: Hui Zhu; +Cc: gdb-patches ml
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
> {
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA/prec] Make i386 handle segment register better
2009-09-05 2:42 ` Michael Snyder
@ 2009-09-05 8:15 ` Mark Kettenis
2009-09-05 15:38 ` Hui Zhu
0 siblings, 1 reply; 10+ messages in thread
From: Mark Kettenis @ 2009-09-05 8:15 UTC (permalink / raw)
To: msnyder; +Cc: teawater, gdb-patches
> Date: Fri, 04 Sep 2009 19:41:21 -0700
> From: Michael Snyder <msnyder@vmware.com>
>
> 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?
The code is basically ok, but I'd like to ask Hui to avoid using
meaningless variable names like "tmp".
> > 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
> > {
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA/prec] Make i386 handle segment register better
2009-09-05 8:15 ` Mark Kettenis
@ 2009-09-05 15:38 ` Hui Zhu
2009-09-06 6:52 ` Hui Zhu
0 siblings, 1 reply; 10+ messages in thread
From: Hui Zhu @ 2009-09-05 15:38 UTC (permalink / raw)
To: Mark Kettenis; +Cc: msnyder, gdb-patches
On Sat, Sep 5, 2009 at 16:14, Mark Kettenis<mark.kettenis@xs4all.nl> wrote:
>> Date: Fri, 04 Sep 2009 19:41:21 -0700
>> From: Michael Snyder <msnyder@vmware.com>
>>
>> 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?
Thanks for you help, Michael.
>
> The code is basically ok, but I'd like to ask Hui to avoid using
> meaningless variable names like "tmp".
Thanks for remind me, Mark.
I checked in this patch with change the "tmp" to "orv".
Hui
>
>> > 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
>> > {
>>
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA/prec] Make i386 handle segment register better
2009-09-05 15:38 ` Hui Zhu
@ 2009-09-06 6:52 ` Hui Zhu
2009-09-06 15:06 ` Hui Zhu
0 siblings, 1 reply; 10+ messages in thread
From: Hui Zhu @ 2009-09-06 6:52 UTC (permalink / raw)
To: Mark Kettenis, Michael Snyder; +Cc: gdb-patches
Hi guys,
Sorry I didn't do more test for this patch on amd64 before I check it in.
But this patch really work not very good in amd64.
For example:
Process record: i386_process_record addr = 0x7ffff7b13cc9 signal = 0
Process record: add mem addr = 0xffffffffffffffa8 len = 4 to record list.
Process record: error reading memory at addr = 0xffffffffffffffa8 len = 4.
Process record: failed to record execution log.
Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff7b13cc9 in pause () from /lib/libc.so.6
(gdb) disassemble
Dump of assembler code for function pause:
0x00007ffff7b13c70 <pause+0>: cmpl $0x0,0x2c93d1(%rip) # 0x7ffff7ddd048
0x00007ffff7b13c77 <pause+7>: jne 0x7ffff7b13c89 <pause+25>
0x00007ffff7b13c79 <pause+9>: mov $0x22,%eax
0x00007ffff7b13c7e <pause+14>: syscall
0x00007ffff7b13c80 <pause+16>: cmp $0xfffffffffffff001,%rax
0x00007ffff7b13c86 <pause+22>: jae 0x7ffff7b13cbd <pause+77>
0x00007ffff7b13c88 <pause+24>: retq
0x00007ffff7b13c89 <pause+25>: sub $0x18,%rsp
0x00007ffff7b13c8d <pause+29>: callq 0x7ffff7b60770
0x00007ffff7b13c92 <pause+34>: mov %rax,(%rsp)
0x00007ffff7b13c96 <pause+38>: mov $0x22,%eax
0x00007ffff7b13c9b <pause+43>: syscall
0x00007ffff7b13c9d <pause+45>: mov (%rsp),%rdi
0x00007ffff7b13ca1 <pause+49>: mov %rax,0x8(%rsp)
0x00007ffff7b13ca6 <pause+54>: callq 0x7ffff7b60740
0x00007ffff7b13cab <pause+59>: mov 0x8(%rsp),%rax
0x00007ffff7b13cb0 <pause+64>: add $0x18,%rsp
0x00007ffff7b13cb4 <pause+68>: cmp $0xfffffffffffff001,%rax
0x00007ffff7b13cba <pause+74>: jae 0x7ffff7b13cbd <pause+77>
0x00007ffff7b13cbc <pause+76>: retq
0x00007ffff7b13cbd <pause+77>: mov 0x2c42cc(%rip),%rcx #
0x7ffff7dd7f90
0x00007ffff7b13cc4 <pause+84>: xor %edx,%edx
0x00007ffff7b13cc6 <pause+86>: sub %rax,%rdx
0x00007ffff7b13cc9 <pause+89>: mov %edx,%fs:(%rcx)
0x00007ffff7b13ccc <pause+92>: or $0xffffffffffffffff,%rax
0x00007ffff7b13cd0 <pause+96>: jmp 0x7ffff7b13cbc <pause+76>
End of assembler dump.
(gdb) info reg
rax 0xfffffffffffffffc -4
rbx 0x4007c0 4196288
rcx 0xffffffffffffffa8 -88
rdx 0x4 4
rsi 0x0 0
rdi 0x1 1
rbp 0x7fffffffe2e0 0x7fffffffe2e0
rsp 0x7fffffffe2b8 0x7fffffffe2b8
r8 0x7fffffffe210 140737488347664
r9 0x7fffffffe170 140737488347504
r10 0x7fffffffe040 140737488347200
r11 0x346 838
r12 0x400640 4195904
r13 0x7fffffffe3b0 140737488348080
r14 0x0 0
r15 0x0 0
rip 0x7ffff7b13cc9 0x7ffff7b13cc9 <pause+89>
eflags 0x313 [ CF AF TF IF ]
cs 0x33 51
ss 0x2b 43
ds 0x0 0
es 0x0 0
fs 0x0 0
gs 0x0 0
fctrl 0x37f 895
fstat 0x0 0
ftag 0xffff 65535
fiseg 0x0 0
fioff 0x0 0
foseg 0x0 0
fooff 0x0 0
fop 0x0 0
mxcsr 0x1f80 [ IM DM ZM OM UM PM ]
(gdb) record stop
Delete recorded log and stop recording?(y or n) y
Process record: record_close
(gdb) set disassemble-next-line on
(gdb) si
0x00007ffff7b13ccc in pause () from /lib/libc.so.6
0x00007ffff7b13ccc <pause+92>: 48 83 c8 ff or $0xffffffffffffffff,%rax
(gdb) info registers
rax 0xfffffffffffffffc -4
rbx 0x4007c0 4196288
rcx 0xffffffffffffffa8 -88
rdx 0x4 4
rsi 0x0 0
rdi 0x1 1
rbp 0x7fffffffe2e0 0x7fffffffe2e0
rsp 0x7fffffffe2b8 0x7fffffffe2b8
r8 0x7fffffffe210 140737488347664
r9 0x7fffffffe170 140737488347504
r10 0x7fffffffe040 140737488347200
r11 0x346 838
r12 0x400640 4195904
r13 0x7fffffffe3b0 140737488348080
r14 0x0 0
r15 0x0 0
rip 0x7ffff7b13ccc 0x7ffff7b13ccc <pause+92>
eflags 0x313 [ CF AF TF IF ]
cs 0x33 51
ss 0x2b 43
ds 0x0 0
es 0x0 0
fs 0x0 0
gs 0x0 0
fctrl 0x37f 895
fstat 0x0 0
ftag 0xffff 65535
fiseg 0x0 0
fioff 0x0 0
foseg 0x0 0
fooff 0x0 0
fop 0x0 0
mxcsr 0x1f80 [ IM DM ZM OM UM PM ]
(gdb) x 0x7ffff7dd7f90
0x7ffff7dd7f90: 0xffffffa8
(gdb) x 0xffffffa8
0xffffffa8: Cannot access memory at address 0xffffffa8
(gdb) x 0xffffffffffffffa8
0xffffffffffffffa8: Cannot access memory at address 0xffffffffffffffa8
(gdb)
The fs is same with gs, but "mov %edx,%fs:(%rcx)" is not same with
"mov %edx,(%rcx)".
I think remove this patch from gdb-cvs-head before 7.0 branch and
make the segment reg clear is better.
What do you think about it?
Thanks,
Hui
On Sat, Sep 5, 2009 at 23:37, Hui Zhu<teawater@gmail.com> wrote:
> On Sat, Sep 5, 2009 at 16:14, Mark Kettenis<mark.kettenis@xs4all.nl> wrote:
>>> Date: Fri, 04 Sep 2009 19:41:21 -0700
>>> From: Michael Snyder <msnyder@vmware.com>
>>>
>>> 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?
>
> Thanks for you help, Michael.
>
>>
>> The code is basically ok, but I'd like to ask Hui to avoid using
>> meaningless variable names like "tmp".
>
> Thanks for remind me, Mark.
>
> I checked in this patch with change the "tmp" to "orv".
>
>
> Hui
>
>
>>
>>> > 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
>>> > {
>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA/prec] Make i386 handle segment register better
2009-09-06 6:52 ` Hui Zhu
@ 2009-09-06 15:06 ` Hui Zhu
2009-09-07 0:07 ` Michael Snyder
0 siblings, 1 reply; 10+ messages in thread
From: Hui Zhu @ 2009-09-06 15:06 UTC (permalink / raw)
To: Mark Kettenis, Michael Snyder; +Cc: gdb-patches
On Sun, Sep 6, 2009 at 14:52, Hui Zhu<teawater@gmail.com> wrote:
> Hi guys,
>
> Sorry I didn't do more test for this patch on amd64 before I check it in.
>
> But this patch really work not very good in amd64.
>
> For example:
>
> Process record: i386_process_record addr = 0x7ffff7b13cc9 signal = 0
> Process record: add mem addr = 0xffffffffffffffa8 len = 4 to record list.
> Process record: error reading memory at addr = 0xffffffffffffffa8 len = 4.
> Process record: failed to record execution log.
>
> Program received signal SIGTRAP, Trace/breakpoint trap.
> 0x00007ffff7b13cc9 in pause () from /lib/libc.so.6
> (gdb) disassemble
> Dump of assembler code for function pause:
> 0x00007ffff7b13c70 <pause+0>: cmpl $0x0,0x2c93d1(%rip) # 0x7ffff7ddd048
> 0x00007ffff7b13c77 <pause+7>: jne 0x7ffff7b13c89 <pause+25>
> 0x00007ffff7b13c79 <pause+9>: mov $0x22,%eax
> 0x00007ffff7b13c7e <pause+14>: syscall
> 0x00007ffff7b13c80 <pause+16>: cmp $0xfffffffffffff001,%rax
> 0x00007ffff7b13c86 <pause+22>: jae 0x7ffff7b13cbd <pause+77>
> 0x00007ffff7b13c88 <pause+24>: retq
> 0x00007ffff7b13c89 <pause+25>: sub $0x18,%rsp
> 0x00007ffff7b13c8d <pause+29>: callq 0x7ffff7b60770
> 0x00007ffff7b13c92 <pause+34>: mov %rax,(%rsp)
> 0x00007ffff7b13c96 <pause+38>: mov $0x22,%eax
> 0x00007ffff7b13c9b <pause+43>: syscall
> 0x00007ffff7b13c9d <pause+45>: mov (%rsp),%rdi
> 0x00007ffff7b13ca1 <pause+49>: mov %rax,0x8(%rsp)
> 0x00007ffff7b13ca6 <pause+54>: callq 0x7ffff7b60740
> 0x00007ffff7b13cab <pause+59>: mov 0x8(%rsp),%rax
> 0x00007ffff7b13cb0 <pause+64>: add $0x18,%rsp
> 0x00007ffff7b13cb4 <pause+68>: cmp $0xfffffffffffff001,%rax
> 0x00007ffff7b13cba <pause+74>: jae 0x7ffff7b13cbd <pause+77>
> 0x00007ffff7b13cbc <pause+76>: retq
> 0x00007ffff7b13cbd <pause+77>: mov 0x2c42cc(%rip),%rcx #
> 0x7ffff7dd7f90
> 0x00007ffff7b13cc4 <pause+84>: xor %edx,%edx
> 0x00007ffff7b13cc6 <pause+86>: sub %rax,%rdx
> 0x00007ffff7b13cc9 <pause+89>: mov %edx,%fs:(%rcx)
> 0x00007ffff7b13ccc <pause+92>: or $0xffffffffffffffff,%rax
> 0x00007ffff7b13cd0 <pause+96>: jmp 0x7ffff7b13cbc <pause+76>
> End of assembler dump.
> (gdb) info reg
> rax 0xfffffffffffffffc -4
> rbx 0x4007c0 4196288
> rcx 0xffffffffffffffa8 -88
> rdx 0x4 4
> rsi 0x0 0
> rdi 0x1 1
> rbp 0x7fffffffe2e0 0x7fffffffe2e0
> rsp 0x7fffffffe2b8 0x7fffffffe2b8
> r8 0x7fffffffe210 140737488347664
> r9 0x7fffffffe170 140737488347504
> r10 0x7fffffffe040 140737488347200
> r11 0x346 838
> r12 0x400640 4195904
> r13 0x7fffffffe3b0 140737488348080
> r14 0x0 0
> r15 0x0 0
> rip 0x7ffff7b13cc9 0x7ffff7b13cc9 <pause+89>
> eflags 0x313 [ CF AF TF IF ]
> cs 0x33 51
> ss 0x2b 43
> ds 0x0 0
> es 0x0 0
> fs 0x0 0
> gs 0x0 0
> fctrl 0x37f 895
> fstat 0x0 0
> ftag 0xffff 65535
> fiseg 0x0 0
> fioff 0x0 0
> foseg 0x0 0
> fooff 0x0 0
> fop 0x0 0
> mxcsr 0x1f80 [ IM DM ZM OM UM PM ]
> (gdb) record stop
> Delete recorded log and stop recording?(y or n) y
> Process record: record_close
> (gdb) set disassemble-next-line on
> (gdb) si
> 0x00007ffff7b13ccc in pause () from /lib/libc.so.6
> 0x00007ffff7b13ccc <pause+92>: 48 83 c8 ff or $0xffffffffffffffff,%rax
> (gdb) info registers
> rax 0xfffffffffffffffc -4
> rbx 0x4007c0 4196288
> rcx 0xffffffffffffffa8 -88
> rdx 0x4 4
> rsi 0x0 0
> rdi 0x1 1
> rbp 0x7fffffffe2e0 0x7fffffffe2e0
> rsp 0x7fffffffe2b8 0x7fffffffe2b8
> r8 0x7fffffffe210 140737488347664
> r9 0x7fffffffe170 140737488347504
> r10 0x7fffffffe040 140737488347200
> r11 0x346 838
> r12 0x400640 4195904
> r13 0x7fffffffe3b0 140737488348080
> r14 0x0 0
> r15 0x0 0
> rip 0x7ffff7b13ccc 0x7ffff7b13ccc <pause+92>
> eflags 0x313 [ CF AF TF IF ]
> cs 0x33 51
> ss 0x2b 43
> ds 0x0 0
> es 0x0 0
> fs 0x0 0
> gs 0x0 0
> fctrl 0x37f 895
> fstat 0x0 0
> ftag 0xffff 65535
> fiseg 0x0 0
> fioff 0x0 0
> foseg 0x0 0
> fooff 0x0 0
> fop 0x0 0
> mxcsr 0x1f80 [ IM DM ZM OM UM PM ]
> (gdb) x 0x7ffff7dd7f90
> 0x7ffff7dd7f90: 0xffffffa8
> (gdb) x 0xffffffa8
> 0xffffffa8: Cannot access memory at address 0xffffffa8
> (gdb) x 0xffffffffffffffa8
> 0xffffffffffffffa8: Cannot access memory at address 0xffffffffffffffa8
> (gdb)
>
>
> The fs is same with gs, but "mov %edx,%fs:(%rcx)" is not same with
> "mov %edx,(%rcx)".
>
> I think remove this patch from gdb-cvs-head before 7.0 branch and
> make the segment reg clear is better.
>
> What do you think about it?
>
> Thanks,
> Hui
>
I make a patch for it. Please help me review it.
Thanks,
Hui
2009-09-06 Hui Zhu <teawater@gmail.com>
* i386-tdep.c (i386_record_check_override): Deleted.
(i386_record_lea_modrm): Ditto.
(i386_process_record): Ditto.
---
i386-tdep.c | 37 +++++++++++--------------------------
1 file changed, 11 insertions(+), 26 deletions(-)
--- a/i386-tdep.c
+++ b/i386-tdep.c
@@ -3148,26 +3148,6 @@ 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 orv, ds;
-
- regcache_raw_read_unsigned (irp->regcache,
- irp->regmap[irp->override],
- &orv);
- regcache_raw_read_unsigned (irp->regcache,
- irp->regmap[X86_RECORD_DS_REGNUM],
- &ds);
- if (orv != 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. */
@@ -3178,7 +3158,7 @@ i386_record_lea_modrm (struct i386_recor
struct gdbarch *gdbarch = irp->gdbarch;
uint64_t addr;
- if (i386_record_check_override (irp))
+ if (irp->override >= 0)
{
warning (_("Process record ignores the memory change "
"of instruction at address %s because it "
@@ -4060,7 +4040,7 @@ reswitch:
/* mov EAX */
case 0xa2:
case 0xa3:
- if (i386_record_check_override (&ir))
+ if (ir.override >= 0)
{
warning (_("Process record ignores the memory change "
"of instruction at address 0x%s because "
@@ -4478,8 +4458,13 @@ reswitch:
ir.regmap[X86_RECORD_REDI_REGNUM],
&tmpulongest);
- ir.override = X86_RECORD_ES_REGNUM;
- if (ir.aflag && i386_record_check_override (&ir))
+ 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))
{
/* addr += ((uint32_t) read_register (I386_ES_REGNUM)) << 4; */
warning (_("Process record ignores the memory "
@@ -5103,7 +5088,7 @@ reswitch:
opcode = opcode << 8 | ir.modrm;
goto no_support;
}
- if (i386_record_check_override (&ir))
+ if (ir.override >= 0)
{
warning (_("Process record ignores the memory "
"change of instruction at "
@@ -5154,7 +5139,7 @@ reswitch:
else
{
/* sidt */
- if (i386_record_check_override (&ir))
+ if (ir.override >= 0)
{
warning (_("Process record ignores the memory "
"change of instruction at "
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA/prec] Make i386 handle segment register better
2009-09-06 15:06 ` Hui Zhu
@ 2009-09-07 0:07 ` Michael Snyder
2009-09-07 11:17 ` Hui Zhu
0 siblings, 1 reply; 10+ messages in thread
From: Michael Snyder @ 2009-09-07 0:07 UTC (permalink / raw)
To: Hui Zhu; +Cc: Mark Kettenis, gdb-patches
Hui Zhu wrote:
> On Sun, Sep 6, 2009 at 14:52, Hui Zhu<teawater@gmail.com> wrote:
>> Hi guys,
>>
>> Sorry I didn't do more test for this patch on amd64 before I check it in.
>>
>> But this patch really work not very good in amd64.
>>
[...]
>> I think remove this patch from gdb-cvs-head before 7.0 branch and
>> make the segment reg clear is better.
>>
>> What do you think about it?
>>
>> Thanks,
>> Hui
>>
>
> I make a patch for it. Please help me review it.
>
> Thanks,
> Hui
>
> 2009-09-06 Hui Zhu <teawater@gmail.com>
>
> * i386-tdep.c (i386_record_check_override): Deleted.
> (i386_record_lea_modrm): Ditto.
> (i386_process_record): Ditto.
(i386_process_record): Revert the use of deleted
function 'i386_record_check_override'.
(i386_record_lea_modrm): Ditto.
Looks to me like you've preserved what you could
of the change, and removed the minimum. Good.
This looks ok to commit.
>
> ---
> i386-tdep.c | 37 +++++++++++--------------------------
> 1 file changed, 11 insertions(+), 26 deletions(-)
>
> --- a/i386-tdep.c
> +++ b/i386-tdep.c
> @@ -3148,26 +3148,6 @@ 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 orv, ds;
> -
> - regcache_raw_read_unsigned (irp->regcache,
> - irp->regmap[irp->override],
> - &orv);
> - regcache_raw_read_unsigned (irp->regcache,
> - irp->regmap[X86_RECORD_DS_REGNUM],
> - &ds);
> - if (orv != 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. */
> @@ -3178,7 +3158,7 @@ i386_record_lea_modrm (struct i386_recor
> struct gdbarch *gdbarch = irp->gdbarch;
> uint64_t addr;
>
> - if (i386_record_check_override (irp))
> + if (irp->override >= 0)
> {
> warning (_("Process record ignores the memory change "
> "of instruction at address %s because it "
> @@ -4060,7 +4040,7 @@ reswitch:
> /* mov EAX */
> case 0xa2:
> case 0xa3:
> - if (i386_record_check_override (&ir))
> + if (ir.override >= 0)
> {
> warning (_("Process record ignores the memory change "
> "of instruction at address 0x%s because "
> @@ -4478,8 +4458,13 @@ reswitch:
> ir.regmap[X86_RECORD_REDI_REGNUM],
> &tmpulongest);
>
> - ir.override = X86_RECORD_ES_REGNUM;
> - if (ir.aflag && i386_record_check_override (&ir))
> + 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))
> {
> /* addr += ((uint32_t) read_register (I386_ES_REGNUM)) << 4; */
> warning (_("Process record ignores the memory "
> @@ -5103,7 +5088,7 @@ reswitch:
> opcode = opcode << 8 | ir.modrm;
> goto no_support;
> }
> - if (i386_record_check_override (&ir))
> + if (ir.override >= 0)
> {
> warning (_("Process record ignores the memory "
> "change of instruction at "
> @@ -5154,7 +5139,7 @@ reswitch:
> else
> {
> /* sidt */
> - if (i386_record_check_override (&ir))
> + if (ir.override >= 0)
> {
> warning (_("Process record ignores the memory "
> "change of instruction at "
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFA/prec] Make i386 handle segment register better
2009-09-07 0:07 ` Michael Snyder
@ 2009-09-07 11:17 ` Hui Zhu
0 siblings, 0 replies; 10+ messages in thread
From: Hui Zhu @ 2009-09-07 11:17 UTC (permalink / raw)
To: Michael Snyder; +Cc: Mark Kettenis, gdb-patches
On Mon, Sep 7, 2009 at 08:06, Michael Snyder<msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>>
>> On Sun, Sep 6, 2009 at 14:52, Hui Zhu<teawater@gmail.com> wrote:
>>>
>>> Hi guys,
>>>
>>> Sorry I didn't do more test for this patch on amd64 before I check it in.
>>>
>>> But this patch really work not very good in amd64.
>>>
> [...]
>>>
>>> I think remove this patch from gdb-cvs-head before 7.0 branch and
>>> make the segment reg clear is better.
>>>
>>> What do you think about it?
>>>
>>> Thanks,
>>> Hui
>>>
>>
>> I make a patch for it. Please help me review it.
>>
>> Thanks,
>> Hui
>>
>> 2009-09-06 Hui Zhu <teawater@gmail.com>
>>
>> * i386-tdep.c (i386_record_check_override): Deleted.
>> (i386_record_lea_modrm): Ditto.
>> (i386_process_record): Ditto.
>
> (i386_process_record): Revert the use of deleted
> function 'i386_record_check_override'.
> (i386_record_lea_modrm): Ditto.
>
> Looks to me like you've preserved what you could
> of the change, and removed the minimum. Good.
>
> This looks ok to commit.
>
>
Thanks. Checked in.
Hui
>
>>
>> ---
>> i386-tdep.c | 37 +++++++++++--------------------------
>> 1 file changed, 11 insertions(+), 26 deletions(-)
>>
>> --- a/i386-tdep.c
>> +++ b/i386-tdep.c
>> @@ -3148,26 +3148,6 @@ 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 orv, ds;
>> -
>> - regcache_raw_read_unsigned (irp->regcache,
>> - irp->regmap[irp->override],
>> - &orv);
>> - regcache_raw_read_unsigned (irp->regcache,
>> - irp->regmap[X86_RECORD_DS_REGNUM],
>> - &ds);
>> - if (orv != 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. */
>> @@ -3178,7 +3158,7 @@ i386_record_lea_modrm (struct i386_recor
>> struct gdbarch *gdbarch = irp->gdbarch;
>> uint64_t addr;
>>
>> - if (i386_record_check_override (irp))
>> + if (irp->override >= 0)
>> {
>> warning (_("Process record ignores the memory change "
>> "of instruction at address %s because it "
>> @@ -4060,7 +4040,7 @@ reswitch:
>> /* mov EAX */
>> case 0xa2:
>> case 0xa3:
>> - if (i386_record_check_override (&ir))
>> + if (ir.override >= 0)
>> {
>> warning (_("Process record ignores the memory change "
>> "of instruction at address 0x%s because "
>> @@ -4478,8 +4458,13 @@ reswitch:
>> ir.regmap[X86_RECORD_REDI_REGNUM],
>> &tmpulongest);
>>
>> - ir.override = X86_RECORD_ES_REGNUM;
>> - if (ir.aflag && i386_record_check_override (&ir))
>> + 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))
>> {
>> /* addr += ((uint32_t) read_register (I386_ES_REGNUM)) << 4;
>> */
>> warning (_("Process record ignores the memory "
>> @@ -5103,7 +5088,7 @@ reswitch:
>> opcode = opcode << 8 | ir.modrm;
>> goto no_support;
>> }
>> - if (i386_record_check_override (&ir))
>> + if (ir.override >= 0)
>> {
>> warning (_("Process record ignores the memory "
>> "change of instruction at "
>> @@ -5154,7 +5139,7 @@ reswitch:
>> else
>> {
>> /* sidt */
>> - if (i386_record_check_override (&ir))
>> + if (ir.override >= 0)
>> {
>> warning (_("Process record ignores the memory "
>> "change of instruction at "
>>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-09-07 11:17 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-29 16:12 [RFA/prec] Make i386 handle segment register better Hui Zhu
2009-08-29 21:34 ` Michael Snyder
2009-08-30 3:21 ` Hui Zhu
2009-09-05 2:42 ` Michael Snyder
2009-09-05 8:15 ` Mark Kettenis
2009-09-05 15:38 ` Hui Zhu
2009-09-06 6:52 ` Hui Zhu
2009-09-06 15:06 ` Hui Zhu
2009-09-07 0:07 ` Michael Snyder
2009-09-07 11:17 ` Hui Zhu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox