From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32502 invoked by alias); 12 Oct 2009 02:08:56 -0000 Received: (qmail 32490 invoked by uid 22791); 12 Oct 2009 02:08:53 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from smtp-outbound-1.vmware.com (HELO smtp-outbound-1.vmware.com) (65.115.85.69) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 12 Oct 2009 02:08:49 +0000 Received: from mailhost3.vmware.com (mailhost3.vmware.com [10.16.27.45]) by smtp-outbound-1.vmware.com (Postfix) with ESMTP id 2374548002; Sun, 11 Oct 2009 19:08:48 -0700 (PDT) Received: from [10.20.94.141] (msnyder-server.eng.vmware.com [10.20.94.141]) by mailhost3.vmware.com (Postfix) with ESMTP id 13ECCCD97E; Sun, 11 Oct 2009 19:08:48 -0700 (PDT) Message-ID: <4AD28E95.1050404@vmware.com> Date: Mon, 12 Oct 2009 02:08:00 -0000 From: Michael Snyder User-Agent: Thunderbird 1.5.0.12 (X11/20080411) MIME-Version: 1.0 To: Jiang Jilin CC: Hui Zhu , gdb-patches ml , "tromey@redhat.com" Subject: Re: [PATCH] Rewrite the codes for opcode 0x0f01 and add more instructions support References: <> <1255182393-15292-1-git-send-email-freephp@gmail.com> In-Reply-To: <1255182393-15292-1-git-send-email-freephp@gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-10/txt/msg00222.txt.bz2 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? [Cc: Tom Tromey] > > --- testsuite/gdb.sum.before 2009-10-10 20:02:35.000000000 +0800 > +++ testsuite/gdb.sum.after 2009-10-10 20:21:35.000000000 +0800 > @@ -1,4 +1,4 @@ > -Test Run By jiang on Sat Oct 10 19:44:30 2009 > +Test Run By jiang on Sat Oct 10 20:05:20 2009 > Native configuration is i686-pc-linux-gnu > > === gdb tests === > @@ -14113,18 +14113,18 @@ > PASS: gdb.threads/watchthreads2.exp: all threads started > PASS: gdb.threads/watchthreads2.exp: watch x > PASS: gdb.threads/watchthreads2.exp: set var test_ready = 1 > -KFAIL: gdb.threads/watchthreads2.exp: gdb can drop watchpoints in multithreaded app (PRMS: gdb/10116) > +PASS: gdb.threads/watchthreads2.exp: all threads incremented x > Running ./gdb.threads/watchthreads.exp ... > PASS: gdb.threads/watchthreads.exp: successfully compiled posix threads test case > PASS: gdb.threads/watchthreads.exp: watch args[0] > PASS: gdb.threads/watchthreads.exp: watch args[1] > PASS: gdb.threads/watchthreads.exp: disable 2 > -FAIL: gdb.threads/watchthreads.exp: threaded watch loop > +PASS: gdb.threads/watchthreads.exp: threaded watch loop > PASS: gdb.threads/watchthreads.exp: first watchpoint on args[0] hit > PASS: gdb.threads/watchthreads.exp: first watchpoint on args[1] hit > PASS: gdb.threads/watchthreads.exp: watchpoint on args[0] hit in thread > PASS: gdb.threads/watchthreads.exp: watchpoint on args[1] hit in thread > -FAIL: gdb.threads/watchthreads.exp: combination of threaded watchpoints = 30 > +PASS: gdb.threads/watchthreads.exp: combination of threaded watchpoints = 30 > Running ./gdb.trace/actions.exp ... > PASS: gdb.trace/actions.exp: 5.1a: set three tracepoints, no actions > PASS: gdb.trace/actions.exp: 5.1b: set actions for first tracepoint > @@ -14288,8 +14288,8 @@ > > === gdb Summary === > > -# of expected passes 13512 > -# of unexpected failures 76 > +# of expected passes 13515 > +# of unexpected failures 74 > > 2009-10-10 Jiang Jilin > * i386-tdep.c (i386_process_record): Rewrite the codes for > opcode 0x0f01 and add more instructions support > --- > gdb/i386-tdep.c | 234 ++++++++++++++++++++++++------------------------------ > 1 files changed, 104 insertions(+), 130 deletions(-) > > diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c > index b79bcd2..2300a91 100644 > --- a/gdb/i386-tdep.c > +++ b/gdb/i386-tdep.c > @@ -5075,152 +5075,126 @@ reswitch: > case 0x0f01: > if (i386_record_modrm (&ir)) > return -1; > - switch (ir.reg) > + if (ir.mod == 3) > { > - /* sgdt */ > - case 0: > - { > - uint64_t tmpu64; > - > - if (ir.mod == 3) > - { > - ir.addr -= 3; > - opcode = opcode << 8 | ir.modrm; > - goto no_support; > - } > - if (ir.override >= 0) > - { > - 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 > - { > - if (i386_record_lea_modrm_addr (&ir, &tmpu64)) > - return -1; > - if (record_arch_list_add_mem (tmpu64, 2)) > - return -1; > - tmpu64 += 2; > - if (ir.regmap[X86_RECORD_R8_REGNUM]) > - { > - if (record_arch_list_add_mem (tmpu64, 8)) > - return -1; > - } > - else > - { > - if (record_arch_list_add_mem (tmpu64, 4)) > - return -1; > - } > - } > - } > - break; > - case 1: > - if (ir.mod == 3) > + uint8_t reg_rm = (ir.reg << 4) | ir.rm; > + > + switch (reg_rm) > { > - switch (ir.rm) > - { > - /* monitor */ > - case 0: > - break; > - /* mwait */ > - case 1: > - I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM); > - break; > - default: > - ir.addr -= 3; > - opcode = opcode << 8 | ir.modrm; > - goto no_support; > - break; > + /* 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 > + { > + ir.addr -= 3; > + opcode = opcode << 8 | ir.modrm; > + goto no_support; > + } > + 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) > + { > + I386_RECORD_ARCH_LIST_ADD_REG (ir.rm | ir.rex_b); > + break; > } > + /* lmsw */ > + else if (ir.reg == 6) > + break; > + ir.addr -= 3; > + opcode = opcode << 8 | ir.modrm; > + goto no_support; > } > - else > + } > + else > + { > + switch (ir.reg) > { > - /* sidt */ > + /* sgdt */ > + case 0: > + /* sidt */ > + case 1: > if (ir.override >= 0) > - { > + { > 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)); > + "change of instruction at " > + "address %s because it can't get " > + "the value of the segment " > + "register."), > + paddress (gdbarch, ir.orig_addr)); > } > else > - { > - uint64_t tmpu64; > + { > + uint64_t tmpu64; > > - if (i386_record_lea_modrm_addr (&ir, &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; > - if (record_arch_list_add_mem (tmpu64, 2)) > - return -1; > - addr += 2; > - if (ir.regmap[X86_RECORD_R8_REGNUM]) > - { > - if (record_arch_list_add_mem (tmpu64, 8)) > - return -1; > - } > - else > - { > - if (record_arch_list_add_mem (tmpu64, 4)) > - 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: > - if (ir.mod == 3) > - { > - ir.addr -= 3; > - opcode = opcode << 8 | ir.modrm; > - goto no_support; > - } > - break; > - /* smsw */ > - case 4: > - if (ir.mod == 3) > - { > - if (record_arch_list_add_reg (ir.regcache, ir.rm | ir.rex_b)) > - return -1; > - } > - else > - { > + 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; > } > - I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM); > - break; > - /* lmsw */ > - case 6: > - I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM); > - break; > - /* invlpg */ > - case 7: > - if (ir.mod == 3) > - { > - 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; > } > break; >