From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3461 invoked by alias); 18 Aug 2009 09:22:13 -0000 Received: (qmail 3439 invoked by uid 22791); 18 Aug 2009 09:22:10 -0000 X-SWARE-Spam-Status: No, hits=-1.1 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_22,J_CHICKENPOX_25,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-px0-f193.google.com (HELO mail-px0-f193.google.com) (209.85.216.193) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 18 Aug 2009 09:22:01 +0000 Received: by pxi31 with SMTP id 31so1930004pxi.24 for ; Tue, 18 Aug 2009 02:21:59 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.74.7 with SMTP id w7mr846225wfa.149.1250587319140; Tue, 18 Aug 2009 02:21:59 -0700 (PDT) In-Reply-To: <4A8A2ACD.9000208@vmware.com> References: <4A7BA1DE.6010103@vmware.com> <4A8097B4.2080709@vmware.com> <4A8A2ACD.9000208@vmware.com> From: Hui Zhu Date: Tue, 18 Aug 2009 11:52:00 -0000 Message-ID: Subject: Re: Bug in i386_process_record? To: Michael Snyder Cc: gdb-patches ml Content-Type: multipart/mixed; boundary=001636e1fb26eb00c40471670cff 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-08/txt/msg00273.txt.bz2 --001636e1fb26eb00c40471670cff Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-length: 4918 On Tue, Aug 18, 2009 at 12:15, Michael Snyder wrote: > Hui Zhu wrote: >> >> On Tue, Aug 11, 2009 at 05:57, Michael Snyder wrote: >>> >>> Yes, this seems to be better. =A0It records only 4 bytes each time >>> it is called. >>> >>> But there seems to be still an off-by-one error? =A0With the test >>> program that I provided, we call memset with an argument of >>> 1024, but we actually record 1025 bytes... this code gets hit >>> 257 times, with the last time recording only 1 byte. >>> >>> >> >> Hi Michael, >> >> This issue is because: >> >> 0xb7edf4e7 : rep stos %eax,%es:(%edi) >> 0xb7edf4e9 : mov =A0 =A0%edx,%ecx >> 0xb7edf4eb : rep stos %al,%es:(%edi) >> 0xb7edf4ed : mov =A0 =A00x8(%esp),%eax >> 0xb7edf4f1 : pop =A0 =A0%edi >> >> If the memcpy size is not align with 4, it will handle by second rep sto= s. >> Then rep stos will not execute if %ecx is 0. >> i386_process_record doesn't check %ecx, so it get this error. >> >> I make a new patch for it. =A0Please help me review it. > > This seems much better. =A0Please give us a change log and post it for re= view. > > By the way, I'm sorry, I only just realized that I posted two > completely different bug reports with the exact same subject line. > ;-( > Don't worry about it. Gmail handle it very well. :) Thanks, Hui 2009-08-18 Hui Zhu * record.c (i386_process_record): Remove some error code. --- i386-tdep.c | 61 +++++++++++++++++++++++--------------------------------= ----- 1 file changed, 24 insertions(+), 37 deletions(-) --- a/i386-tdep.c +++ b/i386-tdep.c @@ -4441,50 +4441,37 @@ reswitch: /* insS */ case 0x6c: case 0x6d: - if ((opcode & 1) =3D=3D 0) - ir.ot =3D OT_BYTE; - else - ir.ot =3D ir.dflag + OT_WORD; regcache_raw_read_unsigned (ir.regcache, - ir.regmap[X86_RECORD_REDI_REGNUM], + ir.regmap[X86_RECORD_RECX_REGNUM], &tmpulongest); - if (!ir.aflag) - { - tmpulongest &=3D 0xffff; - /* addr +=3D ((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 segment " - "register.\n"), - paddress (gdbarch, ir.addr)); - } - if (prefixes & (PREFIX_REPZ | PREFIX_REPNZ)) + if (tmpulongest) { - ULONGEST count, eflags; + if ((opcode & 1) =3D=3D 0) + ir.ot =3D OT_BYTE; + else + ir.ot =3D ir.dflag + OT_WORD; regcache_raw_read_unsigned (ir.regcache, ir.regmap[X86_RECORD_REDI_REGNUM], - &count); - if (!ir.aflag) - count &=3D 0xffff; - regcache_raw_read_unsigned (ir.regcache, - ir.regmap[X86_RECORD_EFLAGS_REGNUM], - &eflags); - if ((eflags >> 10) & 0x1) - tmpulongest -=3D (count - 1) * (1 << ir.ot); - if (record_arch_list_add_mem (tmpulongest, count * (1 << ir.ot))) - return -1; - I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RECX_REGNUM); - } - else - { + &tmpulongest); + if (ir.aflag) + { + /* addr +=3D ((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 becau= se " + "it can't get the value of the segmen= t " + "register.\n"), + paddress (gdbarch, ir.addr)); + } + 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 =3D=3D 0xa4 || opcode =3D=3D 0xa5) - I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RESI_REGNUM); - I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDI_REGNUM); - I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM); + if (opcode =3D=3D 0xa4 || opcode =3D=3D 0xa5) + I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_RESI_REGNUM); + I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDI_REGNUM); + I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_EFLAGS_REGNUM); + } break; /* cmpsS */ --001636e1fb26eb00c40471670cff Content-Type: text/plain; charset=US-ASCII; name="prec-fix-x86-strinsn.txt" Content-Disposition: attachment; filename="prec-fix-x86-strinsn.txt" Content-Transfer-Encoding: base64 X-Attachment-Id: f_fyifij960 Content-length: 4584 LS0tCiBpMzg2LXRkZXAuYyB8ICAgNjEgKysrKysrKysrKysrKysrKysrKysr KystLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tCiAxIGZp bGUgY2hhbmdlZCwgMjQgaW5zZXJ0aW9ucygrKSwgMzcgZGVsZXRpb25zKC0p CgotLS0gYS9pMzg2LXRkZXAuYworKysgYi9pMzg2LXRkZXAuYwpAQCAtNDQ0 MSw1MCArNDQ0MSwzNyBAQCByZXN3aXRjaDoKICAgICAgIC8qIGluc1MgKi8K ICAgICBjYXNlIDB4NmM6CiAgICAgY2FzZSAweDZkOgotICAgICAgaWYgKChv cGNvZGUgJiAxKSA9PSAwKQotCWlyLm90ID0gT1RfQllURTsKLSAgICAgIGVs c2UKLQlpci5vdCA9IGlyLmRmbGFnICsgT1RfV09SRDsKICAgICAgIHJlZ2Nh Y2hlX3Jhd19yZWFkX3Vuc2lnbmVkIChpci5yZWdjYWNoZSwKLSAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICBpci5yZWdtYXBbWDg2X1JFQ09S RF9SRURJX1JFR05VTV0sCisgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgaXIucmVnbWFwW1g4Nl9SRUNPUkRfUkVDWF9SRUdOVU1dLAogICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICZ0bXB1bG9uZ2VzdCk7 Ci0gICAgICBpZiAoIWlyLmFmbGFnKQotICAgICAgICB7Ci0gICAgICAgICAg dG1wdWxvbmdlc3QgJj0gMHhmZmZmOwotICAgICAgICAgIC8qIGFkZHIgKz0g KCh1aW50MzJfdCkgcmVhZF9yZWdpc3RlciAoSTM4Nl9FU19SRUdOVU0pKSA8 PCA0OyAqLwotICAgICAgICAgIGlmIChyZWNvcmRfZGVidWcpCi0gICAgICAg ICAgICBwcmludGZfdW5maWx0ZXJlZCAoXygiUHJvY2VzcyByZWNvcmQgaWdu b3JlcyB0aGUgbWVtb3J5IGNoYW5nZSAiCi0gICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAib2YgaW5zdHJ1Y3Rpb24gYXQgYWRkcmVzcyAweCVz IGJlY2F1c2UgIgotICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg Iml0IGNhbid0IGdldCB0aGUgdmFsdWUgb2YgdGhlIHNlZ21lbnQgIgotICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgInJlZ2lzdGVyLlxuIiks Ci0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgcGFkZHJlc3MgKGdk YmFyY2gsIGlyLmFkZHIpKTsKLSAgICAgICAgfQotICAgICAgaWYgKHByZWZp eGVzICYgKFBSRUZJWF9SRVBaIHwgUFJFRklYX1JFUE5aKSkKKyAgICAgIGlm ICh0bXB1bG9uZ2VzdCkKICAgICAgICAgewotICAgICAgICAgIFVMT05HRVNU IGNvdW50LCBlZmxhZ3M7CisgICAgICAgICAgaWYgKChvcGNvZGUgJiAxKSA9 PSAwKQorCSAgICBpci5vdCA9IE9UX0JZVEU7CisgICAgICAgICAgZWxzZQor CSAgICBpci5vdCA9IGlyLmRmbGFnICsgT1RfV09SRDsKICAgICAgICAgICBy ZWdjYWNoZV9yYXdfcmVhZF91bnNpZ25lZCAoaXIucmVnY2FjaGUsCiAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGlyLnJlZ21hcFtY ODZfUkVDT1JEX1JFRElfUkVHTlVNXSwKLSAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgJmNvdW50KTsKLSAgICAgICAgICBpZiAoIWly LmFmbGFnKQotICAgICAgICAgICAgY291bnQgJj0gMHhmZmZmOwotICAgICAg ICAgIHJlZ2NhY2hlX3Jhd19yZWFkX3Vuc2lnbmVkIChpci5yZWdjYWNoZSwK LSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgaXIucmVn bWFwW1g4Nl9SRUNPUkRfRUZMQUdTX1JFR05VTV0sCi0gICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICZlZmxhZ3MpOwotICAgICAgICAg IGlmICgoZWZsYWdzID4+IDEwKSAmIDB4MSkKLSAgICAgICAgICAgIHRtcHVs b25nZXN0IC09IChjb3VudCAtIDEpICogKDEgPDwgaXIub3QpOwotICAgICAg ICAgIGlmIChyZWNvcmRfYXJjaF9saXN0X2FkZF9tZW0gKHRtcHVsb25nZXN0 LCBjb3VudCAqICgxIDw8IGlyLm90KSkpCi0gICAgICAgICAgICByZXR1cm4g LTE7Ci0gICAgICAgICAgSTM4Nl9SRUNPUkRfQVJDSF9MSVNUX0FERF9SRUcg KFg4Nl9SRUNPUkRfUkVDWF9SRUdOVU0pOwotICAgICAgICB9Ci0gICAgICBl bHNlCi0gICAgICAgIHsKKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgJnRtcHVsb25nZXN0KTsKKyAgICAgICAgICBpZiAoaXIuYWZs YWcpCisgICAgICAgICAgICB7CisgICAgICAgICAgICAgIC8qIGFkZHIgKz0g KCh1aW50MzJfdCkgcmVhZF9yZWdpc3RlciAoSTM4Nl9FU19SRUdOVU0pKSA8 PCA0OyAqLworICAgICAgICAgICAgICBpZiAocmVjb3JkX2RlYnVnKQorICAg ICAgICAgICAgICAgIHByaW50Zl91bmZpbHRlcmVkIChfKCJQcm9jZXNzIHJl Y29yZCBpZ25vcmVzIHRoZSBtZW1vcnkgY2hhbmdlICIKKyAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAib2YgaW5zdHJ1Y3Rpb24gYXQg YWRkcmVzcyAweCVzIGJlY2F1c2UgIgorICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICJpdCBjYW4ndCBnZXQgdGhlIHZhbHVlIG9mIHRo ZSBzZWdtZW50ICIKKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAicmVnaXN0ZXIuXG4iKSwKKyAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgcGFkZHJlc3MgKGdkYmFyY2gsIGlyLmFkZHIpKTsKKyAg ICAgICAgICAgIH0KKyAgICAgICAgICBpZiAocHJlZml4ZXMgJiAoUFJFRklY X1JFUFogfCBQUkVGSVhfUkVQTlopKQorICAgICAgICAgICAgSTM4Nl9SRUNP UkRfQVJDSF9MSVNUX0FERF9SRUcgKFg4Nl9SRUNPUkRfUkVDWF9SRUdOVU0p OwogICAgICAgICAgIGlmIChyZWNvcmRfYXJjaF9saXN0X2FkZF9tZW0gKHRt cHVsb25nZXN0LCAxIDw8IGlyLm90KSkKICAgICAgICAgICAgIHJldHVybiAt MTsKLSAgICAgICAgfQotICAgICAgaWYgKG9wY29kZSA9PSAweGE0IHx8IG9w Y29kZSA9PSAweGE1KQotICAgICAgICBJMzg2X1JFQ09SRF9BUkNIX0xJU1Rf QUREX1JFRyAoWDg2X1JFQ09SRF9SRVNJX1JFR05VTSk7Ci0gICAgICBJMzg2 X1JFQ09SRF9BUkNIX0xJU1RfQUREX1JFRyAoWDg2X1JFQ09SRF9SRURJX1JF R05VTSk7Ci0gICAgICBJMzg2X1JFQ09SRF9BUkNIX0xJU1RfQUREX1JFRyAo WDg2X1JFQ09SRF9FRkxBR1NfUkVHTlVNKTsKKyAgICAgICAgICBpZiAob3Bj b2RlID09IDB4YTQgfHwgb3Bjb2RlID09IDB4YTUpCisgICAgICAgICAgICBJ Mzg2X1JFQ09SRF9BUkNIX0xJU1RfQUREX1JFRyAoWDg2X1JFQ09SRF9SRVNJ X1JFR05VTSk7CisgICAgICAgICAgSTM4Nl9SRUNPUkRfQVJDSF9MSVNUX0FE RF9SRUcgKFg4Nl9SRUNPUkRfUkVESV9SRUdOVU0pOworICAgICAgICAgIEkz ODZfUkVDT1JEX0FSQ0hfTElTVF9BRERfUkVHIChYODZfUkVDT1JEX0VGTEFH U19SRUdOVU0pOworCX0KICAgICAgIGJyZWFrOwogCiAgICAgICAvKiBjbXBz UyAqLwo= --001636e1fb26eb00c40471670cff--