From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6550 invoked by alias); 24 May 2009 00:26:09 -0000 Received: (qmail 6538 invoked by uid 22791); 24 May 2009 00:26:07 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,SARE_MSGID_LONG40,SPF_PASS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.33.17) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 24 May 2009 00:26:00 +0000 Received: from zps19.corp.google.com (zps19.corp.google.com [172.25.146.19]) by smtp-out.google.com with ESMTP id n4O0Pt6L023137 for ; Sun, 24 May 2009 01:25:56 +0100 Received: from gxk2 (gxk2.prod.google.com [10.202.11.2]) by zps19.corp.google.com with ESMTP id n4O0PrhL030242 for ; Sat, 23 May 2009 17:25:53 -0700 Received: by gxk2 with SMTP id 2so4598154gxk.3 for ; Sat, 23 May 2009 17:25:53 -0700 (PDT) MIME-Version: 1.0 Received: by 10.90.33.15 with SMTP id g15mr4727047agg.5.1243124752868; Sat, 23 May 2009 17:25:52 -0700 (PDT) In-Reply-To: <20090507031109.0CBFB84890@localhost> References: <20090507031109.0CBFB84890@localhost> Date: Sun, 24 May 2009 00:26:00 -0000 Message-ID: Subject: Re: [RFA]: Clean up debug printing of pc in gdbserver From: Doug Evans To: gdb-patches@sourceware.org Content-Type: multipart/mixed; boundary=0016e64ec8fc4e6ebd046a9d898d X-System-Of-Record: true 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-05/txt/msg00523.txt.bz2 --0016e64ec8fc4e6ebd046a9d898d Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-length: 6080 Ping. Attached is an updated patch (now that x86 gdbserver is biarch). On Wed, May 6, 2009 at 8:11 PM, Doug Evans wrote: > Hi. > > The debug printing of pc in linux-i386-low.c/linux-x86-64-low.c can > use some clean up. > > - why restrict the printing to just x86? > - the text that is printed for linux_resume_one_lwp is confusing > > Ok to check in? > > 2009-05-06 =A0Doug Evans =A0 > > =A0 =A0 =A0 =A0* linux-x86-64-low.c (debug_threads): Remove declaration. > =A0 =A0 =A0 =A0(x86_64_get_pc,x86_64_set_pc): Remove debug printing of pc. > =A0 =A0 =A0 =A0* linux-i386-low.c (debug_threads): Remove declaration. > =A0 =A0 =A0 =A0(i386_get_pc,i386_set_pc): Remove debug printing of pc. > =A0 =A0 =A0 =A0* linux-low.c (get_stop_pc): Print pc if debug_threads. > =A0 =A0 =A0 =A0(check_removed_breakpoint, linux_wait_for_lwp): Ditto. > =A0 =A0 =A0 =A0(linux_resume_one_lwp): Ditto. > > Index: linux-low.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v > retrieving revision 1.99 > diff -u -p -r1.99 linux-low.c > --- linux-low.c 6 May 2009 17:32:59 -0000 =A0 =A0 =A0 1.99 > +++ linux-low.c 7 May 2009 02:10:55 -0000 > @@ -290,10 +290,14 @@ get_stop_pc (void) > =A0{ > =A0 CORE_ADDR stop_pc =3D (*the_low_target.get_pc) (); > > - =A0if (get_thread_lwp (current_inferior)->stepping) > - =A0 =A0return stop_pc; > - =A0else > - =A0 =A0return stop_pc - the_low_target.decr_pc_after_break; > + =A0if (! get_thread_lwp (current_inferior)->stepping) > + =A0 =A0stop_pc -=3D the_low_target.decr_pc_after_break; > + > + =A0if (debug_threads) > + =A0 =A0fprintf (stderr, "stop pc is %08lx\n", > + =A0 =A0 =A0 =A0 =A0 =A0(long) stop_pc); > + > + =A0return stop_pc; > =A0} > > =A0static void * > @@ -750,7 +754,11 @@ check_removed_breakpoint (struct lwp_inf > =A0 =A0 =A0decrement. =A0We go immediately from this function to resuming, > =A0 =A0 =A0and can not safely call get_stop_pc () again. =A0*/ > =A0 if (the_low_target.set_pc !=3D NULL) > - =A0 =A0(*the_low_target.set_pc) (stop_pc); > + =A0 =A0{ > + =A0 =A0 =A0if (debug_threads) > + =A0 =A0 =A0 fprintf (stderr, "Set pc to %08lx.\n", (long) stop_pc); > + =A0 =A0 =A0(*the_low_target.set_pc) (stop_pc); > + =A0 =A0} > > =A0 /* We consumed the pending SIGTRAP. =A0*/ > =A0 event_child->pending_is_breakpoint =3D 0; > @@ -878,14 +886,14 @@ retry: > =A0 =A0 } > > =A0 if (debug_threads > - =A0 =A0 =A0&& WIFSTOPPED (*wstatp)) > + =A0 =A0 =A0&& WIFSTOPPED (*wstatp) > + =A0 =A0 =A0&& the_low_target.get_pc !=3D NULL) > =A0 =A0 { > =A0 =A0 =A0 struct thread_info *saved_inferior =3D current_inferior; > + =A0 =A0 =A0CORE_ADDR stop_pc =3D (*the_low_target.get_pc) (); > =A0 =A0 =A0 current_inferior =3D (struct thread_info *) > =A0 =A0 =A0 =A0find_inferior_id (&all_threads, child->head.id); > - =A0 =A0 =A0/* For testing only; i386_stop_pc prints out a diagnostic. = =A0*/ > - =A0 =A0 =A0if (the_low_target.get_pc !=3D NULL) > - =A0 =A0 =A0 get_stop_pc (); > + =A0 =A0 =A0fprintf (stderr, "linux_wait_for_lwp: pc is %08lx\n", (long)= stop_pc); > =A0 =A0 =A0 current_inferior =3D saved_inferior; > =A0 =A0 } > > @@ -1644,8 +1652,8 @@ linux_resume_one_lwp (struct inferior_li > > =A0 if (debug_threads && the_low_target.get_pc !=3D NULL) > =A0 =A0 { > - =A0 =A0 =A0fprintf (stderr, " =A0"); > - =A0 =A0 =A0(*the_low_target.get_pc) (); > + =A0 =A0 =A0CORE_ADDR pc =3D (*the_low_target.get_pc) (); > + =A0 =A0 =A0fprintf (stderr, " =A0resuming from pc 0x%lx\n", (long) pc); > =A0 =A0 } > > =A0 /* If we have pending signals, consume one unless we are trying to re= insert > Index: linux-x86-64-low.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/gdbserver/linux-x86-64-low.c,v > retrieving revision 1.22 > diff -u -p -r1.22 linux-x86-64-low.c > --- linux-x86-64-low.c =A022 Mar 2009 23:57:10 -0000 =A0 =A0 =A01.22 > +++ linux-x86-64-low.c =A07 May 2009 03:03:09 -0000 > @@ -128,8 +128,6 @@ struct regset_info target_regsets[] =3D { > =A0static const unsigned char x86_64_breakpoint[] =3D { 0xCC }; > =A0#define x86_64_breakpoint_len 1 > > -extern int debug_threads; > - > =A0static CORE_ADDR > =A0x86_64_get_pc () > =A0{ > @@ -137,16 +135,12 @@ x86_64_get_pc () > > =A0 collect_register_by_name ("rip", &pc); > > - =A0if (debug_threads) > - =A0 =A0fprintf (stderr, "stop pc (before any decrement) is %08lx\n", pc= ); > =A0 return pc; > =A0} > > =A0static void > =A0x86_64_set_pc (CORE_ADDR newpc) > =A0{ > - =A0if (debug_threads) > - =A0 =A0fprintf (stderr, "set pc to %08lx\n", (long) newpc); > =A0 supply_register_by_name ("rip", &newpc); > =A0} > > Index: linux-i386-low.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/gdbserver/linux-i386-low.c,v > retrieving revision 1.19 > diff -u -p -r1.19 linux-i386-low.c > --- linux-i386-low.c =A0 =A022 Mar 2009 23:57:10 -0000 =A0 =A0 =A01.19 > +++ linux-i386-low.c =A0 =A07 May 2009 03:03:09 -0000 > @@ -154,8 +154,6 @@ struct regset_info target_regsets[] =3D { > =A0static const unsigned char i386_breakpoint[] =3D { 0xCC }; > =A0#define i386_breakpoint_len 1 > > -extern int debug_threads; > - > =A0static CORE_ADDR > =A0i386_get_pc () > =A0{ > @@ -163,16 +161,12 @@ i386_get_pc () > > =A0 collect_register_by_name ("eip", &pc); > > - =A0if (debug_threads) > - =A0 =A0fprintf (stderr, "stop pc (before any decrement) is %08lx\n", pc= ); > =A0 return pc; > =A0} > > =A0static void > =A0i386_set_pc (CORE_ADDR newpc) > =A0{ > - =A0if (debug_threads) > - =A0 =A0fprintf (stderr, "set pc to %08lx\n", (long) newpc); > =A0 supply_register_by_name ("eip", &newpc); > =A0} > > --0016e64ec8fc4e6ebd046a9d898d Content-Type: text/plain; charset=US-ASCII; name="gdb-090523-gdbserver-get-pc-2.patch.txt" Content-Disposition: attachment; filename="gdb-090523-gdbserver-get-pc-2.patch.txt" Content-Transfer-Encoding: base64 X-Attachment-Id: f_fv30cs0f0 Content-length: 3392 MjAwOS0wNS0wNiAgRG91ZyBFdmFucyAgPGRqZUBnb29nbGUuY29tPgoKCSog bGludXgtbG93LmMgKGdldF9zdG9wX3BjKTogUHJpbnQgcGMgaWYgZGVidWdf dGhyZWFkcy4KCShjaGVja19yZW1vdmVkX2JyZWFrcG9pbnQsIGxpbnV4X3dh aXRfZm9yX2x3cCk6IERpdHRvLgoJKGxpbnV4X3Jlc3VtZV9vbmVfbHdwKTog RGl0dG8uCgpJbmRleDogbGludXgtbG93LmMKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQpSQ1MgZmlsZTogL2N2cy9zcmMvc3JjL2dkYi9nZGJzZXJ2ZXIvbGlu dXgtbG93LmMsdgpyZXRyaWV2aW5nIHJldmlzaW9uIDEuMTAyCmRpZmYgLXUg LXAgLXIxLjEwMiBsaW51eC1sb3cuYwotLS0gbGludXgtbG93LmMJMTMgTWF5 IDIwMDkgMTc6MTc6MjIgLTAwMDAJMS4xMDIKKysrIGxpbnV4LWxvdy5jCTI0 IE1heSAyMDA5IDAwOjIxOjU4IC0wMDAwCkBAIC0zNTUsMTAgKzM1NSwxMyBA QCBnZXRfc3RvcF9wYyAodm9pZCkKIHsKICAgQ09SRV9BRERSIHN0b3BfcGMg PSAoKnRoZV9sb3dfdGFyZ2V0LmdldF9wYykgKCk7CiAKLSAgaWYgKGdldF90 aHJlYWRfbHdwIChjdXJyZW50X2luZmVyaW9yKS0+c3RlcHBpbmcpCi0gICAg cmV0dXJuIHN0b3BfcGM7Ci0gIGVsc2UKLSAgICByZXR1cm4gc3RvcF9wYyAt IHRoZV9sb3dfdGFyZ2V0LmRlY3JfcGNfYWZ0ZXJfYnJlYWs7CisgIGlmICgh IGdldF90aHJlYWRfbHdwIChjdXJyZW50X2luZmVyaW9yKS0+c3RlcHBpbmcp CisgICAgc3RvcF9wYyAtPSB0aGVfbG93X3RhcmdldC5kZWNyX3BjX2FmdGVy X2JyZWFrOworCisgIGlmIChkZWJ1Z190aHJlYWRzKQorICAgIGZwcmludGYg KHN0ZGVyciwgInN0b3AgcGMgaXMgMHglbHhcbiIsIChsb25nKSBzdG9wX3Bj KTsKKworICByZXR1cm4gc3RvcF9wYzsKIH0KIAogc3RhdGljIHZvaWQgKgpA QCAtODE1LDcgKzgxOCwxMSBAQCBjaGVja19yZW1vdmVkX2JyZWFrcG9pbnQg KHN0cnVjdCBsd3BfaW5mCiAgICAgIGRlY3JlbWVudC4gIFdlIGdvIGltbWVk aWF0ZWx5IGZyb20gdGhpcyBmdW5jdGlvbiB0byByZXN1bWluZywKICAgICAg YW5kIGNhbiBub3Qgc2FmZWx5IGNhbGwgZ2V0X3N0b3BfcGMgKCkgYWdhaW4u ICAqLwogICBpZiAodGhlX2xvd190YXJnZXQuc2V0X3BjICE9IE5VTEwpCi0g ICAgKCp0aGVfbG93X3RhcmdldC5zZXRfcGMpIChzdG9wX3BjKTsKKyAgICB7 CisgICAgICBpZiAoZGVidWdfdGhyZWFkcykKKwlmcHJpbnRmIChzdGRlcnIs ICJTZXQgcGMgdG8gMHglbHguXG4iLCAobG9uZykgc3RvcF9wYyk7CisgICAg ICAoKnRoZV9sb3dfdGFyZ2V0LnNldF9wYykgKHN0b3BfcGMpOworICAgIH0K IAogICAvKiBXZSBjb25zdW1lZCB0aGUgcGVuZGluZyBTSUdUUkFQLiAgKi8K ICAgZXZlbnRfY2hpbGQtPnBlbmRpbmdfaXNfYnJlYWtwb2ludCA9IDA7CkBA IC05NDMsMTQgKzk1MCwxNiBAQCByZXRyeToKICAgICB9CiAKICAgaWYgKGRl YnVnX3RocmVhZHMKLSAgICAgICYmIFdJRlNUT1BQRUQgKCp3c3RhdHApKQor ICAgICAgJiYgV0lGU1RPUFBFRCAoKndzdGF0cCkKKyAgICAgICYmIHRoZV9s b3dfdGFyZ2V0LmdldF9wYyAhPSBOVUxMKQogICAgIHsKICAgICAgIHN0cnVj dCB0aHJlYWRfaW5mbyAqc2F2ZWRfaW5mZXJpb3IgPSBjdXJyZW50X2luZmVy aW9yOworICAgICAgQ09SRV9BRERSIHN0b3BfcGM7CisKICAgICAgIGN1cnJl bnRfaW5mZXJpb3IgPSAoc3RydWN0IHRocmVhZF9pbmZvICopCiAJZmluZF9p bmZlcmlvcl9pZCAoJmFsbF90aHJlYWRzLCBjaGlsZC0+aGVhZC5pZCk7Ci0g ICAgICAvKiBGb3IgdGVzdGluZyBvbmx5OyBpMzg2X3N0b3BfcGMgcHJpbnRz IG91dCBhIGRpYWdub3N0aWMuICAqLwotICAgICAgaWYgKHRoZV9sb3dfdGFy Z2V0LmdldF9wYyAhPSBOVUxMKQotCWdldF9zdG9wX3BjICgpOworICAgICAg c3RvcF9wYyA9ICgqdGhlX2xvd190YXJnZXQuZ2V0X3BjKSAoKTsKKyAgICAg IGZwcmludGYgKHN0ZGVyciwgImxpbnV4X3dhaXRfZm9yX2x3cDogcGMgaXMg MHglbHhcbiIsIChsb25nKSBzdG9wX3BjKTsKICAgICAgIGN1cnJlbnRfaW5m ZXJpb3IgPSBzYXZlZF9pbmZlcmlvcjsKICAgICB9CiAKQEAgLTE3MDksOCAr MTcxOCw4IEBAIGxpbnV4X3Jlc3VtZV9vbmVfbHdwIChzdHJ1Y3QgaW5mZXJp b3JfbGkKIAogICBpZiAoZGVidWdfdGhyZWFkcyAmJiB0aGVfbG93X3Rhcmdl dC5nZXRfcGMgIT0gTlVMTCkKICAgICB7Ci0gICAgICBmcHJpbnRmIChzdGRl cnIsICIgICIpOwotICAgICAgKCp0aGVfbG93X3RhcmdldC5nZXRfcGMpICgp OworICAgICAgQ09SRV9BRERSIHBjID0gKCp0aGVfbG93X3RhcmdldC5nZXRf cGMpICgpOworICAgICAgZnByaW50ZiAoc3RkZXJyLCAiICByZXN1bWluZyBm cm9tIHBjIDB4JWx4XG4iLCAobG9uZykgcGMpOwogICAgIH0KIAogICAvKiBJ ZiB3ZSBoYXZlIHBlbmRpbmcgc2lnbmFscywgY29uc3VtZSBvbmUgdW5sZXNz IHdlIGFyZSB0cnlpbmcgdG8gcmVpbnNlcnQK --0016e64ec8fc4e6ebd046a9d898d--