Re: [RFA_v4 1/8] Add helper functions parse_flags and parse_flags_qcs

[Philippe Waroquiers <philippe.waroquiers@skynet.be>]

On Tue, 2018-07-31 at 09:40 -0600, Tom Tromey wrote:
> > > > > > "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
> > > I also found this bug this weekend, while trying out -fsanitize=address.
> > > 
> > > I have a patch for this one.  I haven't written the ChangeLog yet but I
> > > will try to do it as soon as possible.
> > > 
> > > Actually I have patches to make gdb nearly -fsanitize=address clean; or
> > > at least, the ordinary test suite on my machine is down to 1 failure
> > > (there are some additional gdbserver failures in bugzilla that I haven't
> > > looked at yet).  My series also addresses much of -fsanitize=undefined
> > > as well.
> 
> Joel> Very nice!
> 
> You may have to wait a bit longer because the buildbot is telling me
> this patch is no good.  I'll try to debug it tonight.

As far as I can see, this problem is a regression that appeared in gdb 8.1,
but which was made (more) visible by the 'parse_flags' patch in (future) 8.3.
At least valgrind + gdb 8.0 does not give a problem with the small reproducer 
  command 1 2
  end
while it gives an error with gdb 8.1.

We also have a (small) functional regression:
with gdb 8.0, it was possible to remove all commands of a set of
breapoints by doing the above 'command 1 2/end'.

From 8.1 onwards, when giving such empty command list, gdb asks
the list of command for each breakpoint, instead of asking it once.

The below patch seems to solve the memory corruption (at least
for the simple case), but does not solve the functional regression that
appeared in 8.1.

I am wondering if the correct solution would not be to avoid
having input lines memory being managed 'manually' like it is now,
as having the 'input const char* arg' disappearing 'under the carpet'
is quite tricky, and we might have other places where a previous
line of input must be kept alive, while new lines of input have
to be read.

Philippe

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6b6e1f6c25..dabd81e138 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1222,6 +1222,9 @@ commands_command_1 (const char *arg, int from_tty,
 
   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.  */
   if (arg == NULL || !*arg)
     {
       if (breakpoint_count - prev_breakpoint_count > 1)
@@ -1231,6 +1234,11 @@ 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)