From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16819 invoked by alias); 23 Apr 2012 06:49:10 -0000 Received: (qmail 16808 invoked by uid 22791); 23 Apr 2012 06:49:07 -0000 X-SWARE-Spam-Status: No, hits=-4.1 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 23 Apr 2012 06:48:54 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1SMD4u-0003qk-Mj from Yao_Qi@mentor.com ; Sun, 22 Apr 2012 23:48:52 -0700 Received: from SVR-ORW-FEM-04.mgc.mentorg.com ([147.34.97.41]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Sun, 22 Apr 2012 23:47:55 -0700 Received: from [127.0.0.1] (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.1.289.1; Sun, 22 Apr 2012 23:48:51 -0700 Message-ID: <4F94FB8C.2020906@codesourcery.com> Date: Mon, 23 Apr 2012 06:57:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20120412 Thunderbird/11.0.1 MIME-Version: 1.0 To: Pedro Alves CC: , Eli Zaretskii Subject: Re: [PATCH] breakpoint always inserted in record target References: <1334149619-2738-1-git-send-email-yao@codesourcery.com> <4F85C027.3010105@redhat.com> <4F8783C4.60807@codesourcery.com> <4F88482B.4040604@redhat.com> In-Reply-To: <4F88482B.4040604@redhat.com> Content-Type: multipart/mixed; boundary="------------000502030107050000040109" 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: 2012-04/txt/msg00733.txt.bz2 --------------000502030107050000040109 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Content-length: 3623 On 04/13/2012 11:37 PM, Pedro Alves wrote: > Indeed this is a missing step. I see we're now internal-erroring, > which I missed at: > > > We weren't before, always-inserted mode would just get ignored. > So one thing you're missing is to revert 9572dfe7e9b04046543bd2007f3beb11a9049ba4 : > > 2011-12-05 Pedro Alves > > * breakpoint.c: Include record.h. > (breakpoints_always_inserted_mode): Return false when the record > target is in use. > > as otherwise the record target doesn't really do always inserted. :-) > OK, revert it in this new version. > On 04/13/2012 02:39 AM, Yao Qi wrote: > >> > +/* Invoke CALLBACK for each of bp_location. */ >> > + >> > +void >> > +iterate_over_bp_locations (walk_bp_location_callback callback) >> > +{ >> > + struct bp_location *loc, **loc_tmp; >> > + >> > + ALL_BP_LOCATIONS (loc, loc_tmp) >> > + { >> > + callback (loc); >> > + } >> > +} >> > + > >> > extern void breakpoint_auto_delete (bpstat); >> > >> > +typedef void (*walk_bp_location_callback) (void *data); >> > + >> > +extern void iterate_over_bp_locations (walk_bp_location_callback); >> > + > >> > +static void >> > +record_sync_record_breakpoints (void *data) >> > +{ >> > + struct bp_location *loc = (struct bp_location *) data; > > > You misunderstood Doug's comment. Functions that take callbacks > generally _also_ take a void* to pass to the callback, in addition > to the iterated object. > > extern struct breakpoint *iterate_over_breakpoints (int (*) (struct breakpoint *, > void *), void *); > > struct thread_info * > iterate_over_threads (int (*callback) (struct thread_info *, void *), > void *data) > > etc. So you'd have: > > static void > record_sync_record_breakpoints (struct bp_location *bploc, void *data) > { > > and you'd call the iteration function like: > > iterate_over_bp_locations (record_sync_record_breakpoints, NULL); > > or > > iterate_over_bp_locations (some_other_callback_that_needs_additional_data, &additional_data); > > Got you. Fixed. >> > +static void >> > +record_sync_record_breakpoints (void *data) >> > +{ >> > + struct bp_location *loc = (struct bp_location *) data; >> > + >> > + if (loc->inserted) >> > + { >> > + struct record_breakpoint *bp = XNEW (struct record_breakpoint); >> > + >> > + bp->addr = loc->target_info.placed_address; >> > + bp->address_space = loc->target_info.placed_address_space; >> > + >> > + bp->in_target_beneath = 1; >> > + >> > + VEC_safe_push (record_breakpoint_p, record_breakpoints, bp); >> > + } >> > +} >> > + > This creates a struct record_breakpoint for locations that aren't > really breakpoints. E.g., what about catchpoints and tracepoints? > At least those should be skipped. Haven't thought properly if > we need to do something else for watchpoints too. In this new version, catchpoints, tracepoints, and watchpoints are skipped. Current test cases in gdb.reverse/ doesn't cover the situation that we have catchpoints/tracepoints/watchpoints installed before switching to target record, and verify these points still work as expected in record target. This can be addressed by a separate patch. Tested this new version on x86_64-linux in native gdb and native-gdbserver. No regression. This version relies on this pending patch, [PATCH] ep_is_catchpoint -> is_catchpoint http://sourceware.org/ml/gdb-patches/2012-04/msg00731.html -- Yao (齐尧) --------------000502030107050000040109 Content-Type: text/x-patch; name="0002-breakpoint-always-inserted-in-record-target.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-breakpoint-always-inserted-in-record-target.patch" Content-length: 5015 gdb: 2012-04-23 Yao Qi Revert this patch to allow breakpoint always-inserted in record target. 2011-12-05 Pedro Alves * breakpoint.c: Include record.h. (breakpoints_always_inserted_mode): Return false when the record target is in use. * breakpoint.c (iterate_over_bp_locations): New. * breakpoint.h: Declare. New typedef walk_bp_location_callback. * record.c (record_open): Call record_init_record_breakpoints. (record_sync_record_breakpoints): New. (record_init_record_breakpoints): New. * NEWS: Mention supporting breakpoint always-inserted mode in record target. --- gdb/NEWS | 3 +++ gdb/breakpoint.c | 19 +++++++++++++++---- gdb/breakpoint.h | 4 ++++ gdb/record.c | 38 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index 72b4d90..72ce6b8 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -82,6 +82,9 @@ * Ada support for GDB/MI Variable Objects has been added. +* GDB can now support 'breakpoint always-inserted mode' in 'record' + target. + * New commands ** "catch load" and "catch unload" can be used to stop when a shared diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index ceca221..4204e36 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -64,7 +64,6 @@ #include "continuations.h" #include "stack.h" #include "skip.h" -#include "record.h" #include "gdb_regex.h" #include "ax-gdb.h" @@ -404,9 +403,8 @@ show_always_inserted_mode (struct ui_file *file, int from_tty, int breakpoints_always_inserted_mode (void) { - return ((always_inserted_mode == always_inserted_on - || (always_inserted_mode == always_inserted_auto && non_stop)) - && !RECORD_IS_USED); + return (always_inserted_mode == always_inserted_on + || (always_inserted_mode == always_inserted_auto && non_stop)); } static const char condition_evaluation_both[] = "host or target"; @@ -2425,6 +2423,19 @@ insert_breakpoints (void) insert_breakpoint_locations (); } +/* Invoke CALLBACK for each of bp_location. */ + +void +iterate_over_bp_locations (walk_bp_location_callback callback) +{ + struct bp_location *loc, **loc_tmp; + + ALL_BP_LOCATIONS (loc, loc_tmp) + { + callback (loc, NULL); + } +} + /* This is used when we need to synch breakpoint conditions between GDB and the target. It is the case with deleting and disabling of breakpoints when using always-inserted mode. */ diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index e676659..75b62d2 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1131,6 +1131,10 @@ extern void delete_breakpoint (struct breakpoint *); extern void breakpoint_auto_delete (bpstat); +typedef void (*walk_bp_location_callback) (struct bp_location *, void *); + +extern void iterate_over_bp_locations (walk_bp_location_callback); + /* Return the chain of command lines to execute when this breakpoint is hit. */ extern struct command_line *breakpoint_commands (struct breakpoint *b); diff --git a/gdb/record.c b/gdb/record.c index 9b7ee2f..08e5958 100644 --- a/gdb/record.c +++ b/gdb/record.c @@ -896,6 +896,8 @@ record_open_1 (char *name, int from_tty) push_target (&record_ops); } +static void record_init_record_breakpoints (void); + /* "to_open" target method. Open the process record target. */ static void @@ -993,6 +995,8 @@ record_open (char *name, int from_tty) record_async_inferior_event_token = create_async_event_handler (record_async_inferior_event_handler, NULL); + + record_init_record_breakpoints (); } /* "to_close" target method. Close the process record target. */ @@ -1744,6 +1748,40 @@ DEF_VEC_P(record_breakpoint_p); active. */ VEC(record_breakpoint_p) *record_breakpoints = NULL; +static void +record_sync_record_breakpoints (struct bp_location *loc, void *data) +{ + /* Catchpoints and tracepoints should be skipped. + TODO: something else should be done for watchpoints, and test cases + are needed in testsuite/gdb.reverse for tracepoints, catchpoints, and + watchpoints. */ + if (is_tracepoint (loc->owner) || is_catchpoint (loc->owner) + || is_watchpoint (loc->owner)) + return; + + if (loc->inserted) + { + struct record_breakpoint *bp = XNEW (struct record_breakpoint); + + bp->addr = loc->target_info.placed_address; + bp->address_space = loc->target_info.placed_address_space; + + bp->in_target_beneath = 1; + + VEC_safe_push (record_breakpoint_p, record_breakpoints, bp); + } +} + +/* Sync existing breakpoints to record_breakpoints. */ + +static void +record_init_record_breakpoints (void) +{ + VEC_free (record_breakpoint_p, record_breakpoints); + + iterate_over_bp_locations (record_sync_record_breakpoints); +} + /* Behavior is conditional on RECORD_IS_REPLAY. We will not actually insert or remove breakpoints in the real target when replaying, nor when recording. */ -- 1.7.0.4 --------------000502030107050000040109--