gdbserver: allow the --debug command line option to take a value

Currently, gdbserver has the following command line options related to
debugging output:

  --debug
  --remote-debug
  --event-loop-debug

This doesn't scale well.  If I want an extra debug component I need to
add another command line flag.

This commit changes --debug to take a list of components.

The currently supported components are: all, threads, remote, and
event-loop.  The 'threads' component represents the debug we currently
get from the --debug option.  And if --debug is used without a
component list then the threads component is assumed as the default.

Currently the threads component actually includes a lot of output that
is not really threads related.  In the future I'd like to split this
up into some new, separate components.  But that is not part of this
commit, or even this series.

The special component 'all' does what you'd expect: enables debug
output from all supported components.

The component list is parsed left to write, and you can prefix a
component with '-' to disable that component, so I can write:

  target> gdbserver --debug=all,-event-loop

to get debug for all components except the event-loop component.

I've removed the existing --remote-debug and --event-loop-debug
command line options, these are equivalent to --debug=remote and
--debug=event-loop respectively, or --debug=remote,event-loop to
enable both components.

In this commit I've only update the command line options, in the next
commit I'll update the monitor commands to support a similar
interface.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>

gdb/NEWS

@@ -38,6 +38,16 @@ set remote thread-options-packet
 show remote thread-options-packet
   Set/show the use of the thread options packet.
 
+* New features in the GDB remote stub, GDBserver
+
+  ** The --remote-debug and --event-loop-debug command line options
+     have been removed.
+
+  ** The --debug command line option now takes an optional comma
+     separated list of components to emit debug for.  The currently
+     supported components are: all, threads, event-loop, and remote.
+     If no components are given then threads is assumed.
+
 * Python API
 
   ** New function gdb.notify_mi(NAME, DATA), that emits custom

gdb/doc/gdb.texinfo

@@ -23674,11 +23674,42 @@ program.  For more information,
 @pxref{--multi Option in Types of Remote Connnections}.
 
 @cindex @option{--debug}, @code{gdbserver} option
-The @option{--debug} option tells @code{gdbserver} to display extra
+The @option{--debug[=option1,option2,@dots{}]} option tells
-status information about the debugging process.
+@code{gdbserver} to display extra diagnostic information about the
-@cindex @option{--remote-debug}, @code{gdbserver} option
+debugging process.  The options (@var{option1}, @var{option2}, etc)
-The @option{--remote-debug} option tells @code{gdbserver} to display
+control for which areas of @code{gdbserver} additional information
-remote protocol debug output.
+will be displayed, possible values are:
+
+@table @code
+@item all
+This enables all available diagnostic output.
+@item threads
+This enables diagnostic output related to threading.  Currently other
+general diagnostic output is included in this category, but this could
+change in future releases of @code{gdbserver}.
+@item event-loop
+This enables event-loop specific diagnostic output.
+@item remote
+This enables diagnostic output related to the transfer of remote
+protocol packets too and from the debugger.
+@end table
+
+@noindent
+If no options are passed to @option{--debug} then this is treated as
+equivalent to @option{--debug=threads}.  This could change in future
+releases of @code{gdbserver}.  The options passed to @option{--debug}
+are processed left to right, and individual options can be prefixed
+with the @kbd{-} (minus) character to disable diagnostic output from
+this area, so it is possible to use:
+
+@smallexample
+  target> gdbserver --debug=all,-event-loop
+@end smallexample
+
+@noindent
+In order to enable all diagnostic output except that for the
+event-loop.
+
 @cindex @option{--debug-file}, @code{gdbserver} option
 @cindex @code{gdbserver}, send all debug output to a single file
 The @option{--debug-file=@var{filename}} option tells @code{gdbserver} to
@@ -50671,16 +50702,27 @@ the program you want to debug.  The syntax is:
 target> gdbserver --multi @var{comm}
 @end smallexample
 
-@item --debug
+@item --debug@r{[}=option1,option2,@dots{}@r{]}
-Instruct @code{gdbserver} to display extra status information about the debugging
+Instruct @code{gdbserver} to display extra status information about
-process.
+the debugging process.  This option is intended for @code{gdbserver}
-This option is intended for @code{gdbserver} development and for bug reports to
+development and for bug reports to the developers.
-the developers.
 
-@item --remote-debug
+Each @var{option} is the name of a component for which debugging
-Instruct @code{gdbserver} to display remote protocol debug output.
+should be enabled.  The list of possible options is @option{all},
-This option is intended for @code{gdbserver} development and for bug reports to
+@option{threads}, @option{event-loop}, @option{remote}.  The special
-the developers.
+option @option{all} enables all components.  The option list is
+processed left to right, and an option can be prefixed with the
+@kbd{-} character to disable output for that component, so you could write:
+
+@smallexample
+target> gdbserver --debug=all,-event-loop
+@end smallexample
+
+@noindent
+to turn on debug output for all components except @option{event-loop}.
+If no options are passed to @option{--debug} then this is treated as
+equivalent to @option{--debug=threads}.  This could change in future
+releases of @code{gdbserver}.
 
 @item --debug-file=@var{filename}
 Instruct @code{gdbserver} to send any debug output to the given @var{filename}.

gdbserver/server.cc

@@ -1457,6 +1457,124 @@ parse_debug_format_options (const char *arg, int is_monitor)
   return std::string ();
 }
 
+/* A wrapper to enable, or disable a debug flag.  These are debug flags
+   that control the debug output from gdbserver, that developers might
+   want, this is not something most end users will need.  */
+
+struct debug_opt
+{
+  /* NAME is the name of this debug option, this should be a simple string
+     containing no whitespace, starting with a letter from isalpha(), and
+     contain only isalnum() characters and '_' underscore and '-' hyphen.
+
+     SETTER is a callback function used to set the debug variable.  This
+     callback will be passed true to enable the debug setting, or false to
+     disable the debug setting.  */
+  debug_opt (const char *name, std::function<void (bool)> setter)
+    : m_name (name),
+      m_setter (setter)
+  {
+    gdb_assert (isalpha (*name));
+  }
+
+  /* Called to enable or disable the debug setting.  */
+  void set (bool enable) const
+  {
+    m_setter (enable);
+  }
+
+  /* Return the name of this debug option.  */
+  const char *name () const
+  { return m_name; }
+
+private:
+  /* The name of this debug option.  */
+  const char *m_name;
+
+  /* The callback to update the debug setting.  */
+  std::function<void (bool)> m_setter;
+};
+
+/* The set of all debug options that gdbserver supports.  These are the
+   options that can be passed to the command line '--debug=...' flag, or to
+   the monitor command 'monitor set debug ...'.  */
+
+static std::vector<debug_opt> all_debug_opt {
+  {"threads", [] (bool enable)
+  {
+    debug_threads = enable;
+  }},
+  {"remote", [] (bool enable)
+  {
+    remote_debug = enable;
+  }},
+  {"event-loop", [] (bool enable)
+  {
+    debug_event_loop = (enable ? debug_event_loop_kind::ALL
+			: debug_event_loop_kind::OFF);
+  }}
+};
+
+/* Parse the options to --debug=...
+
+   OPTIONS is the string of debug components which should be enabled (or
+   disabled), and must not be nullptr.  An empty OPTIONS string is valid,
+   in which case a default set of debug components will be enabled.
+
+   An unknown, or otherwise invalid debug component will result in an
+   exception being thrown.
+
+   OPTIONS can consist of multiple debug component names separated by a
+   comma.  Debugging for each component will be turned on.  The special
+   component 'all' can be used to enable debugging for all components.
+
+   A component can also be prefixed with '-' to disable debugging of that
+   component, so a user might use: '--debug=all,-remote', to enable all
+   debugging, except for the remote (protocol) component.  Components are
+   processed left to write in the OPTIONS list.  */
+
+static void
+parse_debug_options (const char *options)
+{
+  gdb_assert (options != nullptr);
+
+  /* Empty options means the "default" set.  This exists mostly for
+     backwards compatibility with gdbserver's legacy behaviour.  */
+  if (*options == '\0')
+    options = "+threads";
+
+  while (*options != '\0')
+    {
+      const char *end = strchrnul (options, ',');
+
+      bool enable = *options != '-';
+      if (*options == '-' || *options == '+')
+	++options;
+
+      std::string opt (options, end - options);
+
+      if (opt.size () == 0)
+	error ("invalid empty debug option");
+
+      bool is_opt_all = opt == "all";
+
+      bool found = false;
+      for (const auto &debug_opt : all_debug_opt)
+	if (is_opt_all || opt == debug_opt.name ())
+	  {
+	    debug_opt.set (enable);
+	    found = true;
+	    if (!is_opt_all)
+	      break;
+	  }
+
+      if (!found)
+	error ("unknown debug option '%s'", opt.c_str ());
+
+      options = (*end == ',') ? end + 1 : end;
+    }
+}
+
 /* Handle monitor commands not handled by target-specific handlers.  */
 
 static void
@@ -3583,15 +3701,19 @@ gdbserver_usage (FILE *stream)
 	   "\n"
 	   "Debug options:\n"
 	   "\n"
-	   "  --debug               Enable general debugging output.\n"
+	   "  --debug[=OPT1,OPT2,...]\n"
+	   "                        Enable debugging output.\n"
+	   "                          Options:\n"
+	   "                            all, threads, event-loop, remote\n"
+	   "                          With no options, 'threads' is assumed.\n"
+	   "                          Prefix an option with '-' to disable\n"
+	   "                          debugging of that component.\n"
 	   "  --debug-format=OPT1[,OPT2,...]\n"
 	   "                        Specify extra content in debugging output.\n"
 	   "                          Options:\n"
 	   "                            all\n"
 	   "                            none\n"
 	   "                            timestamp\n"
-	   "  --remote-debug        Enable remote protocol debugging output.\n"
-	   "  --event-loop-debug    Enable event loop debugging output.\n"
 	   "  --disable-packet=OPT1[,OPT2,...]\n"
 	   "                        Disable support for RSP packets or features.\n"
 	   "                          Options:\n"
@@ -3870,8 +3992,32 @@ captured_main (int argc, char *argv[])
 	  /* Consume the "--".  */
 	  *next_arg = NULL;
 	}
+      else if (startswith (*next_arg, "--debug="))
+	{
+	  try
+	    {
+	      parse_debug_options ((*next_arg) + sizeof ("--debug=") - 1);
+	    }
+	  catch (const gdb_exception_error &exception)
+	    {
+	      fflush (stdout);
+	      fprintf (stderr, "gdbserver: %s\n", exception.what ());
+	      exit (1);
+	    }
+	}
       else if (strcmp (*next_arg, "--debug") == 0)
-	debug_threads = true;
+	{
+	  try
+	    {
+	      parse_debug_options ("");
+	    }
+	  catch (const gdb_exception_error &exception)
+	    {
+	      fflush (stdout);
+	      fprintf (stderr, "gdbserver: %s\n", exception.what ());
+	      exit (1);
+	    }
+	}
       else if (startswith (*next_arg, "--debug-format="))
 	{
 	  std::string error_msg
@@ -3884,10 +4030,6 @@ captured_main (int argc, char *argv[])
 	      exit (1);
 	    }
 	}
-      else if (strcmp (*next_arg, "--remote-debug") == 0)
-	remote_debug = true;
-      else if (strcmp (*next_arg, "--event-loop-debug") == 0)
-	debug_event_loop = debug_event_loop_kind::ALL;
       else if (startswith (*next_arg, "--debug-file="))
 	debug_set_output ((*next_arg) + sizeof ("--debug-file=") -1);
       else if (strcmp (*next_arg, "--disable-packet") == 0)