From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16221 invoked by alias); 26 Nov 2009 02:28:23 -0000 Received: (qmail 16213 invoked by uid 22791); 26 Nov 2009 02:28:22 -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; Thu, 26 Nov 2009 02:28:17 +0000 Received: by pwj21 with SMTP id 21so236047pwj.8 for ; Wed, 25 Nov 2009 18:28:16 -0800 (PST) MIME-Version: 1.0 Received: by 10.142.55.11 with SMTP id d11mr942536wfa.17.1259202496098; Wed, 25 Nov 2009 18:28:16 -0800 (PST) In-Reply-To: <200911240158.17986.pedro@codesourcery.com> References: <4AECE12F.3000704@vmware.com> <200911221255.25826.pedro@codesourcery.com> <200911221544.30010.pedro@codesourcery.com> <200911240158.17986.pedro@codesourcery.com> From: Hui Zhu Date: Thu, 26 Nov 2009 02:28:00 -0000 Message-ID: Subject: Re: [RFA] Fix hw watchpoints in process record. To: Pedro Alves Cc: gdb-patches@sourceware.org, Michael Snyder Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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-11/txt/msg00565.txt.bz2 Thanks. Hui On Tue, Nov 24, 2009 at 09:58, Pedro Alves wrote: > On Sunday 22 November 2009 15:44:29, Pedro Alves wrote: >> We were missing a target_stopped_data_address method in precord > ... > >> =A0=A0=A0=A0=A0=A0=A0=A0* record.c (record_beneath_to_stopped_by_watchpo= int) >> =A0=A0=A0=A0=A0=A0=A0=A0(record_beneath_to_stopped_data_address, record_= hw_watchpoint): >> =A0=A0=A0=A0=A0=A0=A0=A0New globals. > ... >> =A0=A0=A0=A0=A0=A0=A0=A0(record_stopped_by_watchpoint): New. >> =A0=A0=A0=A0=A0=A0=A0=A0(record_stopped_data_address): New. >> =A0=A0=A0=A0=A0=A0=A0=A0(init_record_ops): Install them. >> =A0=A0=A0=A0=A0=A0=A0=A0(init_record_core_ops): Ditto. > > "Them", yeah right... =A0I actually managed to forget to > install record_stopped_data_address in the version I committed. > >> @@ -1594,6 +1657,7 @@ init_record_ops (void) >> =A0 =A0record_ops.to_xfer_partial =3D record_xfer_partial; >> =A0 =A0record_ops.to_insert_breakpoint =3D record_insert_breakpoint; >> =A0 =A0record_ops.to_remove_breakpoint =3D record_remove_breakpoint; >> + =A0record_ops.to_stopped_by_watchpoint =3D record_stopped_by_watchpoin= t; >> =A0 =A0record_ops.to_can_execute_reverse =3D record_can_execute_reverse; >> =A0 =A0record_ops.to_stratum =3D record_stratum; >> =A0 =A0/* Add bookmark target methods. =A0*/ >> @@ -1801,6 +1865,7 @@ init_record_core_ops (void) >> =A0 =A0record_core_ops.to_xfer_partial =3D record_core_xfer_partial; >> =A0 =A0record_core_ops.to_insert_breakpoint =3D record_core_insert_break= point; >> =A0 =A0record_core_ops.to_remove_breakpoint =3D record_core_remove_break= point; >> + =A0record_core_ops.to_stopped_by_watchpoint =3D record_stopped_by_watc= hpoint; >> =A0 =A0record_core_ops.to_can_execute_reverse =3D record_can_execute_rev= erse; >> =A0 =A0record_core_ops.to_has_execution =3D record_core_has_execution; >> =A0 =A0record_core_ops.to_stratum =3D record_stratum; > > ... > > I've applied the patch below to fix it. > > > Without this, if the target beneath supports reporting the > stopped data address, there's a case where record can miss > a watchpoint. =A0That is the case of stopping recording when > stopped at a watchpoint, and then continue/step backwards > until a different watchpoint triggers. =A0The stopp_data_address > of the target beneath is called by mistake, and that may > reports the wrong stopped_data_address, from the last time > the target really ran. =A0E.g., on x86_64-linux : > > =A0>./gdb -q ./gdb > =A0(top-gdb) start > =A0Temporary breakpoint 3 at 0x454727: file ../../src/gdb/gdb.c, line 28. > =A0Starting program: /home/pedro/gdb/baseline/build/gdb/gdb > =A0[Thread debugging using libthread_db enabled] > > =A0Temporary breakpoint 3, main (argc=3D1, argv=3D0x7fffffffe3a8) at ../.= ./src/gdb/gdb.c:28 > =A028 =A0 =A0 =A0 =A0memset (&args, 0, sizeof args); > =A0(top-gdb) record > =A0(top-gdb) watch args.argv > =A0Hardware watchpoint 4: args.argv > =A0(top-gdb) c > =A0Continuing. > =A0Hardware watchpoint 4: args.argv > > =A0Old value =3D (char **) 0x0 > =A0New value =3D (char **) 0x7fffffffe3a8 > =A0main (argc=3D1, argv=3D0x7fffffffe3a8) at ../../src/gdb/gdb.c:31 > =A031 =A0 =A0 =A0 =A0args.use_windows =3D 0; > =A0(top-gdb) del > =A0Delete all breakpoints? (y or n) y > =A0(top-gdb) watch args.argc > =A0Hardware watchpoint 5: args.argc > =A0(top-gdb) reverse-continue > =A0Continuing. > > =A0No more reverse-execution history. > =A0main (argc=3D1, argv=3D0x7fffffffe3a8) at ../../src/gdb/gdb.c:28 > =A028 =A0 =A0 =A0 =A0memset (&args, 0, sizeof args); > =A0(top-gdb) > > It should have triggered a watchpoint. =A0With the > patch applied, the last reverse-continue does this instead: > > =A0(top-gdb) reverse-continue > =A0Continuing. > =A0Hardware watchpoint 5: args.argc > > =A0Old value =3D 1 > =A0New value =3D 0 > =A00x000000000045473d in main (argc=3D1, argv=3D0x7fffffffe3a8) at ../../= src/gdb/gdb.c:29 > =A029 =A0 =A0 =A0 =A0args.argc =3D argc; > =A0(top-gdb) > > which is correct. > > > This deserves a testcase, but I haven't written it yet. =A0Will do > (unless someone else wants to, which I'd appreciate :-) ). > > -- > Pedro Alves > > 2009-11-24 =A0Pedro Alves =A0 > > =A0 =A0 =A0 =A0* record.c (init_record_ops, init_record_core_ops): Actual= ly > =A0 =A0 =A0 =A0install record_stopped_data_address. > > --- > =A0gdb/record.c | =A0 =A02 ++ > =A01 file changed, 2 insertions(+) > > Index: src/gdb/record.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 > --- src.orig/gdb/record.c =A0 =A0 =A0 2009-11-24 01:36:42.000000000 +0000 > +++ src/gdb/record.c =A0 =A02009-11-24 01:37:01.000000000 +0000 > @@ -1668,6 +1668,7 @@ init_record_ops (void) > =A0 record_ops.to_insert_breakpoint =3D record_insert_breakpoint; > =A0 record_ops.to_remove_breakpoint =3D record_remove_breakpoint; > =A0 record_ops.to_stopped_by_watchpoint =3D record_stopped_by_watchpoint; > + =A0record_ops.to_stopped_data_address =3D record_stopped_data_address; > =A0 record_ops.to_can_execute_reverse =3D record_can_execute_reverse; > =A0 record_ops.to_stratum =3D record_stratum; > =A0 /* Add bookmark target methods. =A0*/ > @@ -1876,6 +1877,7 @@ init_record_core_ops (void) > =A0 record_core_ops.to_insert_breakpoint =3D record_core_insert_breakpoin= t; > =A0 record_core_ops.to_remove_breakpoint =3D record_core_remove_breakpoin= t; > =A0 record_core_ops.to_stopped_by_watchpoint =3D record_stopped_by_watchp= oint; > + =A0record_core_ops.to_stopped_data_address =3D record_stopped_data_addr= ess; > =A0 record_core_ops.to_can_execute_reverse =3D record_can_execute_reverse; > =A0 record_core_ops.to_has_execution =3D record_core_has_execution; > =A0 record_core_ops.to_stratum =3D record_stratum; >