[RFA 1/2] Fix regressions for multi breakpoints command line setting/clearing

[Philippe Waroquiers]

When setting commands for several breakpoints
(such as with  'commands 1 2'),
the '1 2' is passed to commands_command_1 as const char* arg.
This arg can however be freed, as this is in a just read input line
that might be freed by the call to read_command_lines.
This patch fixes the problem by ensuring that arg is always
a locally allocated string managed via the std::string new_arg.
Note that this was the logic before the regression was introduced :
the use after free was introduced when (partially) undoing the patch
done by Pedro in 896b6bda6904765f36692d76a37b99c0412ca9ae.

Note that such problem could also (or should?) be fixed by reworking
the way read_command_lines manages the memory of input lines, so as
to solve this globally, and not at all places where possibly a
'being handled' line of input might be re-allocated.
Tom is looking at this, but in the meantime,
this patch just goes back to the previous way to avoid the
error (and be able to have the regression tests for the
functional regression working).
Without the patch, the test fails the following way:
  ...
  commands 2 3
  Type commands for breakpoint(s) 2 3, one per line.
  End with a line saying just "end".
  >  print 1234321
  >end
  No breakpoint number 4321.
  ...
(and under valgrind, the above reports access to freed memory).

breakpoint.c is also modified to fix the regression introduced
when clearing commands of several breakpoint by giving an empty
list of commands, by just typing "end".
GDB should read an empty list of command once, but it reads
it for each breakpoint, as an empty list of command is NULL.

gdb/ChangeLog

2018-08-02  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* breakpoint.c (commands_command_1): New boolean cmd_read
	to detect cmd was already read. Always allocate a new_arg
	c_str to avoid accessing arg after some calls to
	read_command_line as this can free arg memory.
---
 gdb/breakpoint.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6b6e1f6c25..7761bdb496 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1219,9 +1219,17 @@ commands_command_1 (const char *arg, int from_tty,
 		    struct command_line *control)
 {
   counted_command_line cmd;
+  /* cmd_read will be true once we have read cmd.  Note that cmd might still be
+     NULL after the call to read_command_lines if the user provides an empty
+     list of command by just typing "end".  */
+  bool cmd_read = false;
 
   std::string new_arg;
 
+  /* arg might be an input line that might be released when reading
+     new input lines for the list of commands.  So, build a new arg
+     to keep the input alive during the map_breakpoint_numbers call,
+     even if the new arg is just a copy of arg.  */
   if (arg == NULL || !*arg)
     {
       if (breakpoint_count - prev_breakpoint_count > 1)
@@ -1231,12 +1239,18 @@ commands_command_1 (const char *arg, int from_tty,
 	new_arg = string_printf ("%d", breakpoint_count);
       arg = new_arg.c_str ();
     }
+  else
+    {
+      new_arg = arg;
+      arg = new_arg.c_str ();
+    }
 
   map_breakpoint_numbers
     (arg, [&] (breakpoint *b)
      {
-       if (cmd == NULL)
+       if (!cmd_read)
 	 {
+	   gdb_assert (cmd == NULL);
 	   if (control != NULL)
 	     cmd = control->body_list_0;
 	   else
@@ -1256,6 +1270,7 @@ commands_command_1 (const char *arg, int from_tty,
 
 	       cmd = read_command_lines (str.c_str (), from_tty, 1, validator);
 	     }
+	   cmd_read = true;
 	 }
 
        /* If a breakpoint was on the list more than once, we don't need to
-- 
2.18.0