From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11019 invoked by alias); 23 Dec 2009 06:38:47 -0000 Received: (qmail 11006 invoked by uid 22791); 23 Dec 2009 06:38:46 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail-pw0-f49.google.com (HELO mail-pw0-f49.google.com) (209.85.160.49) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 23 Dec 2009 06:38:41 +0000 Received: by pwj12 with SMTP id 12so4933921pwj.8 for ; Tue, 22 Dec 2009 22:38:40 -0800 (PST) MIME-Version: 1.0 Received: by 10.142.66.13 with SMTP id o13mr6426969wfa.307.1261550320087; Tue, 22 Dec 2009 22:38:40 -0800 (PST) In-Reply-To: <20091220133009.GI2788@adacore.com> References: <4B2BD97E.1020106@vmware.com> <20091220133009.GI2788@adacore.com> From: Hui Zhu Date: Wed, 23 Dec 2009 06:38:00 -0000 Message-ID: Subject: Re: [RFC] Add support of software single step to process record To: Joel Brobecker , Michael Snyder , shuchang zhou , paawan oza , Tom Tromey Cc: gdb-patches ml Content-Type: multipart/mixed; boundary=001636e0a510b1f3ea047b5f92ba 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-12/txt/msg00356.txt.bz2 --001636e0a510b1f3ea047b5f92ba Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-length: 1764 Hi guys, I make a new patch according to your comment. Shuchang, please help me try this patch? Joel, I remove the current_gdbarch() from this patch, but for get_current_frame, I cannot find any function that fit for replace it. Could you help me with it? Thanks, Hui On Sun, Dec 20, 2009 at 21:30, Joel Brobecker wrote: > Hui: > >> >2009-12-18 =A0Hui Zhu =A0 >> > >> > =A0 =A0 * breakpoint.c (inserted_single_step_breakpoint_p): New >> > =A0 =A0 function. >> > =A0 =A0 * breakpoint.h (inserted_single_step_breakpoint_p): Extern. >> > =A0 =A0 * record.c (record_resume): Add code for software single step. >> > =A0 =A0 (record_wait): Ditto. > > I understand Michael's answer as approval. =A0I do not see any problem > with it, but my knowledge of the target stack in the resume/wait area > is pretty sketchy. > > Just a stylistic comment on the patch: > >> >+/* Check if the breakpoints used for software single stepping >> >inserted or not. =A0*/ > > Formatting and missing "are". > > /* Check if the breakpoints used for software single stepping > =A0 are inserted or not. =A0*/ > >> >+int >> >+inserted_single_step_breakpoint_p (void) > > Can you rename this function to: > > =A0 =A0 =A0 =A0single_step_breakpoints_inserted > > The "inserted" already conveys the idea of a condition/predicate, > so the _p is superfluous in this case. > > I'm also a little worried about the code adding calls to functions > that in essence return a global variable. For instance, you are > introducing calls to get_current_frame or current_gdbarch(). > Ulrich is one of our specialists in this area, whereas I'm not sure, > but I am wondering if we are introducing any latent issue by using > these routines... > > -- > Joel > --001636e0a510b1f3ea047b5f92ba 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_g3jqrr9r0 Content-length: 4677 LS0tCiBicmVha3BvaW50LmMgfCAgIDEzICsrKysrKysrKysrKysKIGJyZWFr cG9pbnQuaCB8ICAgIDEgKwogcmVjb3JkLmMgICAgIHwgICAzMyArKysrKysr KysrKysrKysrKysrKysrKysrKysrKystLS0KIDMgZmlsZXMgY2hhbmdlZCwg NDQgaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMoLSkKCi0tLSBhL2JyZWFr cG9pbnQuYworKysgYi9icmVha3BvaW50LmMKQEAgLTk2MjQsNiArOTYyNCwx OSBAQCBpbnNlcnRfc2luZ2xlX3N0ZXBfYnJlYWtwb2ludCAoc3RydWN0IGdk CiAJICAgICBwYWRkcmVzcyAoZ2RiYXJjaCwgbmV4dF9wYykpOwogfQogCisv KiBDaGVjayBpZiB0aGUgYnJlYWtwb2ludHMgdXNlZCBmb3Igc29mdHdhcmUg c2luZ2xlIHN0ZXBwaW5nCisgICB3ZXJlIGluc2VydGVkIG9yIG5vdC4gICov CisKK2ludAorc2luZ2xlX3N0ZXBfYnJlYWtwb2ludHNfaW5zZXJ0ZWQgKHZv aWQpCit7CisgIGlmIChzaW5nbGVfc3RlcF9icmVha3BvaW50c1swXSAhPSBO VUxMCisgICAgICB8fCBzaW5nbGVfc3RlcF9icmVha3BvaW50c1sxXSAhPSBO VUxMKQorICAgIHJldHVybiAxOworCisgIHJldHVybiAwOworfQorCiAvKiBS ZW1vdmUgYW5kIGRlbGV0ZSBhbnkgYnJlYWtwb2ludHMgdXNlZCBmb3Igc29m dHdhcmUgc2luZ2xlIHN0ZXAuICAqLwogCiB2b2lkCi0tLSBhL2JyZWFrcG9p bnQuaAorKysgYi9icmVha3BvaW50LmgKQEAgLTk0NCw2ICs5NDQsNyBAQCBl eHRlcm4gaW50IHJlbW92ZV9od193YXRjaHBvaW50cyAodm9pZCk7CiAgICB0 d2ljZSBiZWZvcmUgcmVtb3ZlIGlzIGNhbGxlZC4gICovCiBleHRlcm4gdm9p ZCBpbnNlcnRfc2luZ2xlX3N0ZXBfYnJlYWtwb2ludCAoc3RydWN0IGdkYmFy Y2ggKiwKIAkJCQkJICAgc3RydWN0IGFkZHJlc3Nfc3BhY2UgKiwgQ09SRV9B RERSKTsKK2V4dGVybiBpbnQgc2luZ2xlX3N0ZXBfYnJlYWtwb2ludHNfaW5z ZXJ0ZWQgKHZvaWQpOwogZXh0ZXJuIHZvaWQgcmVtb3ZlX3NpbmdsZV9zdGVw X2JyZWFrcG9pbnRzICh2b2lkKTsKIAogLyogTWFuYWdlIG1hbnVhbCBicmVh a3BvaW50cywgc2VwYXJhdGUgZnJvbSB0aGUgbm9ybWFsIGNoYWluIG9mCi0t LSBhL3JlY29yZC5jCisrKyBiL3JlY29yZC5jCkBAIC0xMDAyLDkgKzEwMDIs MjMgQEAgcmVjb3JkX3Jlc3VtZSAoc3RydWN0IHRhcmdldF9vcHMgKm9wcywg cAogCiAgIGlmICghUkVDT1JEX0lTX1JFUExBWSkKICAgICB7CisgICAgICBz dHJ1Y3QgZ2RiYXJjaCAqZ2RiYXJjaCA9IHRhcmdldF90aHJlYWRfYXJjaGl0 ZWN0dXJlIChwdGlkKTsKKwogICAgICAgcmVjb3JkX21lc3NhZ2UgKGdldF9j dXJyZW50X3JlZ2NhY2hlICgpLCBzaWduYWwpOwogICAgICAgcmVjb3JkX2Jl bmVhdGhfdG9fcmVzdW1lIChyZWNvcmRfYmVuZWF0aF90b19yZXN1bWVfb3Bz LCBwdGlkLCAxLAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBz aWduYWwpOworCisgICAgICAgaWYgKGdkYmFyY2hfc29mdHdhcmVfc2luZ2xl X3N0ZXBfcCAoZ2RiYXJjaCkpCisgICAgICAgICB7CisgICAgICAgICAgIGlm ICghaW5zZXJ0ZWRfc2luZ2xlX3N0ZXBfYnJlYWtwb2ludF9wICgpKQorICAg ICAgICAgICAgIGdkYmFyY2hfc29mdHdhcmVfc2luZ2xlX3N0ZXAgKGdkYmFy Y2gsIGdldF9jdXJyZW50X2ZyYW1lICgpKTsKKyAgICAgICAgICAgcmVjb3Jk X2JlbmVhdGhfdG9fcmVzdW1lIChyZWNvcmRfYmVuZWF0aF90b19yZXN1bWVf b3BzLAorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHB0 aWQsIHN0ZXAsIHNpZ25hbCk7CisgICAgICAgICAgIHJlY29yZF9yZXN1bWVf c3RlcCA9IDA7CisgICAgICAgICB9CisgICAgICAgZWxzZQorICAgICAgICAg cmVjb3JkX2JlbmVhdGhfdG9fcmVzdW1lIChyZWNvcmRfYmVuZWF0aF90b19y ZXN1bWVfb3BzLCBwdGlkLCAxLAorICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICBzaWduYWwpOwogICAgIH0KIH0KIApAQCAtMTA3Nyw2ICsx MDkxLDcgQEAgcmVjb3JkX3dhaXQgKHN0cnVjdCB0YXJnZXRfb3BzICpvcHMs CiAJICAvKiBUaGlzIGlzIG5vdCBhIHNpbmdsZSBzdGVwLiAgKi8KIAkgIHB0 aWRfdCByZXQ7CiAJICBDT1JFX0FERFIgdG1wX3BjOworICAgICAgICAgIHN0 cnVjdCBnZGJhcmNoICpnZGJhcmNoID0gdGFyZ2V0X3RocmVhZF9hcmNoaXRl Y3R1cmUgKGluZmVyaW9yX3B0aWQpOwogCiAJICB3aGlsZSAoMSkKIAkgICAg ewpAQCAtMTA5OSw2ICsxMTE0LDkgQEAgcmVjb3JkX3dhaXQgKHN0cnVjdCB0 YXJnZXRfb3BzICpvcHMsCiAJCSAgdG1wX3BjID0gcmVnY2FjaGVfcmVhZF9w YyAocmVnY2FjaGUpOwogCQkgIGFzcGFjZSA9IGdldF9yZWdjYWNoZV9hc3Bh Y2UgKHJlZ2NhY2hlKTsKIAorICAgICAgICAgICAgICAgICAgaWYgKGdkYmFy Y2hfc29mdHdhcmVfc2luZ2xlX3N0ZXBfcCAoZ2RiYXJjaCkpCisgICAgICAg ICAgICAgICAgICAgIHJlbW92ZV9zaW5nbGVfc3RlcF9icmVha3BvaW50cyAo KTsKKwogCQkgIGlmICh0YXJnZXRfc3RvcHBlZF9ieV93YXRjaHBvaW50ICgp KQogCQkgICAgewogCQkgICAgICAvKiBBbHdheXMgaW50ZXJlc3RlZCBpbiB3 YXRjaHBvaW50cy4gICovCkBAIC0xMTI5LDkgKzExNDcsMTggQEAgcmVjb3Jk X3dhaXQgKHN0cnVjdCB0YXJnZXRfb3BzICpvcHMsCiAgICAgICAgICAgICAg ICAgICAgICAgICAgICBicmVhazsKICAgCQkJfQogCi0JCSAgICAgIHJlY29y ZF9iZW5lYXRoX3RvX3Jlc3VtZSAocmVjb3JkX2JlbmVhdGhfdG9fcmVzdW1l X29wcywKLQkJCQkJCXB0aWQsIDEsCi0JCQkJCQlUQVJHRVRfU0lHTkFMXzAp OworICAgICAgICAgICAgICAgICAgICAgIGlmIChnZGJhcmNoX3NvZnR3YXJl X3NpbmdsZV9zdGVwX3AgKGdkYmFyY2gpKQorICAgICAgICAgICAgICAgICAg ICAgICAgeworICAgICAgICAgICAgICAgICAgICAgICAgICBnZGJhcmNoX3Nv ZnR3YXJlX3NpbmdsZV9zdGVwIChnZGJhcmNoLAorICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBnZXRf Y3VycmVudF9mcmFtZSAoKSk7CisgICAgICAgICAgICAgICAgICAgICAgICAg IHJlY29yZF9iZW5lYXRoX3RvX3Jlc3VtZSAocmVjb3JkX2JlbmVhdGhfdG9f cmVzdW1lX29wcywKKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICBwdGlkLCAwLAorICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIFRBUkdFVF9T SUdOQUxfMCk7CisgICAgICAgICAgICAgICAgICAgICAgICB9CisgICAgICAg ICAgICAgICAgICAgICAgZWxzZQorCQkgICAgICAgIHJlY29yZF9iZW5lYXRo X3RvX3Jlc3VtZSAocmVjb3JkX2JlbmVhdGhfdG9fcmVzdW1lX29wcywKKwkJ CQkJCSAgcHRpZCwgMSwKKwkJCQkJCSAgVEFSR0VUX1NJR05BTF8wKTsKIAkJ ICAgICAgY29udGludWU7CiAJCSAgICB9CiAJCX0K --001636e0a510b1f3ea047b5f92ba--