fix list/edit command in hook-stop

fix list/edit command in hook-stop

[Jason Kraftcheck]

An 'edit' or 'list' command in hook-stop doesn't work because the
current sal isn't updated until after hook-stop is run.  The following
trivial change moves the invocation of hook-stop later in normal_stop.
This won't fix the problem in all cases, as the sal is apparently only
updated if print_stack_frame is called.  But it doesn't hurt anything
and works more often than without the change.


2006-02-23  Jason Kraftcheck  <kraftche@cae.wisc.edu>

	* infrun.c (normal_stop): call hook-stop handler later, after
	  current sal has been set.


diff -upr gdb-6.4/gdb/infrun.c mod/gdb/infrun.c
--- gdb-6.4/gdb/infrun.c	2005-11-14 09:35:16.000000000 -0600
+++ mod/gdb/infrun.c	2006-02-23 11:55:12.469293121 -0600
@@ -3038,12 +3038,6 @@ Further execution is probably impossible

   target_terminal_ours ();

-  /* Look up the hook_stop and run it (CLI internally handles problem
-     of stop_command's pre-hook not existing).  */
-  if (stop_command)
-    catch_errors (hook_stop_stub, stop_command,
-		  "Error while running hook_stop:\n", RETURN_MASK_ALL);
-
   if (!target_has_stack)
     {

@@ -3140,6 +3134,12 @@ Further execution is probably impossible
       select_frame (get_current_frame ());
     }

+  /* Look up the hook_stop and run it (CLI internally handles problem
+     of stop_command's pre-hook not existing).  */
+  if (stop_command)
+    catch_errors (hook_stop_stub, stop_command,
+		  "Error while running hook_stop:\n", RETURN_MASK_ALL);
+
 done:
   annotate_stopped ();
   observer_notify_normal_stop (stop_bpstat);

Re: fix list/edit command in hook-stop

[Daniel Jacobowitz]

On Thu, Feb 23, 2006 at 12:20:14PM -0600, Jason Kraftcheck wrote:
> An 'edit' or 'list' command in hook-stop doesn't work because the
> current sal isn't updated until after hook-stop is run.  The following
> trivial change moves the invocation of hook-stop later in normal_stop.
> This won't fix the problem in all cases, as the sal is apparently only
> updated if print_stack_frame is called.  But it doesn't hurt anything
> and works more often than without the change.
> 
> 
> 2006-02-23  Jason Kraftcheck  <kraftche@cae.wisc.edu>
> 
> 	* infrun.c (normal_stop): call hook-stop handler later, after
> 	  current sal has been set.

From the manual:

   In addition, a pseudo-command, `stop' exists.  Defining
(`hook-stop') makes the associated commands execute every time
execution stops in your program: before breakpoint commands are run,
displays are printed, or the stack frame is printed.

Your patch would change that; a stop hook which conditionally continued
would now be noisy instead of silent, so I don't think this is a good
change.

Can you just update the sal explicitly, instead of moving things around?


-- 
Daniel Jacobowitz
CodeSourcery

Re: fix list/edit command in hook-stop

[Jason Kraftcheck]

Daniel Jacobowitz wrote:
> On Thu, Feb 23, 2006 at 12:20:14PM -0600, Jason Kraftcheck wrote:
> 
>>An 'edit' or 'list' command in hook-stop doesn't work because the
>>current sal isn't updated until after hook-stop is run.  The following
>>trivial change moves the invocation of hook-stop later in normal_stop.
>>This won't fix the problem in all cases, as the sal is apparently only
>>updated if print_stack_frame is called.  But it doesn't hurt anything
>>and works more often than without the change.
>>
>>
>>2006-02-23  Jason Kraftcheck  <kraftche@cae.wisc.edu>
>>
>>	* infrun.c (normal_stop): call hook-stop handler later, after
>>	  current sal has been set.
> 
> 
>>From the manual:
> 
>    In addition, a pseudo-command, `stop' exists.  Defining
> (`hook-stop') makes the associated commands execute every time
> execution stops in your program: before breakpoint commands are run,
> displays are printed, or the stack frame is printed.
> 
> Your patch would change that; a stop hook which conditionally continued
> would now be noisy instead of silent, so I don't think this is a good
> change.
> 
> Can you just update the sal explicitly, instead of moving things around?
> 
> 


Is this OK?


2006-02-23  Jason Kraftcheck  <kraftche@cae.wisc.edu>

	* infrun.c (normal_stop): Set current SAL before calling
          hook-stop handler so edit and list commands work.
	* frame.h: Add delcaration of set_current_sal_from_frame
	* stack.c: Make set_current_sal_from_frame extern


diff -urp gdb-6.4/gdb/frame.h mod/gdb/frame.h
--- gdb-6.4/gdb/frame.h	2005-05-22 09:53:34.000000000 -0500
+++ mod/gdb/frame.h	2006-02-23 15:33:11.861630699 -0600
@@ -300,6 +300,14 @@ extern CORE_ADDR get_frame_func (struct
 extern void find_frame_sal (struct frame_info *frame,
 			    struct symtab_and_line *sal);

+
+/* Set the current source and line to the location given by frame
+   FRAME, if possible.  When CENTER is true, adjust so the relevant
+   line is in the center of the next 'list'.  */
+extern void set_current_sal_from_frame (struct frame_info * frame,
+                                        int center);
+
+
 /* Return the frame base (what ever that is) (DEPRECATED).

    Old code was trying to use this single method for two conflicting
Only in mod/gdb: gdb-6.4.patch.4
diff -urp gdb-6.4/gdb/infrun.c mod/gdb/infrun.c
--- gdb-6.4/gdb/infrun.c	2005-11-14 09:35:16.000000000 -0600
+++ mod/gdb/infrun.c	2006-02-23 15:28:44.485723627 -0600
@@ -3041,8 +3041,12 @@ Further execution is probably impossible
   /* Look up the hook_stop and run it (CLI internally handles problem
      of stop_command's pre-hook not existing).  */
   if (stop_command)
-    catch_errors (hook_stop_stub, stop_command,
-		  "Error while running hook_stop:\n", RETURN_MASK_ALL);
+    {
+      if (target_has_stack)
+        set_current_sal_from_frame (get_current_frame (), 1);
+      catch_errors (hook_stop_stub, stop_command,
+		    "Error while running hook_stop:\n", RETURN_MASK_ALL);
+    }

   if (!target_has_stack)
     {
diff -urp gdb-6.4/gdb/stack.c mod/gdb/stack.c
--- gdb-6.4/gdb/stack.c	2005-08-18 08:26:41.000000000 -0500
+++ mod/gdb/stack.c	2006-02-23 15:36:40.575237163 -0600
@@ -61,8 +61,6 @@ static void print_frame (struct frame_in
 			 enum print_what print_what,  int print_args,
 			 struct symtab_and_line sal);

-static void set_current_sal_from_frame (struct frame_info *, int);
-
 /* Zero means do things normally; we are interacting directly with the
    user.  One means print the full filename and linenumber when a
    frame is printed, and do so in a format emacs18/emacs19.22 can
@@ -373,7 +371,7 @@ print_args_stub (void *args)
    FRAME, if possible.  When CENTER is true, adjust so the relevant
    line is in the center of the next 'list'.  */

-static void
+void
 set_current_sal_from_frame (struct frame_info *frame, int center)
 {
   struct symtab_and_line sal;

Re: fix list/edit command in hook-stop

[Eli Zaretskii]

> Date: Thu, 23 Feb 2006 16:13:01 -0600
> From: Jason Kraftcheck <kraftche@cae.wisc.edu>
> CC: gdb-patches@sources.redhat.com
> 
> --- gdb-6.4/gdb/infrun.c	2005-11-14 09:35:16.000000000 -0600
> +++ mod/gdb/infrun.c	2006-02-23 15:28:44.485723627 -0600
> @@ -3041,8 +3041,12 @@ Further execution is probably impossible
>    /* Look up the hook_stop and run it (CLI internally handles problem
>       of stop_command's pre-hook not existing).  */
>    if (stop_command)
> -    catch_errors (hook_stop_stub, stop_command,
> -		  "Error while running hook_stop:\n", RETURN_MASK_ALL);
> +    {
> +      if (target_has_stack)
> +        set_current_sal_from_frame (get_current_frame (), 1);
> +      catch_errors (hook_stop_stub, stop_command,
> +		    "Error while running hook_stop:\n", RETURN_MASK_ALL);
> +    }
> 

I'd suggest a comment here explaining why set_current_sal_from_frame
is called.

Re: fix list/edit command in hook-stop

[Jason Kraftcheck]

Eli Zaretskii wrote:
...
> 
> I'd suggest a comment here explaining why set_current_sal_from_frame
> is called.

OK.


2006-02-23  Jason Kraftcheck  <kraftche@cae.wisc.edu>

	* infrun.c (normal_stop): Set current SAL before calling
          hook-stop handler so edit and list commands work.
	* frame.h: Add delcaration of set_current_sal_from_frame
	* stack.c: Make set_current_sal_from_frame extern


diff -urp gdb-6.4/gdb/frame.h mod/gdb/frame.h
--- gdb-6.4/gdb/frame.h	2005-05-22 09:53:34.000000000 -0500
+++ mod/gdb/frame.h	2006-02-28 10:27:00.165163260 -0600
@@ -300,6 +300,14 @@ extern CORE_ADDR get_frame_func (struct
 extern void find_frame_sal (struct frame_info *frame,
 			    struct symtab_and_line *sal);

+
+/* Set the current source and line to the location given by frame
+   FRAME, if possible.  When CENTER is true, adjust so the relevant
+   line is in the center of the next 'list'.  */
+extern void set_current_sal_from_frame (struct frame_info * frame,
+                                        int center);
+
+
 /* Return the frame base (what ever that is) (DEPRECATED).

    Old code was trying to use this single method for two conflicting
diff -urp gdb-6.4/gdb/infrun.c mod/gdb/infrun.c
--- gdb-6.4/gdb/infrun.c	2005-11-14 09:35:16.000000000 -0600
+++ mod/gdb/infrun.c	2006-02-28 10:27:53.734132365 -0600
@@ -3041,8 +3041,14 @@ Further execution is probably impossible
   /* Look up the hook_stop and run it (CLI internally handles problem
      of stop_command's pre-hook not existing).  */
   if (stop_command)
-    catch_errors (hook_stop_stub, stop_command,
-		  "Error while running hook_stop:\n", RETURN_MASK_ALL);
+    {
+      /* Set current sal for use by edit or list commands if
+         called from within hook-stop */
+      if (target_has_stack)
+        set_current_sal_from_frame (get_current_frame (), 1);
+      catch_errors (hook_stop_stub, stop_command,
+		    "Error while running hook_stop:\n", RETURN_MASK_ALL);
+    }

   if (!target_has_stack)
     {
diff -urp gdb-6.4/gdb/stack.c mod/gdb/stack.c
--- gdb-6.4/gdb/stack.c	2005-08-18 08:26:41.000000000 -0500
+++ mod/gdb/stack.c	2006-02-28 10:27:00.205158011 -0600
@@ -61,8 +61,6 @@ static void print_frame (struct frame_in
 			 enum print_what print_what,  int print_args,
 			 struct symtab_and_line sal);

-static void set_current_sal_from_frame (struct frame_info *, int);
-
 /* Zero means do things normally; we are interacting directly with the
    user.  One means print the full filename and linenumber when a
    frame is printed, and do so in a format emacs18/emacs19.22 can
@@ -373,7 +371,7 @@ print_args_stub (void *args)
    FRAME, if possible.  When CENTER is true, adjust so the relevant
    line is in the center of the next 'list'.  */

-static void
+void
 set_current_sal_from_frame (struct frame_info *frame, int center)
 {
   struct symtab_and_line sal;

Re: fix list/edit command in hook-stop

[Eli Zaretskii]

> Date: Tue, 28 Feb 2006 10:30:35 -0600
> From: Jason Kraftcheck <kraftche@cae.wisc.edu>
> CC: Eli Zaretskii <eliz@gnu.org>
> 
> Eli Zaretskii wrote:
> ...
> > 
> > I'd suggest a comment here explaining why set_current_sal_from_frame
> > is called.
> 
> OK.
> [...]
> +      /* Set current sal for use by edit or list commands if
> +         called from within hook-stop */
> +      if (target_has_stack)
> +        set_current_sal_from_frame (get_current_frame (), 1);
> +      catch_errors (hook_stop_stub, stop_command,
> +		    "Error while running hook_stop:\n", RETURN_MASK_ALL);
> +    }

Thanks.  However, I thought we should explain why
set_current_sal_from_frame is called here.  I believe the reason is
that without the call, the current sal will not be up to date if
`edit' or `list' are called inside a hook-stop.

Re: fix list/edit command in hook-stop

[Daniel Jacobowitz]

On Tue, Feb 28, 2006 at 10:08:41PM +0200, Eli Zaretskii wrote:
> Thanks.  However, I thought we should explain why
> set_current_sal_from_frame is called here.  I believe the reason is
> that without the call, the current sal will not be up to date if
> `edit' or `list' are called inside a hook-stop.

I think the patch ought to set the sal regardless of whether a stop
hook is defined.  However, there's something more complicated going on. 
The obvious patch from my description causes lots of test failures like
this one:

(gdb) PASS: gdb.base/call-ar-st.exp: run until breakpoint set at a line
print print_double_array(double_array)
array_d :
=========

0.000000  
23.456700  46.913400  70.370100  93.826800  117.283500  140.740200
164.196900  187.653600  


$1 = void
(gdb) PASS: gdb.base/call-ar-st.exp: print
print_double_array(double_array)
print print_char_array(char_array)
array_c :
=========

Z
aZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZ
aZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZ
aZaZaZaZaZaZaZaZaZa

$2 = void
(gdb) PASS: gdb.base/call-ar-st.exp: print print_char_array(char_array)
tbreak 1216
No line 1216 in file "../sysdeps/x86_64/elf/start.S".
(gdb) FAIL: gdb.base/call-ar-st.exp: tbreakpoint line 1216

The reason we're suddenly in start.S is that the breakpoint we hit
after calling print_double_array is in _start.  So the right patch
would actually look like the attached.

Is this better?

-- 
Daniel Jacobowitz
CodeSourcery

2006-03-01  Daniel Jacobowitz  <dan@codesourcery.com>
 
	* frame.h (set_current_sal_from_frame): New prototype.
	* stack.c (set_current_sal_from_frame): Make global.
	* infrun.c (normal_stop): Call set_current_sal_from_frame.

Index: frame.h
===================================================================
RCS file: /cvs/src/src/gdb/frame.h,v
retrieving revision 1.147
diff -u -p -r1.147 frame.h
--- frame.h	17 Dec 2005 22:33:59 -0000	1.147
+++ frame.h	1 Mar 2006 17:53:12 -0000
@@ -300,6 +300,12 @@ extern CORE_ADDR get_frame_func (struct 
 extern void find_frame_sal (struct frame_info *frame,
 			    struct symtab_and_line *sal);
 
+/* Set the current source and line to the location given by frame
+   FRAME, if possible.  When CENTER is true, adjust so the relevant
+   line is in the center of the next 'list'.  */
+
+void set_current_sal_from_frame (struct frame_info *, int);
+
 /* Return the frame base (what ever that is) (DEPRECATED).
 
    Old code was trying to use this single method for two conflicting
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.209
diff -u -p -r1.209 infrun.c
--- infrun.c	4 Jan 2006 19:34:58 -0000	1.209
+++ infrun.c	1 Mar 2006 17:53:13 -0000
@@ -3044,6 +3044,12 @@ Further execution is probably impossible
 
   target_terminal_ours ();
 
+  /* Set the current source location.  This will also happen if we
+     display the frame below, but the current SAL will be incorrect
+     during a user hook-stop function.  */
+  if (target_has_stack && !stop_stack_dummy)
+    set_current_sal_from_frame (get_current_frame (), 1);
+
   /* Look up the hook_stop and run it (CLI internally handles problem
      of stop_command's pre-hook not existing).  */
   if (stop_command)
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.137
diff -u -p -r1.137 stack.c
--- stack.c	17 Dec 2005 22:34:03 -0000	1.137
+++ stack.c	1 Mar 2006 17:53:14 -0000
@@ -61,8 +61,6 @@ static void print_frame (struct frame_in
 			 enum print_what print_what,  int print_args,
 			 struct symtab_and_line sal);
 
-static void set_current_sal_from_frame (struct frame_info *, int);
-
 /* Zero means do things normally; we are interacting directly with the
    user.  One means print the full filename and linenumber when a
    frame is printed, and do so in a format emacs18/emacs19.22 can
@@ -373,7 +371,7 @@ print_args_stub (void *args)
    FRAME, if possible.  When CENTER is true, adjust so the relevant
    line is in the center of the next 'list'.  */
 
-static void
+void
 set_current_sal_from_frame (struct frame_info *frame, int center)
 {
   struct symtab_and_line sal;

Re: fix list/edit command in hook-stop

[Eli Zaretskii]

> Date: Wed, 1 Mar 2006 12:58:14 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Jason Kraftcheck <kraftche@cae.wisc.edu>,
> 	gdb-patches@sources.redhat.com
> 
> Is this better?

Yes, thanks.

Re: fix list/edit command in hook-stop

[Daniel Jacobowitz]

On Wed, Mar 01, 2006 at 12:58:14PM -0500, Daniel Jacobowitz wrote:
> 2006-03-01  Daniel Jacobowitz  <dan@codesourcery.com>
>  
> 	* frame.h (set_current_sal_from_frame): New prototype.
> 	* stack.c (set_current_sal_from_frame): Make global.
> 	* infrun.c (normal_stop): Call set_current_sal_from_frame.

I have belatedly checked this in.  Thanks for the report.

-- 
Daniel Jacobowitz
CodeSourcery