[RFA/prec] Make i386 handle segment register better
Hui Zhu
teawater@gmail.com
Sun Sep 6 06:52:00 GMT 2009
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
>>> > {
>>>
>>>
>>
>
More information about the Gdb-patches
mailing list