[RFC] PR 15075 dprintf interferes with "next"

[Yao Qi <yao at codesourcery dot com>]

Hi,
I'd like to get your comments on this patch, as I think my patch is
hacky but I am unable to figure out a correct one.  This patch fixes more
problems than PR 15075 mentioned, they are,

  - When step a source line by "next" command, and there is a dprintf
    on that line, "next" command will behave like "continue".
  - Two dprintf are set on the same address.  Only one printf is
    executed.
  - dprintf and regular breakpoint are set on the same address,
    inferior doesn't stop.

In order to fix these problems, I don't append "continue" command to
dprintf breakpoint, and after all commands of all breakpoints (caused
the stop) are executed, execute "continue" command if
    1) the inferior is not proceeded by these commands
    2) and the stop is only caused by dprintf (no other types of
breakpoint)
    3) the inferior is not in the state of stepping a source line or
an instruction (to fix PR 15075).

My patch fixes all the three known problems, but I think it is still
hacky.

The printf stuff is implemented as command, it is convenient to move
them to the target side.  I tried to change the bpstat saying that
"don't stop", but the command can't be executed.  We have to keep
bpstat saying that "stop" for dprintf.  I also thought about Tom's
suggestion about Python "Breakpoint.stop" API, but "Breakpoint.stop"
acts as condition instead of command, so I don't go that way.

Known issue: from the user's perspective, inferior stops at the dprintf
breakpoint if dprintf breakpoint and regular breakpoint are set at
the same address.  I'll fix it if my patch is on the right track.

gdb:

2013-02-18  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c (bpstat_do_actions_1): Change argument from
	"bpstat *" to "struc thread_info *".
	(bpstat_do_actions_1): Execute "continue" command if BS has
	dprintf breakpoint only.
	(bpstat_do_actions): Caller update.
	(update_dprintf_command_list): Don't append "continue" command
	to the command of dprintf breakpoint.
---
 gdb/breakpoint.c |   68 ++++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index fb57a57..878ab08 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4263,20 +4263,26 @@ command_line_is_silent (struct command_line *cmd)
 }
 
 /* Execute all the commands associated with all the breakpoints at
-   this location.  Any of these commands could cause the process to
-   proceed beyond this point, etc.  We look out for such changes by
-   checking the global "breakpoint_proceeded" after each command.
+   the location where THREAD stops.  Any of these commands could cause
+   the process to proceed beyond this point, etc.  We look out for
+   such changes by checking the global "breakpoint_proceeded" after
+   each command.
 
    Returns true if a breakpoint command resumed the inferior.  In that
    case, it is the caller's responsibility to recall it again with the
    bpstat of the current thread.  */
 
 static int
-bpstat_do_actions_1 (bpstat *bsp)
+bpstat_do_actions_1 (struct thread_info *thread)
 {
+  bpstat *bsp = &thread->control.stop_bpstat;
   bpstat bs;
   struct cleanup *old_chain;
   int again = 0;
+  /* Breakpoint dprintf in list BS.  */
+  int bkpt_dprintf = 0;
+  /* Breakpoint other than dprintf in list BS.  */
+  int bkpt_other_than_dprintf = 0;
 
   /* Avoid endless recursion if a `source' command is contained
      in bs->commands.  */
@@ -4298,6 +4304,13 @@ bpstat_do_actions_1 (bpstat *bsp)
       struct command_line *cmd;
       struct cleanup *this_cmd_tree_chain;
 
+      if (bs->breakpoint_at != NULL)
+	{
+	  if (bs->breakpoint_at->type == bp_dprintf)
+	    bkpt_dprintf = 1;
+	  else
+	    bkpt_other_than_dprintf = 1;
+	}
       /* Take ownership of the BSP's command tree, if it has one.
 
          The command tree could legitimately contain commands like
@@ -4356,6 +4369,33 @@ bpstat_do_actions_1 (bpstat *bsp)
 	  break;
 	}
     }
+
+  /* Inferior is not proceed by commands.  */
+  if (!breakpoint_proceeded
+      /* No breakpoint of type other than dprintf.  If there is
+	 breakpoint of other types in BS, the inferior really needs
+	 stopped.  Don't execute command "continue".  */
+      && !bkpt_other_than_dprintf && bkpt_dprintf
+      && (0 != strcmp (dprintf_style, dprintf_style_agent))
+      /* GDB is not stepping a line.  When GDB is stepping a source
+	 line, don't execute command "continue", otherwise "next" will
+	 behave like "continue", which is not expected.  */
+      && !(thread->control.step_range_start >= 1
+	   && thread->control.step_range_end >= 1))
+    {
+      struct command_line *cont_cmd_line
+	= xmalloc (sizeof (struct command_line));
+
+      cont_cmd_line->control_type = simple_control;
+      cont_cmd_line->body_count = 0;
+      cont_cmd_line->body_list = NULL;
+      cont_cmd_line->next = NULL;
+      cont_cmd_line->line = xstrdup ("continue");;
+
+      execute_control_command (cont_cmd_line);
+      if (!target_can_async_p ())
+	again = 1;
+    }
   do_cleanups (old_chain);
   return again;
 }
@@ -4374,7 +4414,7 @@ bpstat_do_actions (void)
        and only return when it is stopped at the next breakpoint, we
        keep doing breakpoint actions until it returns false to
        indicate the inferior was not resumed.  */
-    if (!bpstat_do_actions_1 (&inferior_thread ()->control.stop_bpstat))
+    if (!bpstat_do_actions_1 (inferior_thread ()))
       break;
 
   discard_cleanups (cleanup_if_error);
@@ -8947,25 +8987,15 @@ update_dprintf_command_list (struct breakpoint *b)
 		    _("Invalid dprintf style."));
 
   gdb_assert (printf_line != NULL);
-  /* Manufacture a printf/continue sequence.  */
+  /* Manufacture a printf command.  */
   {
-    struct command_line *printf_cmd_line, *cont_cmd_line = NULL;
-
-    if (strcmp (dprintf_style, dprintf_style_agent) != 0)
-      {
-	cont_cmd_line = xmalloc (sizeof (struct command_line));
-	cont_cmd_line->control_type = simple_control;
-	cont_cmd_line->body_count = 0;
-	cont_cmd_line->body_list = NULL;
-	cont_cmd_line->next = NULL;
-	cont_cmd_line->line = xstrdup ("continue");
-      }
+    struct command_line *printf_cmd_line
+      = xmalloc (sizeof (struct command_line));
 
-    printf_cmd_line = xmalloc (sizeof (struct command_line));
     printf_cmd_line->control_type = simple_control;
     printf_cmd_line->body_count = 0;
     printf_cmd_line->body_list = NULL;
-    printf_cmd_line->next = cont_cmd_line;
+    printf_cmd_line->next = NULL;
     printf_cmd_line->line = printf_line;
 
     breakpoint_set_commands (b, printf_cmd_line);
-- 
1.7.7.6