From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19970 invoked by alias); 25 May 2010 03:05:14 -0000 Received: (qmail 19959 invoked by uid 22791); 25 May 2010 03:05:12 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from mail-px0-f169.google.com (HELO mail-px0-f169.google.com) (209.85.212.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 25 May 2010 03:05:07 +0000 Received: by pxi12 with SMTP id 12so4663260pxi.0 for ; Mon, 24 May 2010 20:05:06 -0700 (PDT) Received: by 10.142.66.23 with SMTP id o23mr4158780wfa.321.1274756706098; Mon, 24 May 2010 20:05:06 -0700 (PDT) MIME-Version: 1.0 Received: by 10.143.45.13 with HTTP; Mon, 24 May 2010 20:04:46 -0700 (PDT) In-Reply-To: <201001081624.12634.pedro@codesourcery.com> References: <200912241738.19780.pedro@codesourcery.com> <201001081624.12634.pedro@codesourcery.com> From: Hui Zhu Date: Tue, 25 May 2010 05:14:00 -0000 Message-ID: Subject: Re: [RFC] Add support of software single step to process record To: Pedro Alves , ping huang , shuchang zhou Cc: gdb-patches@sourceware.org, Joel Brobecker , Michael Snyder , paawan oza , Tom Tromey Content-Type: multipart/mixed; boundary=001636e0a72ba45ea10487626ca8 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: 2010-05/txt/msg00562.txt.bz2 --001636e0a72ba45ea10487626ca8 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-length: 5931 Thanks Pedro. On Sat, Jan 9, 2010 at 00:24, Pedro Alves wrote: > On Monday 04 January 2010 14:23:21, Hui Zhu wrote: >> Sorry guys, the prev patch is so ugly. > > :-) > >> Thanks for teach me clear about the gdbarch_software_single_step, Pedro. >> I did some extend with your idea. =A0Because record_wait need >> record_resume_step point out this resume is signal step or continue. >> >> =A0 =A0 =A0 if (!step) >> =A0 =A0 =A0 =A0 { >> =A0 =A0 =A0 =A0 =A0 /* This is not hard single step. =A0*/ >> =A0 =A0 =A0 =A0 =A0 if (!gdbarch_software_single_step_p (gdbarch)) >> =A0 =A0 =A0 =A0 =A0 =A0 { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* This is a normal continue. =A0*/ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 step =3D 1; >> =A0 =A0 =A0 =A0 =A0 =A0 } >> =A0 =A0 =A0 =A0 =A0 else >> =A0 =A0 =A0 =A0 =A0 =A0 { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* This arch support soft sigle step. =A0*/ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (single_step_breakpoints_inserted ()) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* This is a soft single step. =A0*/ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 record_resume_step =3D 1; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 else >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* This is a continue. >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Try to insert a soft single s= tep breakpoint. =A0*/ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!gdbarch_software_single_step (g= dbarch, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0get_current_frame ())) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* This system don't want us= e soft single step. >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Use hard sigle step. = =A0*/ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 step =3D 1; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> =A0 =A0 =A0 =A0 =A0 =A0 } >> =A0 =A0 =A0 =A0 } > > Cool, this looks pretty clear to me now. =A0Thanks. > > > >> @@ -1077,6 +1111,7 @@ record_wait (struct target_ops *ops, >> =A0 =A0 =A0 =A0 =A0 /* This is not a single step. =A0*/ >> =A0 =A0 =A0 =A0 =A0 ptid_t ret; >> =A0 =A0 =A0 =A0 =A0 CORE_ADDR tmp_pc; >> + =A0 =A0 =A0 =A0 =A0struct gdbarch *gdbarch =3D target_thread_architect= ure (inferior_ptid); >> >> =A0 =A0 =A0 =A0 =A0 while (1) >> =A0 =A0 =A0 =A0 =A0 =A0 { >> @@ -1099,6 +1134,9 @@ record_wait (struct target_ops *ops, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tmp_pc =3D regcache_read_pc (regcach= e); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 aspace =3D get_regcache_aspace (regc= ache); >> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (gdbarch_software_single_step_p = (gdbarch)) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0remove_single_step_breakpoints = (); > > This will gdb_assert inside remove_single_step_breakpoints > if SSS bkpts are not inserted, but gdbarch_software_single_step_p > returns true. =A0This instead is safer: > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (single_step_breakpoints_inserted (= )) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0remove_single_step_breakpoints (); OK. I will fix it. > > But, what if it was infrun that had inserted the single-step > breakpoints, for a "next" or "step", etc.? =A0Shouldn't you check > for record_resume_step too? > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!record_resume_step && single_step_breakp= oints_inserted ()) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 remove_single_step_breakpoints (); > > Otherwise, the check below for > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else if (breakpoint_inserted_here_p (a= space, tmp_pc)) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0{ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* There is a breakpoint here.= =A0Let the core > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 handle it. =A0*/ > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (software_breakpoint_insert= ed_here_p (aspace, tmp_pc)) > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0{ > > would fail, and the finished single-step wouldn't be reported to the > core, right? I think this single step will be handle by line: if (record_resume_step) { /* This is a single step. */ return record_beneath_to_wait (record_beneath_to_wait_ops, ptid, status, options); } > > > Lastly, you may also want to confirm that the SSS bkpt managed by record.= d itself explains the SIGTRAP before removing before issueing another > single-step. =A0If any unexplainable SIGTRAP happens for any reason while > single-stepping, you should report it to infrun instead. =A0In other word= s: > > With software single-stepping, we can distinguish most random > SIGTRAPs from SSS SIGTRAPs, so: > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* This must be a single-step = trap. =A0Record the > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 insn and issue another st= ep. =A0*/ > > ... the "must" here ends up being a bit too strong. =A0I'd certainly > understand ignoring this for simplicity or performance reasons though. Ah. Looks we didn't have good way to handle it. I change this comment to: /* This is a single-step trap. Record the insn and issue another step. FIXME: this part can be a random SIGTRAP too. But GDB cannot handle it. */ Shuchang, could you try your code just use command si and reverse-xxx. If that part OK. Please help me try this patch. Ping, please help me test this patch. And about hellogcc, you can find us = in: https://groups.google.com/group/hellogcc https://webchat.freenode.net/ #hellogcc Thanks, Hui 2010-05-25 Hui Zhu * breakpoint.c (single_step_breakpoints_inserted): New function. * breakpoint.h (single_step_breakpoints_inserted): Extern. * record.c (record_resume): Add code for software single step. (record_wait): Ditto. --001636e0a72ba45ea10487626ca8 Content-Type: text/plain; charset=US-ASCII; name="prec_software_single_step.txt" Content-Disposition: attachment; filename="prec_software_single_step.txt" Content-Transfer-Encoding: base64 X-Attachment-Id: f_g9m5oyf50 Content-length: 5897 LS0tCiBicmVha3BvaW50LmMgfCAgIDEwICsrKysrKysrKysKIGJyZWFrcG9p bnQuaCB8ICAgIDEgKwogcmVjb3JkLmMgICAgIHwgICA1NyArKysrKysrKysr KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrLS0t LS0KIDMgZmlsZXMgY2hhbmdlZCwgNjMgaW5zZXJ0aW9ucygrKSwgNSBkZWxl dGlvbnMoLSkKCi0tLSBhL2JyZWFrcG9pbnQuYworKysgYi9icmVha3BvaW50 LmMKQEAgLTEwMjkzLDYgKzEwMjkzLDE2IEBAIGluc2VydF9zaW5nbGVfc3Rl cF9icmVha3BvaW50IChzdHJ1Y3QgZ2QKIAkgICAgIHBhZGRyZXNzIChnZGJh cmNoLCBuZXh0X3BjKSk7CiB9CiAKKy8qIENoZWNrIGlmIHRoZSBicmVha3Bv aW50cyB1c2VkIGZvciBzb2Z0d2FyZSBzaW5nbGUgc3RlcHBpbmcKKyAgIHdl cmUgaW5zZXJ0ZWQgb3Igbm90LiAgKi8KKworaW50CitzaW5nbGVfc3RlcF9i cmVha3BvaW50c19pbnNlcnRlZCAodm9pZCkKK3sKKyAgcmV0dXJuIChzaW5n bGVfc3RlcF9icmVha3BvaW50c1swXSAhPSBOVUxMCisgICAgICAgICAgfHwg c2luZ2xlX3N0ZXBfYnJlYWtwb2ludHNbMV0gIT0gTlVMTCk7Cit9CisKIC8q IFJlbW92ZSBhbmQgZGVsZXRlIGFueSBicmVha3BvaW50cyB1c2VkIGZvciBz b2Z0d2FyZSBzaW5nbGUgc3RlcC4gICovCiAKIHZvaWQKLS0tIGEvYnJlYWtw b2ludC5oCisrKyBiL2JyZWFrcG9pbnQuaApAQCAtOTgzLDYgKzk4Myw3IEBA IGV4dGVybiBpbnQgcmVtb3ZlX2h3X3dhdGNocG9pbnRzICh2b2lkKTsKICAg IHR3aWNlIGJlZm9yZSByZW1vdmUgaXMgY2FsbGVkLiAgKi8KIGV4dGVybiB2 b2lkIGluc2VydF9zaW5nbGVfc3RlcF9icmVha3BvaW50IChzdHJ1Y3QgZ2Ri YXJjaCAqLAogCQkJCQkgICBzdHJ1Y3QgYWRkcmVzc19zcGFjZSAqLCBDT1JF X0FERFIpOworZXh0ZXJuIGludCBzaW5nbGVfc3RlcF9icmVha3BvaW50c19p bnNlcnRlZCAodm9pZCk7CiBleHRlcm4gdm9pZCByZW1vdmVfc2luZ2xlX3N0 ZXBfYnJlYWtwb2ludHMgKHZvaWQpOwogCiAvKiBNYW5hZ2UgbWFudWFsIGJy ZWFrcG9pbnRzLCBzZXBhcmF0ZSBmcm9tIHRoZSBub3JtYWwgY2hhaW4gb2YK LS0tIGEvcmVjb3JkLmMKKysrIGIvcmVjb3JkLmMKQEAgLTEwMDcsOSArMTAw Nyw0MyBAQCByZWNvcmRfcmVzdW1lIChzdHJ1Y3QgdGFyZ2V0X29wcyAqb3Bz LCBwCiAKICAgaWYgKCFSRUNPUkRfSVNfUkVQTEFZKQogICAgIHsKKyAgICAg IHN0cnVjdCBnZGJhcmNoICpnZGJhcmNoID0gdGFyZ2V0X3RocmVhZF9hcmNo aXRlY3R1cmUgKHB0aWQpOworCiAgICAgICByZWNvcmRfbWVzc2FnZSAoZ2V0 X2N1cnJlbnRfcmVnY2FjaGUgKCksIHNpZ25hbCk7Ci0gICAgICByZWNvcmRf YmVuZWF0aF90b19yZXN1bWUgKHJlY29yZF9iZW5lYXRoX3RvX3Jlc3VtZV9v cHMsIHB0aWQsIDEsCi0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg IHNpZ25hbCk7CisKKyAgICAgIGlmICghc3RlcCkKKyAgICAgICAgeworICAg ICAgICAgIC8qIFRoaXMgaXMgbm90IGhhcmQgc2luZ2xlIHN0ZXAuICAqLwor ICAgICAgICAgIGlmICghZ2RiYXJjaF9zb2Z0d2FyZV9zaW5nbGVfc3RlcF9w IChnZGJhcmNoKSkKKyAgICAgICAgICAgIHsKKyAgICAgICAgICAgICAgLyog VGhpcyBpcyBhIG5vcm1hbCBjb250aW51ZS4gICovCisgICAgICAgICAgICAg IHN0ZXAgPSAxOworICAgICAgICAgICAgfQorICAgICAgICAgIGVsc2UKKyAg ICAgICAgICAgIHsKKyAgICAgICAgICAgICAgLyogVGhpcyBhcmNoIHN1cHBv cnQgc29mdCBzaWdsZSBzdGVwLiAgKi8KKyAgICAgICAgICAgICAgaWYgKHNp bmdsZV9zdGVwX2JyZWFrcG9pbnRzX2luc2VydGVkICgpKQorICAgICAgICAg ICAgICAgIHsKKyAgICAgICAgICAgICAgICAgIC8qIFRoaXMgaXMgYSBzb2Z0 IHNpbmdsZSBzdGVwLiAgKi8KKyAgICAgICAgICAgICAgICAgIHJlY29yZF9y ZXN1bWVfc3RlcCA9IDE7CisgICAgICAgICAgICAgICAgfQorICAgICAgICAg ICAgICBlbHNlCisgICAgICAgICAgICAgICAgeworICAgICAgICAgICAgICAg ICAgLyogVGhpcyBpcyBhIGNvbnRpbnVlLgorICAgICAgICAgICAgICAgICAg ICAgVHJ5IHRvIGluc2VydCBhIHNvZnQgc2luZ2xlIHN0ZXAgYnJlYWtwb2lu dC4gICovCisgICAgICAgICAgICAgICAgICBpZiAoIWdkYmFyY2hfc29mdHdh cmVfc2luZ2xlX3N0ZXAgKGdkYmFyY2gsCisgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGdldF9jdXJyZW50 X2ZyYW1lICgpKSkKKyAgICAgICAgICAgICAgICAgICAgeworICAgICAgICAg ICAgICAgICAgICAgIC8qIFRoaXMgc3lzdGVtIGRvbid0IHdhbnQgdXNlIHNv ZnQgc2luZ2xlIHN0ZXAuCisgICAgICAgICAgICAgICAgICAgICAgICAgVXNl IGhhcmQgc2lnbGUgc3RlcC4gICovCisgICAgICAgICAgICAgICAgICAgICAg c3RlcCA9IDE7CisgICAgICAgICAgICAgICAgICAgIH0KKyAgICAgICAgICAg ICAgICB9CisgICAgICAgICAgICB9CisgICAgICAgIH0KKworICAgICAgcmVj b3JkX2JlbmVhdGhfdG9fcmVzdW1lIChyZWNvcmRfYmVuZWF0aF90b19yZXN1 bWVfb3BzLAorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBwdGlk LCBzdGVwLCBzaWduYWwpOwogICAgIH0KIH0KIApAQCAtMTA4Miw2ICsxMTE2 LDcgQEAgcmVjb3JkX3dhaXQgKHN0cnVjdCB0YXJnZXRfb3BzICpvcHMsCiAJ ICAvKiBUaGlzIGlzIG5vdCBhIHNpbmdsZSBzdGVwLiAgKi8KIAkgIHB0aWRf dCByZXQ7CiAJICBDT1JFX0FERFIgdG1wX3BjOworICAgICAgICAgIHN0cnVj dCBnZGJhcmNoICpnZGJhcmNoID0gdGFyZ2V0X3RocmVhZF9hcmNoaXRlY3R1 cmUgKGluZmVyaW9yX3B0aWQpOwogCiAJICB3aGlsZSAoMSkKIAkgICAgewpA QCAtMTEwNCw2ICsxMTM5LDkgQEAgcmVjb3JkX3dhaXQgKHN0cnVjdCB0YXJn ZXRfb3BzICpvcHMsCiAJCSAgdG1wX3BjID0gcmVnY2FjaGVfcmVhZF9wYyAo cmVnY2FjaGUpOwogCQkgIGFzcGFjZSA9IGdldF9yZWdjYWNoZV9hc3BhY2Ug KHJlZ2NhY2hlKTsKIAorICAgICAgICAgICAgICAgICAgaWYgKHNpbmdsZV9z dGVwX2JyZWFrcG9pbnRzX2luc2VydGVkICgpKQorICAgICAgICAgICAgICAg ICAgICByZW1vdmVfc2luZ2xlX3N0ZXBfYnJlYWtwb2ludHMgKCk7CisKIAkJ ICBpZiAodGFyZ2V0X3N0b3BwZWRfYnlfd2F0Y2hwb2ludCAoKSkKIAkJICAg IHsKIAkJICAgICAgLyogQWx3YXlzIGludGVyZXN0ZWQgaW4gd2F0Y2hwb2lu dHMuICAqLwpAQCAtMTEyNCw4ICsxMTYyLDEyIEBAIHJlY29yZF93YWl0IChz dHJ1Y3QgdGFyZ2V0X29wcyAqb3BzLAogCQkgICAgfQogCQkgIGVsc2UKIAkJ ICAgIHsKLQkJICAgICAgLyogVGhpcyBtdXN0IGJlIGEgc2luZ2xlLXN0ZXAg dHJhcC4gIFJlY29yZCB0aGUKLQkJICAgICAgICAgaW5zbiBhbmQgaXNzdWUg YW5vdGhlciBzdGVwLiAgKi8KKwkJICAgICAgLyogVGhpcyBpcyBhIHNpbmds ZS1zdGVwIHRyYXAuICBSZWNvcmQgdGhlCisJCSAgICAgICAgIGluc24gYW5k IGlzc3VlIGFub3RoZXIgc3RlcC4KKyAgICAgICAgICAgICAgICAgICAgICAg ICBGSVhNRTogdGhpcyBwYXJ0IGNhbiBiZSBhIHJhbmRvbSBTSUdUUkFQIHRv by4KKyAgICAgICAgICAgICAgICAgICAgICAgICBCdXQgR0RCIGNhbm5vdCBo YW5kbGUgaXQuICAqLworICAgICAgICAgICAgICAgICAgICAgIGludCBzdGVw ID0gMTsKKwogCQkgICAgICBpZiAoIXJlY29yZF9tZXNzYWdlX3dyYXBwZXJf c2FmZSAocmVnY2FjaGUsCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgIFRBUkdFVF9TSUdOQUxfMCkp CiAgIAkJCXsKQEAgLTExMzQsOCArMTE3NiwxMyBAQCByZWNvcmRfd2FpdCAo c3RydWN0IHRhcmdldF9vcHMgKm9wcywKICAgICAgICAgICAgICAgICAgICAg ICAgICAgIGJyZWFrOwogICAJCQl9CiAKKyAgICAgICAgICAgICAgICAgICAg ICBpZiAoZ2RiYXJjaF9zb2Z0d2FyZV9zaW5nbGVfc3RlcF9wIChnZGJhcmNo KQorICAgICAgICAgICAgICAgICAgICAgICAgICAmJiBnZGJhcmNoX3NvZnR3 YXJlX3NpbmdsZV9zdGVwIChnZGJhcmNoLAorICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBnZXRf Y3VycmVudF9mcmFtZSAoKSkpCisgICAgICAgICAgICAgICAgICAgICAgICBz dGVwID0gMDsKKwogCQkgICAgICByZWNvcmRfYmVuZWF0aF90b19yZXN1bWUg KHJlY29yZF9iZW5lYXRoX3RvX3Jlc3VtZV9vcHMsCi0JCQkJCQlwdGlkLCAx LAorCQkJCQkJcHRpZCwgc3RlcCwKIAkJCQkJCVRBUkdFVF9TSUdOQUxfMCk7 CiAJCSAgICAgIGNvbnRpbnVlOwogCQkgICAgfQo= --001636e0a72ba45ea10487626ca8--