From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31031 invoked by alias); 24 Nov 2009 01:58:25 -0000 Received: (qmail 30885 invoked by uid 22791); 24 Nov 2009 01:58:24 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 24 Nov 2009 01:58:19 +0000 Received: (qmail 17581 invoked from network); 24 Nov 2009 01:58:17 -0000 Received: from unknown (HELO orlando) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 Nov 2009 01:58:17 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFA] Fix hw watchpoints in process record. Date: Tue, 24 Nov 2009 01:58:00 -0000 User-Agent: KMail/1.9.10 Cc: Hui Zhu , Michael Snyder References: <4AECE12F.3000704@vmware.com> <200911221255.25826.pedro@codesourcery.com> <200911221544.30010.pedro@codesourcery.com> In-Reply-To: <200911221544.30010.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Message-Id: <200911240158.17986.pedro@codesourcery.com> 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/msg00519.txt.bz2 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_watchpoi= nt) > =A0=A0=A0=A0=A0=A0=A0=A0(record_beneath_to_stopped_data_address, record_h= w_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... I 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_watchpoint; > =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_breakp= oint; > =A0 =A0record_core_ops.to_remove_breakpoint =3D record_core_remove_breakp= oint; > + =A0record_core_ops.to_stopped_by_watchpoint =3D record_stopped_by_watch= point; > =A0 =A0record_core_ops.to_can_execute_reverse =3D record_can_execute_reve= rse; > =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. That is the case of stopping recording when stopped at a watchpoint, and then continue/step backwards until a different watchpoint triggers. The 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. E.g., on x86_64-linux : >./gdb -q ./gdb (top-gdb) start Temporary breakpoint 3 at 0x454727: file ../../src/gdb/gdb.c, line 28. Starting program: /home/pedro/gdb/baseline/build/gdb/gdb [Thread debugging using libthread_db enabled] =20 Temporary breakpoint 3, main (argc=3D1, argv=3D0x7fffffffe3a8) at ../../sr= c/gdb/gdb.c:28 28 memset (&args, 0, sizeof args); (top-gdb) record (top-gdb) watch args.argv Hardware watchpoint 4: args.argv (top-gdb) c Continuing. Hardware watchpoint 4: args.argv =20 Old value =3D (char **) 0x0 New value =3D (char **) 0x7fffffffe3a8 main (argc=3D1, argv=3D0x7fffffffe3a8) at ../../src/gdb/gdb.c:31 31 args.use_windows =3D 0; (top-gdb) del Delete all breakpoints? (y or n) y (top-gdb) watch args.argc Hardware watchpoint 5: args.argc (top-gdb) reverse-continue Continuing. No more reverse-execution history. main (argc=3D1, argv=3D0x7fffffffe3a8) at ../../src/gdb/gdb.c:28 28 memset (&args, 0, sizeof args); (top-gdb)=20=20 It should have triggered a watchpoint. With the patch applied, the last reverse-continue does this instead: (top-gdb) reverse-continue Continuing. Hardware watchpoint 5: args.argc Old value =3D 1 New value =3D 0 0x000000000045473d in main (argc=3D1, argv=3D0x7fffffffe3a8) at ../../src/= gdb/gdb.c:29 29 args.argc =3D argc; (top-gdb)=20 which is correct. This deserves a testcase, but I haven't written it yet. Will do (unless someone else wants to, which I'd appreciate :-) ). --=20 Pedro Alves 2009-11-24 Pedro Alves * record.c (init_record_ops, init_record_core_ops): Actually install record_stopped_data_address. --- gdb/record.c | 2 ++ 1 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 2009-11-24 01:36:42.000000000 +0000 +++ src/gdb/record.c 2009-11-24 01:37:01.000000000 +0000 @@ -1668,6 +1668,7 @@ init_record_ops (void) record_ops.to_insert_breakpoint =3D record_insert_breakpoint; record_ops.to_remove_breakpoint =3D record_remove_breakpoint; record_ops.to_stopped_by_watchpoint =3D record_stopped_by_watchpoint; + record_ops.to_stopped_data_address =3D record_stopped_data_address; record_ops.to_can_execute_reverse =3D record_can_execute_reverse; record_ops.to_stratum =3D record_stratum; /* Add bookmark target methods. */ @@ -1876,6 +1877,7 @@ init_record_core_ops (void) record_core_ops.to_insert_breakpoint =3D record_core_insert_breakpoint; record_core_ops.to_remove_breakpoint =3D record_core_remove_breakpoint; record_core_ops.to_stopped_by_watchpoint =3D record_stopped_by_watchpoin= t; + record_core_ops.to_stopped_data_address =3D record_stopped_data_address; record_core_ops.to_can_execute_reverse =3D record_can_execute_reverse; record_core_ops.to_has_execution =3D record_core_has_execution; record_core_ops.to_stratum =3D record_stratum;