* Re: [PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support
[not found] ` <7d77a27d0910111921v13a8e8acpb98c29a9aa72ed8@mail.gmail.com>
@ 2010-09-16 18:01 ` Jiang Jilin
2010-09-17 8:33 ` Michael Snyder
0 siblings, 1 reply; 6+ messages in thread
From: Jiang Jilin @ 2010-09-16 18:01 UTC (permalink / raw)
To: Michael Snyder; +Cc: Hui Zhu, gdb-patches ml, tromey
Hi,
I'm very sorry due to my work, however, it's good news that the I've
finished the paper assignment.
Should I rewrite the patch again? Any comments are welcome!
Thank you!
On Mon, Oct 12, 2009 at 10:21 AM, Jiang Jilin <freephp@gmail.com> wrote:
> On Mon, Oct 12, 2009 at 10:04 AM, Michael Snyder <msnyder@vmware.com> wrote:
>> Jiang Jilin wrote:
>>>
>>> Hi, guys
>>>
>>> I've rewrite the codes for opcode 0x0f01 with more readable, add
>>> xgetbv/xsetbv/rdtscp/vmcall/vmlaunch/vmresume/vmxoff instructions
>>> support as well.
>>>
>>> However, I'm *not* sure it's whether right or not, especially with
>>> the new supported instructions beginning with "vm". And I remove all
>>> codes to save EFLAGS register which is not specified to be saved by
>>> Intel's manual, so please help me review them.
>>>
>>> Luckily, there is no regression when using precord.exp board file to test.
>>>
>>> At last but not least, there is some differences in gdb.sum when
>>> 'make check' before and after applying this patch. I cannot make
>>> a decision whether it's correct, so please help me. The diff are
>>> as follows:
>>
>> Ah well, but you see, now the change is too big to be accepted
>> without a copyright assignment. Do you want to start the process
>> of filing one?
>
> I've done as Joel told me, now I just wait for the paper arrival.
>
> Anyway, thank you! :)
>
>> [Cc: Tom Tromey]
>>
> --
> Jiang
>
--
Jiang
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support
2010-09-16 18:01 ` [PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support Jiang Jilin
@ 2010-09-17 8:33 ` Michael Snyder
2011-03-29 12:07 ` Jiang Jilin
0 siblings, 1 reply; 6+ messages in thread
From: Michael Snyder @ 2010-09-17 8:33 UTC (permalink / raw)
To: Jiang Jilin; +Cc: Hui Zhu, gdb-patches ml, tromey
Since it is a pretty actively changing file, it would probably be good
to supply a new diff and changelog, please.
Hui, you should be one of the reviewers, yes?
Jiang Jilin wrote:
> Hi,
>
> I'm very sorry due to my work, however, it's good news that the I've
> finished the paper assignment.
>
> Should I rewrite the patch again? Any comments are welcome!
>
> Thank you!
>
> On Mon, Oct 12, 2009 at 10:21 AM, Jiang Jilin <freephp@gmail.com> wrote:
>> On Mon, Oct 12, 2009 at 10:04 AM, Michael Snyder <msnyder@vmware.com> wrote:
>>> Jiang Jilin wrote:
>>>> Hi, guys
>>>>
>>>> I've rewrite the codes for opcode 0x0f01 with more readable, add
>>>> xgetbv/xsetbv/rdtscp/vmcall/vmlaunch/vmresume/vmxoff instructions
>>>> support as well.
>>>>
>>>> However, I'm *not* sure it's whether right or not, especially with
>>>> the new supported instructions beginning with "vm". And I remove all
>>>> codes to save EFLAGS register which is not specified to be saved by
>>>> Intel's manual, so please help me review them.
>>>>
>>>> Luckily, there is no regression when using precord.exp board file to test.
>>>>
>>>> At last but not least, there is some differences in gdb.sum when
>>>> 'make check' before and after applying this patch. I cannot make
>>>> a decision whether it's correct, so please help me. The diff are
>>>> as follows:
>>> Ah well, but you see, now the change is too big to be accepted
>>> without a copyright assignment. Do you want to start the process
>>> of filing one?
>> I've done as Joel told me, now I just wait for the paper arrival.
>>
>> Anyway, thank you! :)
>>
>>> [Cc: Tom Tromey]
>>>
>> --
>> Jiang
>>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support
2010-09-17 8:33 ` Michael Snyder
@ 2011-03-29 12:07 ` Jiang Jilin
2011-03-29 12:21 ` Yao Qi
0 siblings, 1 reply; 6+ messages in thread
From: Jiang Jilin @ 2011-03-29 12:07 UTC (permalink / raw)
To: Michael Snyder; +Cc: Hui Zhu, gdb-patches ml, tromey
[-- Attachment #1: Type: text/plain, Size: 10305 bytes --]
Michael,
Miss you! I hope you could get the patch.
Sorry to Michael and Hui, it should be finished earlier.
No regression found after testing precord.exp.
Hui, please help review it.
2011-03-29 Jiang Jilin <freephp@gmail.com>
* i386-tdep.c (i386_process_record): Rewrite the codes for
opcode 0x0f01 and add more instructions support
---
gdb/i386-tdep.c | 281 ++++++++++++++++++++++++-------------------------------
1 files changed, 122 insertions(+), 159 deletions(-)
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index c7ad3a6..94833cb 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -5802,174 +5802,137 @@ Do you want to stop the program?"),
case 0x0f01:
if (i386_record_modrm (&ir))
return -1;
- switch (ir.reg)
+ if (ir.mod == 3)
{
- case 0: /* sgdt */
- {
- uint64_t addr64;
-
- if (ir.mod == 3)
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- if (ir.override >= 0)
- {
- if (record_memory_query)
- {
- int q;
+ uint8_t reg_rm = (ir.reg << 4) | ir.rm;
- target_terminal_ours ();
- q = yquery (_("\
-Process record ignores the memory change of instruction at address %s\n\
-because it can't get the value of the segment register.\n\
-Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
- return -1;
- }
- }
- else
- {
- if (i386_record_lea_modrm_addr (&ir, &addr64))
- return -1;
- if (record_arch_list_add_mem (addr64, 2))
- return -1;
- addr64 += 2;
+ switch (reg_rm)
+ {
+ /* vmcall */
+ case 0x01:
+ /* vmlaunch */
+ case 0x02:
+ /* vmresume */
+ case 0x03:
+ /* vmxoff */
+ case 0x04:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+ break;
+ /* monitor */
+ case 0x10:
+ break;
+ /* mwait */
+ case 0x11:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+ break;
+ /* xgetbv */
+ case 0x20:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
+ break;
+ /* xsetbv */
+ case 0x21:
+ break;
+ /* swapgs */
+ case 0x70:
if (ir.regmap[X86_RECORD_R8_REGNUM])
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_GS_REGNUM);
+ else
{
- if (record_arch_list_add_mem (addr64, 8))
- return -1;
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
}
- else
+ break;
+ /* rdtscp */
+ case 0x71:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
+ break;
+ default:
+ /* smsw */
+ if (ir.reg == 4)
{
- if (record_arch_list_add_mem (addr64, 4))
- return -1;
+ I386_RECORD_ARCH_LIST_ADD_REG (ir.rm | ir.rex_b);
+ break;
}
- }
- }
- break;
- case 1:
- if (ir.mod == 3)
- {
- switch (ir.rm)
- {
- case 0: /* monitor */
- break;
- case 1: /* mwait */
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- default:
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- break;
- }
- }
- else
- {
- /* sidt */
- if (ir.override >= 0)
- {
- if (record_memory_query)
- {
- int q;
-
- target_terminal_ours ();
- q = yquery (_("\
-Process record ignores the memory change of instruction at address %s\n\
-because it can't get the value of the segment register.\n\
-Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
- return -1;
- }
- }
- else
- {
- uint64_t addr64;
+ /* lmsw */
+ else if (ir.reg == 6)
+ break;
- if (i386_record_lea_modrm_addr (&ir, &addr64))
- return -1;
- if (record_arch_list_add_mem (addr64, 2))
- return -1;
- addr64 += 2;
- if (ir.regmap[X86_RECORD_R8_REGNUM])
- {
- if (record_arch_list_add_mem (addr64, 8))
- return -1;
- }
- else
- {
- if (record_arch_list_add_mem (addr64, 4))
- return -1;
- }
- }
- }
- break;
- case 2: /* lgdt */
- if (ir.mod == 3)
- {
- /* xgetbv */
- if (ir.rm == 0)
- {
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
- break;
- }
- /* xsetbv */
- else if (ir.rm == 1)
- break;
- }
- case 3: /* lidt */
- if (ir.mod == 3)
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- break;
- case 4: /* smsw */
- if (ir.mod == 3)
- {
- if (record_arch_list_add_reg (ir.regcache, ir.rm | ir.rex_b))
- return -1;
- }
- else
- {
- ir.ot = OT_WORD;
- if (i386_record_lea_modrm (&ir))
- return -1;
- }
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- case 6: /* lmsw */
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- case 7: /* invlpg */
- if (ir.mod == 3)
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
+ }
+ }
+ else
+ {
+ switch (ir.reg)
{
- if (ir.rm == 0 && ir.regmap[X86_RECORD_R8_REGNUM])
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_GS_REGNUM);
- else
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- }
- else
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- default:
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- break;
- }
+ /* sgdt */
+ case 0:
+ /* sidt */
+ case 1:
+ if (ir.override >= 0)
+ {
+ if (record_memory_query)
+ {
+ int q;
+
+ target_terminal_ours ();
+ q = yquery (_("\
+ Process record ignores the memory change of \
+ instruction at address %s\n\
+ because it can't get the value of the segment \
+ register.\n Do you want to stop the program?"),
+ paddress (gdbarch, ir.orig_addr));
+ target_terminal_inferior ();
+ if (q)
+ return -1;
+ }
+ }
+ else
+ {
+ uint64_t tmpu64;
+
+ /* We have to store at least (4 + 2 = 6) bytes,
+ or (8 + 2 = 10) bytes at most. */
+ if (i386_record_lea_modrm_addr (&ir, &tmpu64))
+ return -1;
+ if (record_arch_list_add_mem (tmpu64, 6))
+ return -1;
+ tmpu64 += 6;
+ if (ir.regmap[X86_RECORD_R8_REGNUM])
+ {
+ if (record_arch_list_add_mem (tmpu64, 4))
+ return -1;
+ }
+ }
+ break;
+ /* lgdt */
+ case 2:
+ /* lidt */
+ case 3:
+ break;
+ /* smsw */
+ case 4:
+ ir.ot = OT_WORD;
+ if (i386_record_lea_modrm (&ir))
+ return -1;
+ break;
+ /* lmsw */
+ case 6:
+ break;
+ /* invlpg */
+ case 7:
+ break;
+ default:
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
+ }
+ }
break;
case 0x0f08: /* invd */
--
1.7.0.4
[-- Attachment #2: 0001-Rewrite-the-codes-for-opcode-0x0f01-and-add-more-ins.patch --]
[-- Type: text/x-patch, Size: 9077 bytes --]
2011-03-29 Jiang Jilin <freephp@gmail.com>
* i386-tdep.c (i386_process_record): Rewrite the codes for
opcode 0x0f01 and add more instructions support
---
gdb/i386-tdep.c | 281 ++++++++++++++++++++++++-------------------------------
1 files changed, 122 insertions(+), 159 deletions(-)
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index c7ad3a6..94833cb 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -5802,174 +5802,137 @@ Do you want to stop the program?"),
case 0x0f01:
if (i386_record_modrm (&ir))
return -1;
- switch (ir.reg)
+ if (ir.mod == 3)
{
- case 0: /* sgdt */
- {
- uint64_t addr64;
-
- if (ir.mod == 3)
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- if (ir.override >= 0)
- {
- if (record_memory_query)
- {
- int q;
+ uint8_t reg_rm = (ir.reg << 4) | ir.rm;
- target_terminal_ours ();
- q = yquery (_("\
-Process record ignores the memory change of instruction at address %s\n\
-because it can't get the value of the segment register.\n\
-Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
- return -1;
- }
- }
- else
- {
- if (i386_record_lea_modrm_addr (&ir, &addr64))
- return -1;
- if (record_arch_list_add_mem (addr64, 2))
- return -1;
- addr64 += 2;
+ switch (reg_rm)
+ {
+ /* vmcall */
+ case 0x01:
+ /* vmlaunch */
+ case 0x02:
+ /* vmresume */
+ case 0x03:
+ /* vmxoff */
+ case 0x04:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+ break;
+ /* monitor */
+ case 0x10:
+ break;
+ /* mwait */
+ case 0x11:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+ break;
+ /* xgetbv */
+ case 0x20:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
+ break;
+ /* xsetbv */
+ case 0x21:
+ break;
+ /* swapgs */
+ case 0x70:
if (ir.regmap[X86_RECORD_R8_REGNUM])
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_GS_REGNUM);
+ else
{
- if (record_arch_list_add_mem (addr64, 8))
- return -1;
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
}
- else
+ break;
+ /* rdtscp */
+ case 0x71:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
+ break;
+ default:
+ /* smsw */
+ if (ir.reg == 4)
{
- if (record_arch_list_add_mem (addr64, 4))
- return -1;
+ I386_RECORD_ARCH_LIST_ADD_REG (ir.rm | ir.rex_b);
+ break;
}
- }
- }
- break;
- case 1:
- if (ir.mod == 3)
- {
- switch (ir.rm)
- {
- case 0: /* monitor */
- break;
- case 1: /* mwait */
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- default:
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- break;
- }
- }
- else
- {
- /* sidt */
- if (ir.override >= 0)
- {
- if (record_memory_query)
- {
- int q;
-
- target_terminal_ours ();
- q = yquery (_("\
-Process record ignores the memory change of instruction at address %s\n\
-because it can't get the value of the segment register.\n\
-Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
- return -1;
- }
- }
- else
- {
- uint64_t addr64;
+ /* lmsw */
+ else if (ir.reg == 6)
+ break;
- if (i386_record_lea_modrm_addr (&ir, &addr64))
- return -1;
- if (record_arch_list_add_mem (addr64, 2))
- return -1;
- addr64 += 2;
- if (ir.regmap[X86_RECORD_R8_REGNUM])
- {
- if (record_arch_list_add_mem (addr64, 8))
- return -1;
- }
- else
- {
- if (record_arch_list_add_mem (addr64, 4))
- return -1;
- }
- }
- }
- break;
- case 2: /* lgdt */
- if (ir.mod == 3)
- {
- /* xgetbv */
- if (ir.rm == 0)
- {
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
- break;
- }
- /* xsetbv */
- else if (ir.rm == 1)
- break;
- }
- case 3: /* lidt */
- if (ir.mod == 3)
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- break;
- case 4: /* smsw */
- if (ir.mod == 3)
- {
- if (record_arch_list_add_reg (ir.regcache, ir.rm | ir.rex_b))
- return -1;
- }
- else
- {
- ir.ot = OT_WORD;
- if (i386_record_lea_modrm (&ir))
- return -1;
- }
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- case 6: /* lmsw */
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- case 7: /* invlpg */
- if (ir.mod == 3)
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
+ }
+ }
+ else
+ {
+ switch (ir.reg)
{
- if (ir.rm == 0 && ir.regmap[X86_RECORD_R8_REGNUM])
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_GS_REGNUM);
- else
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- }
- else
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- default:
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- break;
- }
+ /* sgdt */
+ case 0:
+ /* sidt */
+ case 1:
+ if (ir.override >= 0)
+ {
+ if (record_memory_query)
+ {
+ int q;
+
+ target_terminal_ours ();
+ q = yquery (_("\
+ Process record ignores the memory change of \
+ instruction at address %s\n\
+ because it can't get the value of the segment \
+ register.\n Do you want to stop the program?"),
+ paddress (gdbarch, ir.orig_addr));
+ target_terminal_inferior ();
+ if (q)
+ return -1;
+ }
+ }
+ else
+ {
+ uint64_t tmpu64;
+
+ /* We have to store at least (4 + 2 = 6) bytes,
+ or (8 + 2 = 10) bytes at most. */
+ if (i386_record_lea_modrm_addr (&ir, &tmpu64))
+ return -1;
+ if (record_arch_list_add_mem (tmpu64, 6))
+ return -1;
+ tmpu64 += 6;
+ if (ir.regmap[X86_RECORD_R8_REGNUM])
+ {
+ if (record_arch_list_add_mem (tmpu64, 4))
+ return -1;
+ }
+ }
+ break;
+ /* lgdt */
+ case 2:
+ /* lidt */
+ case 3:
+ break;
+ /* smsw */
+ case 4:
+ ir.ot = OT_WORD;
+ if (i386_record_lea_modrm (&ir))
+ return -1;
+ break;
+ /* lmsw */
+ case 6:
+ break;
+ /* invlpg */
+ case 7:
+ break;
+ default:
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
+ }
+ }
break;
case 0x0f08: /* invd */
--
1.7.0.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support
2011-03-29 12:07 ` Jiang Jilin
@ 2011-03-29 12:21 ` Yao Qi
2011-03-29 14:28 ` Jiang Jilin
0 siblings, 1 reply; 6+ messages in thread
From: Yao Qi @ 2011-03-29 12:21 UTC (permalink / raw)
To: gdb-patches
On 03/29/2011 06:11 PM, Jiang Jilin wrote:
I am not the people to approve this patch. Some comments on code format
and indent.
> 2011-03-29 Jiang Jilin <freephp@gmail.com>
There should be a newline here. Please reference other changelog entries.
> * i386-tdep.c (i386_process_record): Rewrite the codes for
> opcode 0x0f01 and add more instructions support
> + switch (reg_rm)
> + {
> + /* vmcall */
> + case 0x01:
> + /* vmlaunch */
Looks like there are two extra spaces in front of your comment. Please
remove them. There are several instances of this problem in your patch.
> +
> + /* We have to store at least (4 + 2 = 6) bytes,
> + or (8 + 2 = 10) bytes at most. */
According to my limited English knowledge, it should be "6 bytes at
least *and* 10 bytes at most".
> + if (i386_record_lea_modrm_addr (&ir, &tmpu64))
> + return -1;
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support
2011-03-29 12:21 ` Yao Qi
@ 2011-03-29 14:28 ` Jiang Jilin
2011-05-19 19:27 ` Tom Tromey
0 siblings, 1 reply; 6+ messages in thread
From: Jiang Jilin @ 2011-03-29 14:28 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches, Hui Zhu, tromey
[-- Attachment #1: Type: text/plain, Size: 11729 bytes --]
On Tue, Mar 29, 2011 at 7:10 PM, Yao Qi <yao@codesourcery.com> wrote:
> On 03/29/2011 06:11 PM, Jiang Jilin wrote:
>
> I am not the people to approve this patch. Some comments on code format
> and indent.
>
>> 2011-03-29 Jiang Jilin <freephp@gmail.com>
>
> There should be a newline here. Please reference other changelog entries.
fixed.
>> * i386-tdep.c (i386_process_record): Rewrite the codes for
>> opcode 0x0f01 and add more instructions support
>
>> + switch (reg_rm)
>> + {
>> + /* vmcall */
>> + case 0x01:
>> + /* vmlaunch */
> Looks like there are two extra spaces in front of your comment. Please
> remove them. There are several instances of this problem in your patch.
fixed.
>
>> +
>> + /* We have to store at least (4 + 2 = 6) bytes,
>> + or (8 + 2 = 10) bytes at most. */
>
> According to my limited English knowledge, it should be "6 bytes at
> least *and* 10 bytes at most".
fixed.
>
>> + if (i386_record_lea_modrm_addr (&ir, &tmpu64))
>> + return -1;
>
>
> --
> Yao (齐尧)
>
Yao, Thank you!
The new version:
2011-03-29 Jiang Jilin <freephp@gmail.com>
* i386-tdep.c (i386_process_record): Rewrite the codes for
opcode 0x0f01 and add more instructions support.
---
gdb/i386-tdep.c | 281 ++++++++++++++++++++++++-------------------------------
1 files changed, 122 insertions(+), 159 deletions(-)
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index c7ad3a6..631a505 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -5802,174 +5802,137 @@ Do you want to stop the program?"),
case 0x0f01:
if (i386_record_modrm (&ir))
return -1;
- switch (ir.reg)
+ if (ir.mod == 3)
{
- case 0: /* sgdt */
- {
- uint64_t addr64;
-
- if (ir.mod == 3)
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- if (ir.override >= 0)
- {
- if (record_memory_query)
- {
- int q;
+ uint8_t reg_rm = (ir.reg << 4) | ir.rm;
- target_terminal_ours ();
- q = yquery (_("\
-Process record ignores the memory change of instruction at address %s\n\
-because it can't get the value of the segment register.\n\
-Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
- return -1;
- }
- }
- else
- {
- if (i386_record_lea_modrm_addr (&ir, &addr64))
- return -1;
- if (record_arch_list_add_mem (addr64, 2))
- return -1;
- addr64 += 2;
+ switch (reg_rm)
+ {
+ /* vmcall */
+ case 0x01:
+ /* vmlaunch */
+ case 0x02:
+ /* vmresume */
+ case 0x03:
+ /* vmxoff */
+ case 0x04:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+ break;
+ /* monitor */
+ case 0x10:
+ break;
+ /* mwait */
+ case 0x11:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+ break;
+ /* xgetbv */
+ case 0x20:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
+ break;
+ /* xsetbv */
+ case 0x21:
+ break;
+ /* swapgs */
+ case 0x70:
if (ir.regmap[X86_RECORD_R8_REGNUM])
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_GS_REGNUM);
+ else
{
- if (record_arch_list_add_mem (addr64, 8))
- return -1;
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
}
- else
+ break;
+ /* rdtscp */
+ case 0x71:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
+ break;
+ default:
+ /* smsw */
+ if (ir.reg == 4)
{
- if (record_arch_list_add_mem (addr64, 4))
- return -1;
+ I386_RECORD_ARCH_LIST_ADD_REG (ir.rm | ir.rex_b);
+ break;
}
- }
- }
- break;
- case 1:
- if (ir.mod == 3)
- {
- switch (ir.rm)
- {
- case 0: /* monitor */
- break;
- case 1: /* mwait */
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- default:
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- break;
- }
- }
- else
- {
- /* sidt */
- if (ir.override >= 0)
- {
- if (record_memory_query)
- {
- int q;
-
- target_terminal_ours ();
- q = yquery (_("\
-Process record ignores the memory change of instruction at address %s\n\
-because it can't get the value of the segment register.\n\
-Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
- return -1;
- }
- }
- else
- {
- uint64_t addr64;
+ /* lmsw */
+ else if (ir.reg == 6)
+ break;
- if (i386_record_lea_modrm_addr (&ir, &addr64))
- return -1;
- if (record_arch_list_add_mem (addr64, 2))
- return -1;
- addr64 += 2;
- if (ir.regmap[X86_RECORD_R8_REGNUM])
- {
- if (record_arch_list_add_mem (addr64, 8))
- return -1;
- }
- else
- {
- if (record_arch_list_add_mem (addr64, 4))
- return -1;
- }
- }
- }
- break;
- case 2: /* lgdt */
- if (ir.mod == 3)
- {
- /* xgetbv */
- if (ir.rm == 0)
- {
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
- break;
- }
- /* xsetbv */
- else if (ir.rm == 1)
- break;
- }
- case 3: /* lidt */
- if (ir.mod == 3)
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- break;
- case 4: /* smsw */
- if (ir.mod == 3)
- {
- if (record_arch_list_add_reg (ir.regcache, ir.rm | ir.rex_b))
- return -1;
- }
- else
- {
- ir.ot = OT_WORD;
- if (i386_record_lea_modrm (&ir))
- return -1;
- }
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- case 6: /* lmsw */
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- case 7: /* invlpg */
- if (ir.mod == 3)
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
+ }
+ }
+ else
+ {
+ switch (ir.reg)
{
- if (ir.rm == 0 && ir.regmap[X86_RECORD_R8_REGNUM])
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_GS_REGNUM);
- else
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- }
- else
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- default:
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- break;
- }
+ /* sgdt */
+ case 0:
+ /* sidt */
+ case 1:
+ if (ir.override >= 0)
+ {
+ if (record_memory_query)
+ {
+ int q;
+
+ target_terminal_ours ();
+ q = yquery (_("\
+ Process record ignores the memory change of \
+ instruction at address %s\n\
+ because it can't get the value of the segment \
+ register.\n Do you want to stop the program?"),
+ paddress (gdbarch, ir.orig_addr));
+ target_terminal_inferior ();
+ if (q)
+ return -1;
+ }
+ }
+ else
+ {
+ uint64_t tmpu64;
+
+ /* We have to store (4 + 2 = 6) bytes at least,
+ and (8 + 2 = 10) bytes at most. */
+ if (i386_record_lea_modrm_addr (&ir, &tmpu64))
+ return -1;
+ if (record_arch_list_add_mem (tmpu64, 6))
+ return -1;
+ tmpu64 += 6;
+ if (ir.regmap[X86_RECORD_R8_REGNUM])
+ {
+ if (record_arch_list_add_mem (tmpu64, 4))
+ return -1;
+ }
+ }
+ break;
+ /* lgdt */
+ case 2:
+ /* lidt */
+ case 3:
+ break;
+ /* smsw */
+ case 4:
+ ir.ot = OT_WORD;
+ if (i386_record_lea_modrm (&ir))
+ return -1;
+ break;
+ /* lmsw */
+ case 6:
+ break;
+ /* invlpg */
+ case 7:
+ break;
+ default:
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
+ }
+ }
break;
case 0x0f08: /* invd */
--
1.7.0.4
[-- Attachment #2: 0001-Rewrite-the-codes-for-opcode-0x0f01-and-add-more-ins.patch --]
[-- Type: text/x-patch, Size: 9278 bytes --]
From ce36bed46faab3de61747e483e3278cba8dc3f2d Mon Sep 17 00:00:00 2001
From: Jiang Jilin <freephp@gmail.com>
Date: Tue, 29 Mar 2011 21:36:41 +0800
Subject: [PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support
2011-03-29 Jiang Jilin <freephp@gmail.com>
* i386-tdep.c (i386_process_record): Rewrite the codes for
opcode 0x0f01 and add more instructions support.
---
gdb/i386-tdep.c | 281 ++++++++++++++++++++++++-------------------------------
1 files changed, 122 insertions(+), 159 deletions(-)
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index c7ad3a6..631a505 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -5802,174 +5802,137 @@ Do you want to stop the program?"),
case 0x0f01:
if (i386_record_modrm (&ir))
return -1;
- switch (ir.reg)
+ if (ir.mod == 3)
{
- case 0: /* sgdt */
- {
- uint64_t addr64;
-
- if (ir.mod == 3)
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- if (ir.override >= 0)
- {
- if (record_memory_query)
- {
- int q;
+ uint8_t reg_rm = (ir.reg << 4) | ir.rm;
- target_terminal_ours ();
- q = yquery (_("\
-Process record ignores the memory change of instruction at address %s\n\
-because it can't get the value of the segment register.\n\
-Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
- return -1;
- }
- }
- else
- {
- if (i386_record_lea_modrm_addr (&ir, &addr64))
- return -1;
- if (record_arch_list_add_mem (addr64, 2))
- return -1;
- addr64 += 2;
+ switch (reg_rm)
+ {
+ /* vmcall */
+ case 0x01:
+ /* vmlaunch */
+ case 0x02:
+ /* vmresume */
+ case 0x03:
+ /* vmxoff */
+ case 0x04:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+ break;
+ /* monitor */
+ case 0x10:
+ break;
+ /* mwait */
+ case 0x11:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
+ break;
+ /* xgetbv */
+ case 0x20:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
+ break;
+ /* xsetbv */
+ case 0x21:
+ break;
+ /* swapgs */
+ case 0x70:
if (ir.regmap[X86_RECORD_R8_REGNUM])
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_GS_REGNUM);
+ else
{
- if (record_arch_list_add_mem (addr64, 8))
- return -1;
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
}
- else
+ break;
+ /* rdtscp */
+ case 0x71:
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM);
+ I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
+ break;
+ default:
+ /* smsw */
+ if (ir.reg == 4)
{
- if (record_arch_list_add_mem (addr64, 4))
- return -1;
+ I386_RECORD_ARCH_LIST_ADD_REG (ir.rm | ir.rex_b);
+ break;
}
- }
- }
- break;
- case 1:
- if (ir.mod == 3)
- {
- switch (ir.rm)
- {
- case 0: /* monitor */
- break;
- case 1: /* mwait */
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- default:
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- break;
- }
- }
- else
- {
- /* sidt */
- if (ir.override >= 0)
- {
- if (record_memory_query)
- {
- int q;
-
- target_terminal_ours ();
- q = yquery (_("\
-Process record ignores the memory change of instruction at address %s\n\
-because it can't get the value of the segment register.\n\
-Do you want to stop the program?"),
- paddress (gdbarch, ir.orig_addr));
- target_terminal_inferior ();
- if (q)
- return -1;
- }
- }
- else
- {
- uint64_t addr64;
+ /* lmsw */
+ else if (ir.reg == 6)
+ break;
- if (i386_record_lea_modrm_addr (&ir, &addr64))
- return -1;
- if (record_arch_list_add_mem (addr64, 2))
- return -1;
- addr64 += 2;
- if (ir.regmap[X86_RECORD_R8_REGNUM])
- {
- if (record_arch_list_add_mem (addr64, 8))
- return -1;
- }
- else
- {
- if (record_arch_list_add_mem (addr64, 4))
- return -1;
- }
- }
- }
- break;
- case 2: /* lgdt */
- if (ir.mod == 3)
- {
- /* xgetbv */
- if (ir.rm == 0)
- {
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
- break;
- }
- /* xsetbv */
- else if (ir.rm == 1)
- break;
- }
- case 3: /* lidt */
- if (ir.mod == 3)
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- break;
- case 4: /* smsw */
- if (ir.mod == 3)
- {
- if (record_arch_list_add_reg (ir.regcache, ir.rm | ir.rex_b))
- return -1;
- }
- else
- {
- ir.ot = OT_WORD;
- if (i386_record_lea_modrm (&ir))
- return -1;
- }
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- case 6: /* lmsw */
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- case 7: /* invlpg */
- if (ir.mod == 3)
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
+ }
+ }
+ else
+ {
+ switch (ir.reg)
{
- if (ir.rm == 0 && ir.regmap[X86_RECORD_R8_REGNUM])
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_GS_REGNUM);
- else
- {
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- }
- }
- else
- I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM);
- break;
- default:
- ir.addr -= 3;
- opcode = opcode << 8 | ir.modrm;
- goto no_support;
- break;
- }
+ /* sgdt */
+ case 0:
+ /* sidt */
+ case 1:
+ if (ir.override >= 0)
+ {
+ if (record_memory_query)
+ {
+ int q;
+
+ target_terminal_ours ();
+ q = yquery (_("\
+ Process record ignores the memory change of \
+ instruction at address %s\n\
+ because it can't get the value of the segment \
+ register.\n Do you want to stop the program?"),
+ paddress (gdbarch, ir.orig_addr));
+ target_terminal_inferior ();
+ if (q)
+ return -1;
+ }
+ }
+ else
+ {
+ uint64_t tmpu64;
+
+ /* We have to store (4 + 2 = 6) bytes at least,
+ and (8 + 2 = 10) bytes at most. */
+ if (i386_record_lea_modrm_addr (&ir, &tmpu64))
+ return -1;
+ if (record_arch_list_add_mem (tmpu64, 6))
+ return -1;
+ tmpu64 += 6;
+ if (ir.regmap[X86_RECORD_R8_REGNUM])
+ {
+ if (record_arch_list_add_mem (tmpu64, 4))
+ return -1;
+ }
+ }
+ break;
+ /* lgdt */
+ case 2:
+ /* lidt */
+ case 3:
+ break;
+ /* smsw */
+ case 4:
+ ir.ot = OT_WORD;
+ if (i386_record_lea_modrm (&ir))
+ return -1;
+ break;
+ /* lmsw */
+ case 6:
+ break;
+ /* invlpg */
+ case 7:
+ break;
+ default:
+ ir.addr -= 3;
+ opcode = opcode << 8 | ir.modrm;
+ goto no_support;
+ }
+ }
break;
case 0x0f08: /* invd */
--
1.7.0.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support
2011-03-29 14:28 ` Jiang Jilin
@ 2011-05-19 19:27 ` Tom Tromey
0 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2011-05-19 19:27 UTC (permalink / raw)
To: Jiang Jilin; +Cc: Yao Qi, gdb-patches, Hui Zhu
>>>>> "Jiang" == Jiang Jilin <freephp@gmail.com> writes:
I'm sorry you haven't gotten a reply to this.
Jiang> 2011-03-29 Jiang Jilin <freephp@gmail.com>
Jiang> * i386-tdep.c (i386_process_record): Rewrite the codes for
Jiang> opcode 0x0f01 and add more instructions support.
I am not the right person to review this.
I would have to look up all this stuff in the x86 references to try to
understand it. I'd appreciate it if Hui would reply about these details.
Most of the code seems to at least be in the same style as the
surrounding code, so at least on those grounds I find it basically ok.
Jiang> + switch (reg_rm)
Jiang> + {
Jiang> + /* vmcall */
Jiang> + case 0x01:
It looks like there is an extra space in the indentation here.
Jiang> + q = yquery (_("\
Jiang> + Process record ignores the memory change of \
Jiang> + instruction at address %s\n\
Jiang> + because it can't get the value of the segment \
Jiang> + register.\n Do you want to stop the program?"),
Reformatting this message like this is not ok.
The way it was before the patch is more correct.
Tom
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-05-19 19:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1255182393-15292-1-git-send-email-freephp@gmail.com>
[not found] ` <4AD28E95.1050404@vmware.com>
[not found] ` <7d77a27d0910111921v13a8e8acpb98c29a9aa72ed8@mail.gmail.com>
2010-09-16 18:01 ` [PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support Jiang Jilin
2010-09-17 8:33 ` Michael Snyder
2011-03-29 12:07 ` Jiang Jilin
2011-03-29 12:21 ` Yao Qi
2011-03-29 14:28 ` Jiang Jilin
2011-05-19 19:27 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).