From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4941 invoked by alias); 28 Sep 2009 09:27:13 -0000 Received: (qmail 4929 invoked by uid 22791); 28 Sep 2009 09:27:11 -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-px0-f202.google.com (HELO mail-px0-f202.google.com) (209.85.216.202) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 28 Sep 2009 09:27:06 +0000 Received: by pxi40 with SMTP id 40so5244086pxi.24 for ; Mon, 28 Sep 2009 02:27:05 -0700 (PDT) MIME-Version: 1.0 Received: by 10.142.7.39 with SMTP id 39mr232098wfg.113.1254130025254; Mon, 28 Sep 2009 02:27:05 -0700 (PDT) In-Reply-To: <4ABE5E8D.8080209@vmware.com> References: <4AA5D713.1060305@vmware.com> <20090908065843.GO30677@adacore.com> <4AA68C92.7070905@vmware.com> <4ABE5E8D.8080209@vmware.com> From: Hui Zhu Date: Mon, 28 Sep 2009 09:27:00 -0000 Message-ID: Subject: Re: [RFA] let record_resume fail immediately on error To: Michael Snyder , Joel Brobecker Cc: "gdb-patches@sourceware.org" Content-Type: multipart/mixed; boundary=00504502ae47a84e7a04749fe60f 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-09/txt/msg00863.txt.bz2 --00504502ae47a84e7a04749fe60f Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-length: 11130 Hi Joel and Michael, I think we handler record really not very well. For example: cat 1.c #include #include #include #include #include #include #include #include #include int main(int argc,char *argv[],char *envp[]) { asm ("rdtsc"); return (0); } Without the fix error patch: we will get: gdb ./a.out (gdb) start During symbol reading, DW_AT_name missing from DW_TAG_base_type. Temporary breakpoint 1 at 0x8048352: file 4.c, line 14. Starting program: /home/teawater/gdb/bgdbno/gdb/a.out Temporary breakpoint 1, main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); (gdb) record (gdb) c Continuing. Process record doesn't support instruction rdtsc. Process record doesn't support instruction 0xf31 at address 0x8048352. Process record: failed to record execution log. Program received signal SIGABRT, Aborted. main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); (gdb) c Continuing. Program terminated with signal SIGABRT, Aborted. The program no longer exists. (gdb) record stop Process record is not started. The inferior is killed by gdb, user cannot close the record and keep debug the inferior. With the fix error patch: (gdb) start During symbol reading, DW_AT_name missing from DW_TAG_base_type. Temporary breakpoint 1 at 0x8048352: file 4.c, line 14. Starting program: /home/teawater/gdb/bgdbno/gdb/a.out Temporary breakpoint 1, main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); (gdb) record (gdb) c Continuing. Process record doesn't support instruction rdtsc. Process record doesn't support instruction 0xf31 at address 0x8048352. main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); Process record: failed to record execution log. (gdb) c Continuing. Process record doesn't support instruction rdtsc. Process record doesn't support instruction 0xf31 at address 0x8048352. main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); Process record: failed to record execution log. (gdb) Continuing. Process record doesn't support instruction rdtsc. Process record doesn't support instruction 0xf31 at address 0x8048352. main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); Process record: failed to record execution log. (gdb) Continuing. Process record doesn't support instruction rdtsc. Process record doesn't support instruction 0xf31 at address 0x8048352. main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); Process record: failed to record execution log. (gdb) Continuing. Process record doesn't support instruction rdtsc. Process record doesn't support instruction 0xf31 at address 0x8048352. main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); Process record: failed to record execution log. (gdb) Continuing. Process record doesn't support instruction rdtsc. Process record doesn't support instruction 0xf31 at address 0x8048352. main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); Process record: failed to record execution log. (gdb) Continuing. Process record doesn't support instruction rdtsc. Process record doesn't support instruction 0xf31 at address 0x8048352. main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); Process record: failed to record execution log. (gdb) Continuing. Process record doesn't support instruction rdtsc. Process record doesn't support instruction 0xf31 at address 0x8048352. main (argc=3D, argv=3D, envp=3D) at 4.c:14 14 asm ("rdtsc"); Process record: failed to record execution log. (gdb) record stop Process record is stoped and all execution log is deleted. (gdb) c Continuing. User can keep debug the inferior after close the record. So maybe we need this patch in 7.0. What do you think about it? Thanks, Hui BTW, I make a new patch follow the gdb update: 2009-09-28 Hui Zhu * record.c (record_resume_error): Deleted. (record_resume): Call record_message. (record_wait): Deleted record_resume_error. --- record.c | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) --- a/record.c +++ b/record.c @@ -566,7 +566,6 @@ record_close (int quitting) } static int record_resume_step =3D 0; -static int record_resume_error; static void record_resume (struct target_ops *ops, ptid_t ptid, int step, @@ -576,15 +575,11 @@ record_resume (struct target_ops *ops, p if (!RECORD_IS_REPLAY) { - if (do_record_message (get_current_regcache (), signal)) - { - record_resume_error =3D 0; - } - else - { - record_resume_error =3D 1; - return; - } + struct record_message_args args; + + args.regcache =3D get_current_regcache (); + args.signal =3D signal; + record_message (&args); record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1, signal); } @@ -636,14 +631,6 @@ record_wait (struct target_ops *ops, if (!RECORD_IS_REPLAY) { - if (record_resume_error) - { - /* If record_resume get error, return directly. */ - status->kind =3D TARGET_WAITKIND_STOPPED; - status->value.sig =3D TARGET_SIGNAL_ABRT; - return inferior_ptid; - } - if (record_resume_step) { /* This is a single step. */ On Sun, Sep 27, 2009 at 02:33, Michael Snyder wrote: > Personally, I think this one isn't critical for 7.0. > Waiting for 7.1 will allow us more time to assess it. > > Hui Zhu wrote: >> >> Hi Joel, >> >> Sorry to disturb you. =A0Ping >> http://sourceware.org/ml/gdb-patches/2009-09/msg00231.html >> >> Thanks, >> Hui >> >> On Wed, Sep 9, 2009 at 10:05, Hui Zhu wrote: >>> >>> On Wed, Sep 9, 2009 at 00:55, Michael Snyder wrote: >>>> >>>> Hui Zhu wrote: >>>>> >>>>> On Tue, Sep 8, 2009 at 15:25, Hui Zhu wrote: >>>>>> >>>>>> If GDB call error in record_resume, user cannot keep debug the >>>>>> inferior. >>>>>> >>>>>> Hui >>>>>> >>>>>> On Tue, Sep 8, 2009 at 15:23, Hui Zhu wrote: >>>>>>> >>>>>>> The "record_resume_error" in gdb-cvs is to make user after get a >>>>>>> error >>>>>>> of record_message, they can "record stop" close the record and keep >>>>>>> debug the inferior. >>>>>>> >>>>>>> Thanks, >>>>>>> Hui >>>>>>> >>>>>>> On Tue, Sep 8, 2009 at 14:58, Joel Brobecker >>>>>>> wrote: >>>>>>>>> >>>>>>>>> =A0if (!RECORD_IS_REPLAY) >>>>>>>>> =A0 =A0{ >>>>>>>>> =A0 =A0 =A0if (do_record_message (get_current_regcache ())) >>>>>>>>> - =A0 =A0 =A0 =A0{ >>>>>>>>> - =A0 =A0 =A0 =A0 =A0record_resume_error =3D 0; >>>>>>>>> - =A0 =A0 =A0 =A0} >>>>>>>>> - =A0 =A0 =A0else >>>>>>>>> - =A0 =A0 =A0 =A0{ >>>>>>>>> - =A0 =A0 =A0 =A0 =A0record_resume_error =3D 1; >>>>>>>>> - =A0 =A0 =A0 =A0 =A0return; >>>>>>>>> - =A0 =A0 =A0 =A0} >>>>>>>>> + =A0 =A0 internal_error (__FILE__, __LINE__, >>>>>>>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 _("record_resume: do_re= cord_message >>>>>>>>> failed.")); >>>>>>>>> + >>>>>>>> >>>>>>>> Forgive me if I'm wrong, as I don't know the record.c code at all, >>>>>>>> but >>>>>>>> I cannot help but think that the internal_error is suspicious here. >>>>>>>> Why is this an internal_error? >>>>>>>> >>>>>>>> -- >>>>>>>> Joel >>>>>>>> >>>>> Hi guys, >>>>> >>>>> I make a patch that make "record_resume_error" work better. =A0I did >>>>> some test. =A0It seems better than before. >>>> >>>> Could you explain what you mean by better? >>>> I mean, what behavior are you looking for here? >>>> >>>> Here is the behavior that I see --- I am making a recording, >>>> I say "continue", and after a while this "record_resume_error" >>>> is triggered, and gdb stops and says "No more reverse-execution >>>> history." >>>> >>>> I would never expect to see that message during recording. >>> >>> In before, GDB cannot throw error in record_resume (It just in my >>> memory, maybe I am wrong). >>> If we try to do it, it will get: >>> (gdb) c >>> Continuing. >>> Cannot execute this command while the selected thread is running. >>> So I add record_resume_error to let record_wait to handle it. >>> >>> But record_wait cannot call error too. =A0So I let it: >>> =A0 =A0 =A0 =A0 /* If record_resume get error, return directly. =A0*/ >>> =A0 =A0 =A0 =A0 status->kind =3D TARGET_WAITKIND_STOPPED; >>> =A0 =A0 =A0 =A0 status->value.sig =3D TARGET_SIGNAL_0; >>> =A0 =A0 =A0 =A0 return inferior_ptid; >>> >>> But I found that record_resume can call error now. =A0So I make a new >>> patch for it. >>> >>> Could you help me try it? >>> >>> >>> Thanks, >>> Hui >>> >>> 2009-09-09 =A0Hui Zhu =A0 >>> >>> =A0 =A0 =A0 * record.c (record_resume_error): Deleted. >>> =A0 =A0 =A0 (record_resume): Call record_message. >>> =A0 =A0 =A0 (record_wait): Deleted record_resume_error. >>> >>> --- >>> =A0record.c | =A0 19 +------------------ >>> =A01 file changed, 1 insertion(+), 18 deletions(-) >>> >>> --- a/record.c >>> +++ b/record.c >>> @@ -516,7 +516,6 @@ record_close (int quitting) >>> =A0} >>> >>> =A0static int record_resume_step =3D 0; >>> -static int record_resume_error; >>> >>> =A0static void >>> =A0record_resume (struct target_ops *ops, ptid_t ptid, int step, >>> @@ -526,15 +525,7 @@ record_resume (struct target_ops *ops, p >>> >>> =A0if (!RECORD_IS_REPLAY) >>> =A0 =A0{ >>> - =A0 =A0 =A0if (do_record_message (get_current_regcache ())) >>> - =A0 =A0 =A0 =A0{ >>> - =A0 =A0 =A0 =A0 =A0record_resume_error =3D 0; >>> - =A0 =A0 =A0 =A0} >>> - =A0 =A0 =A0else >>> - =A0 =A0 =A0 =A0{ >>> - =A0 =A0 =A0 =A0 =A0record_resume_error =3D 1; >>> - =A0 =A0 =A0 =A0 =A0return; >>> - =A0 =A0 =A0 =A0} >>> + =A0 =A0 =A0record_message (get_current_regcache ()); >>> =A0 =A0 =A0record_beneath_to_resume (record_beneath_to_resume_ops, ptid= , 1, >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0siggnal); >>> =A0 =A0} >>> @@ -586,14 +577,6 @@ record_wait (struct target_ops *ops, >>> >>> =A0if (!RECORD_IS_REPLAY) >>> =A0 =A0{ >>> - =A0 =A0 =A0if (record_resume_error) >>> - =A0 =A0 =A0 { >>> - =A0 =A0 =A0 =A0 /* If record_resume get error, return directly. =A0*/ >>> - =A0 =A0 =A0 =A0 status->kind =3D TARGET_WAITKIND_STOPPED; >>> - =A0 =A0 =A0 =A0 status->value.sig =3D TARGET_SIGNAL_ABRT; >>> - =A0 =A0 =A0 =A0 return inferior_ptid; >>> - =A0 =A0 =A0 } >>> - >>> =A0 =A0 =A0if (record_resume_step) >>> =A0 =A0 =A0 { >>> =A0 =A0 =A0 =A0 /* This is a single step. =A0*/ >>> > > --00504502ae47a84e7a04749fe60f Content-Type: text/plain; charset=US-ASCII; name="prec-fix-error-handler.txt" Content-Disposition: attachment; filename="prec-fix-error-handler.txt" Content-Transfer-Encoding: base64 X-Attachment-Id: f_g050sqsk0 Content-length: 1737 LS0tCiByZWNvcmQuYyB8ICAgMjMgKysrKystLS0tLS0tLS0tLS0tLS0tLS0K IDEgZmlsZSBjaGFuZ2VkLCA1IGluc2VydGlvbnMoKyksIDE4IGRlbGV0aW9u cygtKQoKLS0tIGEvcmVjb3JkLmMKKysrIGIvcmVjb3JkLmMKQEAgLTU2Niw3 ICs1NjYsNiBAQCByZWNvcmRfY2xvc2UgKGludCBxdWl0dGluZykKIH0KIAog c3RhdGljIGludCByZWNvcmRfcmVzdW1lX3N0ZXAgPSAwOwotc3RhdGljIGlu dCByZWNvcmRfcmVzdW1lX2Vycm9yOwogCiBzdGF0aWMgdm9pZAogcmVjb3Jk X3Jlc3VtZSAoc3RydWN0IHRhcmdldF9vcHMgKm9wcywgcHRpZF90IHB0aWQs IGludCBzdGVwLApAQCAtNTc2LDE1ICs1NzUsMTEgQEAgcmVjb3JkX3Jlc3Vt ZSAoc3RydWN0IHRhcmdldF9vcHMgKm9wcywgcAogCiAgIGlmICghUkVDT1JE X0lTX1JFUExBWSkKICAgICB7Ci0gICAgICBpZiAoZG9fcmVjb3JkX21lc3Nh Z2UgKGdldF9jdXJyZW50X3JlZ2NhY2hlICgpLCBzaWduYWwpKQotICAgICAg ICB7Ci0gICAgICAgICAgcmVjb3JkX3Jlc3VtZV9lcnJvciA9IDA7Ci0gICAg ICAgIH0KLSAgICAgIGVsc2UKLSAgICAgICAgewotICAgICAgICAgIHJlY29y ZF9yZXN1bWVfZXJyb3IgPSAxOwotICAgICAgICAgIHJldHVybjsKLSAgICAg ICAgfQorICAgICAgc3RydWN0IHJlY29yZF9tZXNzYWdlX2FyZ3MgYXJnczsK KworICAgICAgYXJncy5yZWdjYWNoZSA9IGdldF9jdXJyZW50X3JlZ2NhY2hl ICgpOworICAgICAgYXJncy5zaWduYWwgPSBzaWduYWw7CisgICAgICByZWNv cmRfbWVzc2FnZSAoJmFyZ3MpOwogICAgICAgcmVjb3JkX2JlbmVhdGhfdG9f cmVzdW1lIChyZWNvcmRfYmVuZWF0aF90b19yZXN1bWVfb3BzLCBwdGlkLCAx LAogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzaWduYWwpOwog ICAgIH0KQEAgLTYzNiwxNCArNjMxLDYgQEAgcmVjb3JkX3dhaXQgKHN0cnVj dCB0YXJnZXRfb3BzICpvcHMsCiAKICAgaWYgKCFSRUNPUkRfSVNfUkVQTEFZ KQogICAgIHsKLSAgICAgIGlmIChyZWNvcmRfcmVzdW1lX2Vycm9yKQotCXsK LQkgIC8qIElmIHJlY29yZF9yZXN1bWUgZ2V0IGVycm9yLCByZXR1cm4gZGly ZWN0bHkuICAqLwotCSAgc3RhdHVzLT5raW5kID0gVEFSR0VUX1dBSVRLSU5E X1NUT1BQRUQ7Ci0JICBzdGF0dXMtPnZhbHVlLnNpZyA9IFRBUkdFVF9TSUdO QUxfQUJSVDsKLQkgIHJldHVybiBpbmZlcmlvcl9wdGlkOwotCX0KLQogICAg ICAgaWYgKHJlY29yZF9yZXN1bWVfc3RlcCkKIAl7CiAJICAvKiBUaGlzIGlz IGEgc2luZ2xlIHN0ZXAuICAqLwo= --00504502ae47a84e7a04749fe60f--