[RFA] deleting breakpoints inside of 'commands'

[RFA] deleting breakpoints inside of 'commands'

[Don Howard]

The following patch addresses a core-dump that occurs when a 'commands'
script deletes the current breakpoint:

(gdb) commands
>clear
>step
>end
(gdb) break foo
(gdb) continue
	.
	.
	.
warning: Invalid control type in command structure.
Segmentation fault (core dumped)

What happens is the breakpoint (and the associated 'commands' script) is
deleted when first statement of the script is executed.  GDB runs into a
core dump when it attempts to execute the remaining (deleted)
statements.

The patch below detects if the current breakpoint has been deleted and
terminates execution of the associated 'commands' script and issues a
warning.




Index: breakpoint.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/breakpoint.c,v
retrieving revision 1.315
diff -p -u -r1.315 breakpoint.c
--- breakpoint.c        2001/07/13 23:55:47     1.315
+++ breakpoint.c        2001/09/11 22:29:40
@@ -1826,12 +1826,41 @@ top:
       cmd = bs->commands;
       while (cmd != NULL)
        {
+         int number = bs->breakpoint_at->number;
+
          execute_control_command (cmd);

          if (breakpoint_proceeded)
            break;
          else
-           cmd = cmd->next;
+           {
+             /* It is possible that the list of commands we are
+                executing includes a command to delete the current
+                breakpoint, which will also delete the current
+                command list (oops!).
+
+                If this happens we stop processing of this command
+                chain and issue a warning.
+             */
+
+             struct command_line *next_cmd = cmd->next;
+             struct breakpoint *b;
+             ALL_BREAKPOINTS (b)
+               {
+                 if (bs->breakpoint_at == b)
+                   {
+                     cmd = next_cmd;
+                     break;
+                   }
+                 else
+                   {
+                     cmd = NULL;
+                   }
+               }
+             if (cmd == NULL)
+               warning ("Breakpoint %d deleted while executing commands.",
+                        number);
+           }
        }
       if (breakpoint_proceeded)
        /* The inferior is proceeded by the command; bomb out now.



-- 
-Don
dhoward@redhat.com
gdb engineering

Re: [RFA] deleting breakpoints inside of 'commands'

[Pierre Muller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1648 bytes --]

At 00:37 12/09/01 , vous avez écrit:


>The following patch addresses a core-dump that occurs when a 'commands'
>script deletes the current breakpoint:
>
>(gdb) commands
> >clear
> >step
> >end
>(gdb) break foo
>(gdb) continue
>         .
>         .
>         .
>warning: Invalid control type in command structure.
>Segmentation fault (core dumped)
>
>What happens is the breakpoint (and the associated 'commands' script) is
>deleted when first statement of the script is executed.  GDB runs into a
>core dump when it attempts to execute the remaining (deleted)
>statements.
>
>The patch below detects if the current breakpoint has been deleted and
>terminates execution of the associated 'commands' script and issues a
>warning.

   Wouldn't it seem more logical to
isolate the commands temporarily,
by resetting the commands field to NULL ?

        headcmd = bs->commands;
+     bs->commands = NULL;
+     cmd = headcmd;
        while (cmd != NULL)

   This would allow to execute the whole
command sequences to its end.
   At end of the commands,
the commands field should be restored if
the breakpoint has not been distroyed,
otherwise headcmd should be disposed.

    This seems a much more reasonable action scheme
to me.

    Otherwise the command parser should at least warn
if any commands are given past a
'clear' or 'delete breakpoint-number',
but this will anyhow not be possible if
the 'delete' argument is an expression that is not a simple constant.


Pierre Muller
Institut Charles Sadron
6,rue Boussingault
F 67083 STRASBOURG CEDEX (France)
mailto:muller@ics.u-strasbg.fr
Phone : (33)-3-88-41-40-07  Fax : (33)-3-88-41-40-99

Re: [RFA] deleting breakpoints inside of 'commands'

[Pierre Muller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]

At 10:37 12/09/01 , vous avez écrit:
>At 00:37 12/09/01 , vous avez écrit:
>
>
>>The following patch addresses a core-dump that occurs when a 'commands'
>>script deletes the current breakpoint:
>>
>>(gdb) commands
>> >clear
>> >step
>> >end
>>(gdb) break foo
>>(gdb) continue
>>         .
>>         .
>>         .
>>warning: Invalid control type in command structure.
>>Segmentation fault (core dumped)
>>
>>What happens is the breakpoint (and the associated 'commands' script) is
>>deleted when first statement of the script is executed.  GDB runs into a
>>core dump when it attempts to execute the remaining (deleted)
>>statements.
>>
>>The patch below detects if the current breakpoint has been deleted and
>>terminates execution of the associated 'commands' script and issues a
>>warning.
>
>   Wouldn't it seem more logical to
>isolate the commands temporarily,
>by resetting the commands field to NULL ?
>
>        headcmd = bs->commands;
>+     bs->commands = NULL;
>+     cmd = headcmd;
>        while (cmd != NULL)
>
>   This would allow to execute the whole
>command sequences to its end.
>   At end of the commands,
>the commands field should be restored if
>the breakpoint has not been distroyed,
>otherwise headcmd should be disposed.
>
>    This seems a much more reasonable action scheme
>to me.
>
>    Otherwise the command parser should at least warn
>if any commands are given past a
>'clear' or 'delete breakpoint-number',
>but this will anyhow not be possible if
>the 'delete' argument is an expression that is not a simple constant.
Just a remark to say that  I realized that my suggestion
also has troubles:
   If you imagine that
b func1
command 1
p arg1
   command 1
     p arg2
   end
end
This command should normally print arg1 the first time func1 is hit,
and arg2 afterwards.
   But my suggestion above doesn't handle this !
Of course it should be easy to add a check to test if
the bs->commands still is NULL before restoring
the old value, but this will not be enough,
because it would probably fail in case of
b func1
command 1
p arg1
   command 1
   end
end

where the command is reset after the first hit.
   A possible solution is to create a dummy bs->commands
and to check if it has been change.




Pierre Muller
Institut Charles Sadron
6,rue Boussingault
F 67083 STRASBOURG CEDEX (France)
mailto:muller@ics.u-strasbg.fr
Phone : (33)-3-88-41-40-07  Fax : (33)-3-88-41-40-99

Re: [RFA] deleting breakpoints inside of 'commands'

[Don Howard]

I've attempted to implement your sugesstion in the patch below.  I need
some feedback from folks who are familiar with the breakpoint code.  I
think it maybe as you say, that a copy of the commands list should be
allocated.  With this patch, gdb no longer dumps core when a command
list deletes it's breakpoint, but commands lists get lost after hitting
the associated breakpoint once or twice.

If someone can explain to me what gdb does with bpstat structures when
the target continues, (and how that may interact with this patch) that
would be most useful.



2001-09-12  Don Howard  <dhoward@redhat.com>

        * breakpoint.c (bpstat_do_actions): Avoid deleting a 'commands'
        list while executing that list.  Thanks to Pierre Muller
        <muller@ics.u-strasbg.fr> for suggesting this implementation.


Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.52
diff -p -u -w -r1.52 breakpoint.c
--- breakpoint.c	2001/08/02 11:58:28	1.52
+++ breakpoint.c	2001/09/13 20:16:18
@@ -1824,7 +1824,24 @@ top:
   breakpoint_proceeded = 0;
   for (; bs != NULL; bs = bs->next)
     {
-      cmd = bs->commands;
+      /* It is possible that the list of commands we are executing
+         includes a command to delete the current breakpoint, which
+         will also delete the current command list (oops!).
+
+         Pierre Muller <muller at cerbere dot u-strasbg dot fr>
+         suggested that the commands list could be disconnected from
+         it's associated breakpoint structure during execution of the
+         list.  Once execution is complete, we check for the
+         non-existence of the breakpoint. If the breakpoint has been
+         deleted, we free the associated command list. */
+
+      struct command_line *head_cmd;
+
+      head_cmd = cmd = bs->commands;
+      bs->commands = NULL;
+      if (bs->breakpoint_at)
+        bs->breakpoint_at->commands = NULL;
+
       while (cmd != NULL)
 	{
 	  execute_control_command (cmd);
@@ -1834,6 +1851,31 @@ top:
 	  else
 	    cmd = cmd->next;
 	}
+
+      /* Detect if the command list has deleted the current
+         breakpoint. */
+      {
+        int deleted = 1;
+        struct breakpoint *b;
+
+        ALL_BREAKPOINTS (b)
+	  if (bs->breakpoint_at == b)
+	    {
+	      deleted = 0;
+	      break;
+	    }
+
+        if (deleted)
+          free_command_lines (&head_cmd);
+        else
+          {
+            bs->commands = head_cmd;
+            if (bs->breakpoint_at)
+              bs->breakpoint_at->commands = head_cmd;
+          }
+      }
+
+
       if (breakpoint_proceeded)
 	/* The inferior is proceeded by the command; bomb out now.
 	   The bpstat chain has been blown away by wait_for_inferior.


-- 
-Don
dhoward@redhat.com
gdb engineering

Re: [RFA] deleting breakpoints inside of 'commands'

[Andrew Cagney]

> +      head_cmd = cmd = bs->commands;
> +      bs->commands = NULL;
> +      if (bs->breakpoint_at)
> +        bs->breakpoint_at->commands = NULL;

don, i've reservations about this.  could it leave the bp in an 
undefined state - one of the commands could abort.

hoewever, i am very ok with the theory behind it - the comand sequence 
that is executed shouldn't be affected by commands that update the 
breakpoint.  perhaphs a duplication would be better?

andrew

Re: [RFA] deleting breakpoints inside of 'commands'

[Don Howard]

Here is an other variation on how to deal with 'commands' scripts that
delete their own breakpoint.  The patch below makes a copy of the
commands list before executing it and deletes the copy when finished.
Comments?


2001-09-14  Don Howard  <dhoward@redhat.com>

	* breakpoint.c (bpstat_do_actions): Avoid deleting a 'commands'
	list while executing that list.  Thanks to Pierre Muller
	<muller@ics.u-strasbg.fr> for suggesting this implementation.

	* cli/cli-script.c (dup_command_lines): New function.

	* defs.h: Added declaration of new function.


Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.52
diff -p -u -w -r1.52 breakpoint.c
--- breakpoint.c	2001/08/02 11:58:28	1.52
+++ breakpoint.c	2001/09/15 00:39:33
@@ -1824,7 +1824,8 @@ top:
   breakpoint_proceeded = 0;
   for (; bs != NULL; bs = bs->next)
     {
-      cmd = bs->commands;
+      cmd = dup_command_lines (bs->commands);
+
       while (cmd != NULL)
 	{
 	  execute_control_command (cmd);
@@ -1834,6 +1835,9 @@ top:
 	  else
 	    cmd = cmd->next;
 	}
+
+      free_command_lines (&cmd);
+
       if (breakpoint_proceeded)
 	/* The inferior is proceeded by the command; bomb out now.
 	   The bpstat chain has been blown away by wait_for_inferior.
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.63
diff -p -u -w -r1.63 defs.h
--- defs.h	2001/09/07 21:33:08	1.63
+++ defs.h	2001/09/15 00:39:34
@@ -837,6 +837,7 @@ struct command_line
 extern struct command_line *read_command_lines (char *, int);

 extern void free_command_lines (struct command_line **);
+extern struct command_line * dup_command_lines (struct command_line *);

 /* To continue the execution commands when running gdb asynchronously.
    A continuation structure contains a pointer to a function to be called
Index: cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.7
diff -p -u -w -r1.7 cli-script.c
--- cli-script.c	2001/06/17 15:16:12	1.7
+++ cli-script.c	2001/09/15 00:39:35
@@ -1005,6 +1005,59 @@ read_command_lines (char *prompt_arg, in
   return (head);
 }

+/* Duplicate a chain of struct command_line's */
+
+struct command_line *
+dup_command_lines (struct command_line *l)
+{
+  struct command_line *dup = NULL;
+  register struct command_line *next = NULL;
+
+
+  for (; l ; l = l->next)
+    {
+      if (next == NULL)
+	{
+	  dup = next = (struct command_line *)
+	    xmalloc (sizeof (struct command_line));
+	}
+      else
+	{
+	  next->next = (struct command_line *)
+	    xmalloc (sizeof (struct command_line));
+
+	  next = next->next;
+	}
+
+
+      if (next == NULL)
+	return NULL;
+
+
+      next->next = NULL;
+      next->line = savestring (l->line, strlen (l->line));
+      next->control_type = l->control_type;
+      next->body_count = l->body_count;
+
+
+      if (l->body_count > 0)
+	{
+	  int i;
+	  struct command_line **blist = l->body_list;
+
+	  next->body_list =
+	    (struct command_line **) xmalloc (sizeof (struct command_line *)
+					      * l->body_count);
+
+	  for (i = 0; i < l->body_count; i++, blist++)
+	    next->body_list[i] = dup_command_lines (*blist);
+	}
+    }
+
+  return dup;
+}
+
+
 /* Free a chain of struct command_line's.  */

 void

-- 
-Don
dhoward@redhat.com
gdb engineering