From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6978 invoked by alias); 2 Nov 2012 18:47:46 -0000 Received: (qmail 6969 invoked by uid 22791); 2 Nov 2012 18:47:45 -0000 X-SWARE-Spam-Status: No, hits=-4.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_EG 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; Fri, 02 Nov 2012 18:47:40 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1TUMHL-0001ix-Iz from Ali_Anwar@mentor.com ; Fri, 02 Nov 2012 11:47:39 -0700 Received: from SVR-ORW-FEM-05.mgc.mentorg.com ([147.34.97.43]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Fri, 2 Nov 2012 11:47:39 -0700 Received: from [137.202.157.121] (147.34.91.1) by mail-na.mentorg.com (147.34.97.43) with Microsoft SMTP Server (TLS) id 14.1.289.1; Fri, 2 Nov 2012 11:47:38 -0700 Message-ID: <50941519.6010005@codesourcery.com> Date: Fri, 02 Nov 2012 18:47:00 -0000 From: ali_anwar User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.28) Gecko/20120313 Thunderbird/3.1.20 MIME-Version: 1.0 To: CC: Yao Qi , Subject: Re: Patch to propagate GDB's knowledge of the executing state to frontend References: <50891E05.7050509@codesourcery.com> <508F719C.2080409@codesourcery.com> <20627.61842.606081.697743@ruffy2.mtv.corp.google.com> In-Reply-To: <20627.61842.606081.697743@ruffy2.mtv.corp.google.com> Content-Type: multipart/mixed; boundary="------------050608070103070107020804" 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-11/txt/msg00066.txt.bz2 --------------050608070103070107020804 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1766 On 11/02/2012 09:15 PM, dje@google.com wrote: > Yao Qi writes: > > On 10/25/2012 07:09 PM, ali_anwar wrote: > > > [...] > > > @@ -1,3 +1,13 @@ > > > +2012-10-25 Ali Anwar > > > + > > > + * infrun.c (handle_inferior_event_stub, regcache_dup_stub): > > > + New functions. > > > + (normal_stop): Change to propagate GDB's knowledge of the > > > + executing state to frontend when not able to fetch registers. > > > + (wait_for_inferior): Chnage to propagate GDB's knowledge of > > ^^^^^^ typo > > > > > > > + the executing state if not able to fetch backtrace once the > > > + step has already occured. > > ^^^^^^^ typo. > > > > In each changelog entry, we'll put 'what do we change' instead of 'why > > do we change in this way'. So this entry can be simplified. > > Hi. > > I agree with your first sentence, and would add that if such an > explanation is needed, it belongs in the code not the changelog. > [We don't have enough comments in the code explaining *why* things > are the way they are.] > > But I'd say that's not the case here, at least for the changelog entries. > Instead, I would remove the leading "Change to", and just say "Propagate ...". > > Also, I would add a comment to the code explaining *why* the calls are wrapped > in catch_error (and I would have the comment live at the call to catch_error, > not in the definition of the two new stubs). > > One could also say the two new functions also require comments, > but they're pretty simple and hook_stop_stub doesn't have a comment, > so I'd be ok with leaving them out. Thanks for the review. Please find attached the modified patch. -Ali --------------050608070103070107020804 Content-Type: text/x-patch; name="target_state.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="target_state.patch" Content-length: 2853 Index: gdb/ChangeLog =================================================================== RCS file: /cvs/src/src/gdb/ChangeLog,v retrieving revision 1.14760 diff -u -r1.14760 ChangeLog --- gdb/ChangeLog 24 Oct 2012 19:08:15 -0000 1.14760 +++ gdb/ChangeLog 2 Nov 2012 18:41:48 -0000 @@ -1,3 +1,11 @@ +2012-10-25 Ali Anwar + + * infrun.c (handle_inferior_event_stub, regcache_dup_stub): + New functions. + (normal_stop): Propagate GDB's knowledge of the executing + state to frontend. + (wait_for_inferior): Likewise. + 2012-10-24 Tristan Gingold * ravenscar-sparc-thread.c (ravenscar_sparc_fetch_registers): Index: gdb/infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.559 diff -u -r1.559 infrun.c --- gdb/infrun.c 17 Sep 2012 07:26:55 -0000 1.559 +++ gdb/infrun.c 2 Nov 2012 18:41:49 -0000 @@ -73,6 +73,10 @@ static int hook_stop_stub (void *); +static int regcache_dup_stub (void *); + +static int handle_inferior_event_stub (void *); + static int restore_selected_frame (void *); static int follow_fork (void); @@ -2700,8 +2704,11 @@ state. */ old_chain = make_cleanup (finish_thread_state_cleanup, &minus_one_ptid); - /* Now figure out what to do with the result of the result. */ - handle_inferior_event (ecs); + /* Now figure out what to do with the result of the result. If an + error happens while handling the event, catch it to propagate + GDB's knowledge of the executing state. */ + catch_errors (handle_inferior_event_stub, ecs, + "Error while handling inferior event:\n", RETURN_MASK_ALL); /* No error, don't finish the state yet. */ discard_cleanups (old_chain); @@ -6080,9 +6087,12 @@ if (stop_registers) regcache_xfree (stop_registers); - /* NB: The copy goes through to the target picking up the value of - all the registers. */ - stop_registers = regcache_dup (get_current_regcache ()); + /* NB: The copy goes through to the target picking up the value + of all the registers. Catch error to propagate GDB's knowledge + of the executing state to frontend even when not able to fetch + registers. */ + catch_errors (regcache_dup_stub, NULL, + "Error while running regcache_dup:\n", RETURN_MASK_ALL); } if (stop_stack_dummy == STOP_STACK_DUMMY) @@ -6154,6 +6164,20 @@ } static int +handle_inferior_event_stub (void *ecs) +{ + handle_inferior_event (ecs); + return (0); +} + +static int +regcache_dup_stub (void *arg) +{ + stop_registers = regcache_dup (get_current_regcache ()); + return (0); +} + +static int hook_stop_stub (void *cmd) { execute_cmd_pre_hook ((struct cmd_list_element *) cmd); --------------050608070103070107020804--