Bug in i386_process_record

Bug in i386_process_record

[Michael Snyder]

Hi Hui,

This line in i386-tdep.c causes 4 failures in machinestate.exp.

diff -u -p -r1.283 i386-tdep.c
--- i386-tdep.c 10 Aug 2009 03:02:39 -0000      1.283
+++ i386-tdep.c 16 Aug 2009 01:07:48 -0000
@@ -3283,7 +3283,7 @@ i386_process_record (struct gdbarch *gdb
          case 0x40:
          case 0x41:
          case 0x42:
-        case 0x43:
+         //        case 0x43:
          case 0x44:
          case 0x45:
          case 0x46:

0x43 is "inc %ebx", and this line causes it to be treated as a prefix,
consuming the instruction without recording the register change.

I don't want to change it myself, because I'm not sure what other
side effects the change might have.  Could you fix it please?  ;-)

Thanks,
Michael

Re: Bug in i386_process_record

[Hui Zhu]

	case 0x67:
	  prefixes |= PREFIX_ADDR;
	  break;
        case 0x40:
        case 0x41:
        case 0x42:
        case 0x43:
        case 0x44:
        case 0x45:
        case 0x46:
        case 0x47:

      /* inv */
    case 0x40:
    case 0x41:
    case 0x42:
    case 0x43:
    case 0x44:
    case 0x45:
    case 0x46:
    case 0x47:

Oops, I must make something wrong.  I need check the spec of amd64 clear.

Thanks,
Hui

On Sun, Aug 16, 2009 at 09:08, Michael Snyder<msnyder@vmware.com> wrote:
> Hi Hui,
>
> This line in i386-tdep.c causes 4 failures in machinestate.exp.
>
> diff -u -p -r1.283 i386-tdep.c
> --- i386-tdep.c 10 Aug 2009 03:02:39 -0000      1.283
> +++ i386-tdep.c 16 Aug 2009 01:07:48 -0000
> @@ -3283,7 +3283,7 @@ i386_process_record (struct gdbarch *gdb
>         case 0x40:
>         case 0x41:
>         case 0x42:
> -        case 0x43:
> +         //        case 0x43:
>         case 0x44:
>         case 0x45:
>         case 0x46:
>
> 0x43 is "inc %ebx", and this line causes it to be treated as a prefix,
> consuming the instruction without recording the register change.
>
> I don't want to change it myself, because I'm not sure what other
> side effects the change might have.  Could you fix it please?  ;-)
>
> Thanks,
> Michael
>
>

Re: Bug in i386_process_record

[Hui Zhu]

Hi Michael,

I think this is not a bug.

In "Intel® 64 and IA-32 Architectures Software Developer’s Manual
Volume 2A: Instruction Set Reference, A-M" INC—Increment by 1, it
said:"In 64-bit mode, INC r16 and INC r32 are not encodable (because
opcodes 40H
through 47H are REX prefixes)."
And disas of machinestate is clear:
(gdb) disas /m register_state
Dump of assembler code for function register_state:
29	{
0x0000000000400488 <register_state+0>:	push   %rbp
0x0000000000400489 <register_state+1>:	mov    %rsp,%rbp
0x000000000040048c <register_state+4>:	push   %rbx
0x000000000040048d <register_state+5>:	sub    $0x8,%rsp

30	  register int a = 0;
0x0000000000400491 <register_state+9>:	mov    $0x0,%ebx

31	
32	  hide (a);	/* External function to defeat optimization.  */
0x0000000000400496 <register_state+14>:	mov    %ebx,%edi
0x0000000000400498 <register_state+16>:	callq  0x400598 <hide>

33	  a++;		/* register_state: set breakpoint here */
0x000000000040049d <register_state+21>:	add    $0x1,%ebx

34	  hide (a);	/* register post-change */
0x00000000004004a0 <register_state+24>:	mov    %ebx,%edi
0x00000000004004a2 <register_state+26>:	callq  0x400598 <hide>

35	}
0x00000000004004a7 <register_state+31>:	add    $0x8,%rsp
0x00000000004004ab <register_state+35>:	pop    %rbx
0x00000000004004ac <register_state+36>:	leaveq
0x00000000004004ad <register_state+37>:	retq

End of assembler dump.

In amd64, 0x40-0x47 will not be use to inv.


Thanks,
Hui


On Mon, Aug 17, 2009 at 00:12, Hui Zhu<teawater@gmail.com> wrote:
>        case 0x67:
>          prefixes |= PREFIX_ADDR;
>          break;
>        case 0x40:
>        case 0x41:
>        case 0x42:
>        case 0x43:
>        case 0x44:
>        case 0x45:
>        case 0x46:
>        case 0x47:
>
>      /* inv */
>    case 0x40:
>    case 0x41:
>    case 0x42:
>    case 0x43:
>    case 0x44:
>    case 0x45:
>    case 0x46:
>    case 0x47:
>
> Oops, I must make something wrong.  I need check the spec of amd64 clear.
>
> Thanks,
> Hui
>
> On Sun, Aug 16, 2009 at 09:08, Michael Snyder<msnyder@vmware.com> wrote:
>> Hi Hui,
>>
>> This line in i386-tdep.c causes 4 failures in machinestate.exp.
>>
>> diff -u -p -r1.283 i386-tdep.c
>> --- i386-tdep.c 10 Aug 2009 03:02:39 -0000      1.283
>> +++ i386-tdep.c 16 Aug 2009 01:07:48 -0000
>> @@ -3283,7 +3283,7 @@ i386_process_record (struct gdbarch *gdb
>>         case 0x40:
>>         case 0x41:
>>         case 0x42:
>> -        case 0x43:
>> +         //        case 0x43:
>>         case 0x44:
>>         case 0x45:
>>         case 0x46:
>>
>> 0x43 is "inc %ebx", and this line causes it to be treated as a prefix,
>> consuming the instruction without recording the register change.
>>
>> I don't want to change it myself, because I'm not sure what other
>> side effects the change might have.  Could you fix it please?  ;-)
>>
>> Thanks,
>> Michael
>>
>>
>

Re: Bug in i386_process_record

[Hui Zhu]

I try to find the reason of 4 failures in machinestate.exp.

(gdb) PASS: gdb.reverse/machinestate.exp: forward to 33
print a

$16 = 0

(gdb) PASS: gdb.reverse/machinestate.exp: register var forward-breakpoint
reverse-step

32	  hide (a);	/* External function to defeat optimization.  */

(gdb) step

hide (x=0) at ../src/gdb/testsuite/gdb.reverse/ms1.c:25

25	}

(gdb) FAIL: gdb.reverse/machinestate.exp: step

I think the fail is because rs didn't step into hide.


Thanks,
Hui



On Mon, Aug 17, 2009 at 11:34, Hui Zhu<teawater@gmail.com> wrote:
> Hi Michael,
>
> I think this is not a bug.
>
> In "Intel® 64 and IA-32 Architectures Software Developer’s Manual
> Volume 2A: Instruction Set Reference, A-M" INC—Increment by 1, it
> said:"In 64-bit mode, INC r16 and INC r32 are not encodable (because
> opcodes 40H
> through 47H are REX prefixes)."
> And disas of machinestate is clear:
> (gdb) disas /m register_state
> Dump of assembler code for function register_state:
> 29      {
> 0x0000000000400488 <register_state+0>:  push   %rbp
> 0x0000000000400489 <register_state+1>:  mov    %rsp,%rbp
> 0x000000000040048c <register_state+4>:  push   %rbx
> 0x000000000040048d <register_state+5>:  sub    $0x8,%rsp
>
> 30        register int a = 0;
> 0x0000000000400491 <register_state+9>:  mov    $0x0,%ebx
>
> 31
> 32        hide (a);     /* External function to defeat optimization.  */
> 0x0000000000400496 <register_state+14>: mov    %ebx,%edi
> 0x0000000000400498 <register_state+16>: callq  0x400598 <hide>
>
> 33        a++;          /* register_state: set breakpoint here */
> 0x000000000040049d <register_state+21>: add    $0x1,%ebx
>
> 34        hide (a);     /* register post-change */
> 0x00000000004004a0 <register_state+24>: mov    %ebx,%edi
> 0x00000000004004a2 <register_state+26>: callq  0x400598 <hide>
>
> 35      }
> 0x00000000004004a7 <register_state+31>: add    $0x8,%rsp
> 0x00000000004004ab <register_state+35>: pop    %rbx
> 0x00000000004004ac <register_state+36>: leaveq
> 0x00000000004004ad <register_state+37>: retq
>
> End of assembler dump.
>
> In amd64, 0x40-0x47 will not be use to inv.
>
>
> Thanks,
> Hui
>
>
> On Mon, Aug 17, 2009 at 00:12, Hui Zhu<teawater@gmail.com> wrote:
>>        case 0x67:
>>          prefixes |= PREFIX_ADDR;
>>          break;
>>        case 0x40:
>>        case 0x41:
>>        case 0x42:
>>        case 0x43:
>>        case 0x44:
>>        case 0x45:
>>        case 0x46:
>>        case 0x47:
>>
>>      /* inv */
>>    case 0x40:
>>    case 0x41:
>>    case 0x42:
>>    case 0x43:
>>    case 0x44:
>>    case 0x45:
>>    case 0x46:
>>    case 0x47:
>>
>> Oops, I must make something wrong.  I need check the spec of amd64 clear.
>>
>> Thanks,
>> Hui
>>
>> On Sun, Aug 16, 2009 at 09:08, Michael Snyder<msnyder@vmware.com> wrote:
>>> Hi Hui,
>>>
>>> This line in i386-tdep.c causes 4 failures in machinestate.exp.
>>>
>>> diff -u -p -r1.283 i386-tdep.c
>>> --- i386-tdep.c 10 Aug 2009 03:02:39 -0000      1.283
>>> +++ i386-tdep.c 16 Aug 2009 01:07:48 -0000
>>> @@ -3283,7 +3283,7 @@ i386_process_record (struct gdbarch *gdb
>>>         case 0x40:
>>>         case 0x41:
>>>         case 0x42:
>>> -        case 0x43:
>>> +         //        case 0x43:
>>>         case 0x44:
>>>         case 0x45:
>>>         case 0x46:
>>>
>>> 0x43 is "inc %ebx", and this line causes it to be treated as a prefix,
>>> consuming the instruction without recording the register change.
>>>
>>> I don't want to change it myself, because I'm not sure what other
>>> side effects the change might have.  Could you fix it please?  ;-)
>>>
>>> Thanks,
>>> Michael
>>>
>>>
>>
>

Re: Bug in i386_process_record

[Michael Snyder]

Hui Zhu wrote:
> Hi Michael,
> 
> I think this is not a bug.

Have you tried running the machinestate.exp test on intel 32?
It fails!  Take out the amd64 patch, and it passes!  Put the
amd64 patch back in, but comment out "case 0x43", it passes!

Heh -- sorry about the exclamation points...   ;-)

See below.

> In "Intel® 64 and IA-32 Architectures Software Developer’s Manual
> Volume 2A: Instruction Set Reference, A-M" INC—Increment by 1, it
> said:"In 64-bit mode, INC r16 and INC r32 are not encodable (because
> opcodes 40H
> through 47H are REX prefixes)."
> And disas of machinestate is clear:
> (gdb) disas /m register_state
> Dump of assembler code for function register_state:
> 29	{
> 0x0000000000400488 <register_state+0>:	push   %rbp
> 0x0000000000400489 <register_state+1>:	mov    %rsp,%rbp
> 0x000000000040048c <register_state+4>:	push   %rbx
> 0x000000000040048d <register_state+5>:	sub    $0x8,%rsp
> 
> 30	  register int a = 0;
> 0x0000000000400491 <register_state+9>:	mov    $0x0,%ebx
> 
> 31	
> 32	  hide (a);	/* External function to defeat optimization.  */
> 0x0000000000400496 <register_state+14>:	mov    %ebx,%edi
> 0x0000000000400498 <register_state+16>:	callq  0x400598 <hide>
> 
> 33	  a++;		/* register_state: set breakpoint here */
> 0x000000000040049d <register_state+21>:	add    $0x1,%ebx
> 
> 34	  hide (a);	/* register post-change */
> 0x00000000004004a0 <register_state+24>:	mov    %ebx,%edi
> 0x00000000004004a2 <register_state+26>:	callq  0x400598 <hide>
> 
> 35	}
> 0x00000000004004a7 <register_state+31>:	add    $0x8,%rsp
> 0x00000000004004ab <register_state+35>:	pop    %rbx
> 0x00000000004004ac <register_state+36>:	leaveq
> 0x00000000004004ad <register_state+37>:	retq
> 
> End of assembler dump.
> 
> In amd64, 0x40-0x47 will not be use to inv.

Yes, but in intel32, it is used.  By the way, I used your
clever "disas /r" to discover it:


33        a++;          /* register_state: set breakpoint here */
0x08048358 <register_state+24>:  43     inc    %ebx



> On Mon, Aug 17, 2009 at 00:12, Hui Zhu<teawater@gmail.com> wrote:
>>        case 0x67:
>>          prefixes |= PREFIX_ADDR;
>>          break;
>>        case 0x40:
>>        case 0x41:
>>        case 0x42:
>>        case 0x43:
>>        case 0x44:
>>        case 0x45:
>>        case 0x46:
>>        case 0x47:
>>
>>      /* inv */
>>    case 0x40:
>>    case 0x41:
>>    case 0x42:
>>    case 0x43:
>>    case 0x44:
>>    case 0x45:
>>    case 0x46:
>>    case 0x47:
>>
>> Oops, I must make something wrong.  I need check the spec of amd64 clear.
>>
>> Thanks,
>> Hui
>>
>> On Sun, Aug 16, 2009 at 09:08, Michael Snyder<msnyder@vmware.com> wrote:
>>> Hi Hui,
>>>
>>> This line in i386-tdep.c causes 4 failures in machinestate.exp.
>>>
>>> diff -u -p -r1.283 i386-tdep.c
>>> --- i386-tdep.c 10 Aug 2009 03:02:39 -0000      1.283
>>> +++ i386-tdep.c 16 Aug 2009 01:07:48 -0000
>>> @@ -3283,7 +3283,7 @@ i386_process_record (struct gdbarch *gdb
>>>         case 0x40:
>>>         case 0x41:
>>>         case 0x42:
>>> -        case 0x43:
>>> +         //        case 0x43:
>>>         case 0x44:
>>>         case 0x45:
>>>         case 0x46:
>>>
>>> 0x43 is "inc %ebx", and this line causes it to be treated as a prefix,
>>> consuming the instruction without recording the register change.
>>>
>>> I don't want to change it myself, because I'm not sure what other
>>> side effects the change might have.  Could you fix it please?  ;-)
>>>
>>> Thanks,
>>> Michael
>>>
>>>

Re: Bug in i386_process_record

[Michael Snyder]

Hui Zhu wrote:

> (gdb) FAIL: gdb.reverse/machinestate.exp: step
> 
> I think the fail is because rs didn't step into hide.

32bit or 64?
If 64, did you try it after applying my two recent patches?

Anyway, that is not the only reason it fails.
It fails for me on 32 bit intel.

Re: Bug in i386_process_record

[Hui Zhu]

On Tue, Aug 18, 2009 at 02:24, Michael Snyder<msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>
>> (gdb) FAIL: gdb.reverse/machinestate.exp: step
>>
>> I think the fail is because rs didn't step into hide.
>
> 32bit or 64?
> If 64, did you try it after applying my two recent patches?
>
> Anyway, that is not the only reason it fails.
> It fails for me on 32 bit intel.

I try amd64 with your new patch in last night, everything is OK.

For x86, I try it in prev some day, everything is OK too.

This pc don't have the gdb-cvs-head and I cannot connect gdb cvs now.
I will try it later.

Thanks,
Hui

Re: Bug in i386_process_record

[Hui Zhu]

I tried.  Everything is OK.

Thanks,
Hui

On Tue, Aug 18, 2009 at 07:01, Hui Zhu<teawater@gmail.com> wrote:
> On Tue, Aug 18, 2009 at 02:24, Michael Snyder<msnyder@vmware.com> wrote:
>> Hui Zhu wrote:
>>
>>> (gdb) FAIL: gdb.reverse/machinestate.exp: step
>>>
>>> I think the fail is because rs didn't step into hide.
>>
>> 32bit or 64?
>> If 64, did you try it after applying my two recent patches?
>>
>> Anyway, that is not the only reason it fails.
>> It fails for me on 32 bit intel.
>
> I try amd64 with your new patch in last night, everything is OK.
>
> For x86, I try it in prev some day, everything is OK too.
>
> This pc don't have the gdb-cvs-head and I cannot connect gdb cvs now.
> I will try it later.
>
> Thanks,
> Hui
>

Re: Bug in i386_process_record

[Hui Zhu]

About rex in i386:
        case 0x40:
        case 0x41:
        case 0x42:
        case 0x43:
        case 0x44:
        case 0x45:
        case 0x46:
        case 0x47:
        case 0x48:
        case 0x49:
        case 0x4a:
        case 0x4b:
        case 0x4c:
        case 0x4d:
        case 0x4e:
        case 0x4f:
          if (ir.regmap[X86_RECORD_R8_REGNUM])
            {

Only in amd64, ir.regmap[X86_RECORD_R8_REGNUM] is true, so it will not
affect 32bits.

Thanks,
Hui

On Tue, Aug 18, 2009 at 17:06, Hui Zhu<teawater@gmail.com> wrote:
> I tried.  Everything is OK.
>
> Thanks,
> Hui
>
> On Tue, Aug 18, 2009 at 07:01, Hui Zhu<teawater@gmail.com> wrote:
>> On Tue, Aug 18, 2009 at 02:24, Michael Snyder<msnyder@vmware.com> wrote:
>>> Hui Zhu wrote:
>>>
>>>> (gdb) FAIL: gdb.reverse/machinestate.exp: step
>>>>
>>>> I think the fail is because rs didn't step into hide.
>>>
>>> 32bit or 64?
>>> If 64, did you try it after applying my two recent patches?
>>>
>>> Anyway, that is not the only reason it fails.
>>> It fails for me on 32 bit intel.
>>
>> I try amd64 with your new patch in last night, everything is OK.
>>
>> For x86, I try it in prev some day, everything is OK too.
>>
>> This pc don't have the gdb-cvs-head and I cannot connect gdb cvs now.
>> I will try it later.
>>
>> Thanks,
>> Hui
>>
>