Re: Patch to propagate GDB's knowledge of the executing state to frontend

[ali_anwar <ali_anwar@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 2689 bytes --]

On 10/30/2012 04:52 PM, ali_anwar wrote:
> On 10/30/2012 11:20 AM, Yao Qi wrote:
>> On 10/25/2012 07:09 PM, ali_anwar wrote:
>>> Hi,
>>>
>>> Attached patch is to let GDB propagate the target state under following
>>> two scenarios:
>>>
>>> 1. Attached patch will enable GDB to inform about the state of the
>>> target when it was not able to fetch the non-general registers, when
>>> target is already stopped.
>>>
>>> The reason behind this behavior was an error message which was caused
>>> when the GDB was not able to fetch the value of a certain register. The
>>> GDB should have told the front end about the current state of the
>>> target. The attached patch makes sure that it happens. This patch should
>>> be a safety measure in case some debugging stub behaves badly.
>>>
>>> 2. Attached patch will enable GDB to inform about the state of the
>>> target when it was not able to fetch the backtrace once the step has
>>> already occurred and target is in stopped state.
>>>
>>
>> It is better to describe what will happen or what is wrong if this patch
>> is not applied.
>>
>
> Thanks Yao for the review. Let me restate the actual problem:
>
> Under certain scenarios, GDB is unable to specify the correct target
> state once the step/finish instruction is executed.
>
> 1. If you perform a step out (finish) and there is an error when GDB
> tries to fetch the register values.
>
> 2. If you perform a ste and there is an error when GDB tries to fetch
> the backtrace.
>
> In both the cases the only output is an error message and nothing is
> printed as far as current target state is concerned.e.g.
>
>
> (gdb)
> -exec-finish
> ^running
> *running,thread-id="all"
> (gdb)
> ^error,msg="Could not fetch register \"\"; remote failure reply 'E22'"
> (gdb)
>
>
> In other words from MI's perspective, the step hasn't completed yet –
> the state is still "running".
>
> The only concern is GDB not printing the state of the target. It does
> not matter why the error occurred.
>
>>> + 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.
>>
>
> I will fix the both typos.
>
>> 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.
>>
>
> I will look into it as well.
>
>>> + handle_inferior_event (ecs);
>>> + return (0);
>>
>> parentheses are not needed.
>>
>>> +}
>>> +
>>> + return (0);
>>
>> Likewise.
>>
>
> I will remove the parentheses.
>

PFA the latest patch. OK to commit?

-Ali

[-- Attachment #2: target_state.patch --]
[-- Type: text/x-patch, Size: 2318 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.14760
diff -u -r1.14760 ChangeLog
--- ChangeLog	24 Oct 2012 19:08:15 -0000	1.14760
+++ ChangeLog	25 Oct 2012 10:52:02 -0000
@@ -1,3 +1,13 @@
+2012-10-25  Ali Anwar  <ali_anwar@codesourcery.com>
+
+	* 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.
+	(wait_for_inferior): Likewise.
+
 2012-10-24  Tristan Gingold  <gingold@adacore.com>
 
 	* ravenscar-sparc-thread.c (ravenscar_sparc_fetch_registers):
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.559
diff -u -r1.559 infrun.c
--- infrun.c	17 Sep 2012 07:26:55 -0000	1.559
+++ infrun.c	25 Oct 2012 10:52:04 -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);
@@ -2701,7 +2705,8 @@
       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);
+      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);
@@ -6082,7 +6087,8 @@
 
       /* NB: The copy goes through to the target picking up the value of
 	 all the registers.  */
-      stop_registers = regcache_dup (get_current_regcache ());
+      catch_errors (regcache_dup_stub, NULL,
+                    "Error while running regcache_dup:\n", RETURN_MASK_ALL);
     }
 
   if (stop_stack_dummy == STOP_STACK_DUMMY)
@@ -6154,6 +6160,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);