[PATCH 0/3] Add -P command line switch for executing Python scripts

[PATCH 0/3] Add -P command line switch for executing Python scripts

[Kevin Buettner]

This three part patch set adds a -P command line switch to GDB.  I
won't say much about it here as it's described in the documentation
patch along with some additional discussion in part 2.  I will note,
however, that the original work was done by Tom Tromey back in 2008. 
It's been in the Fedora and RHEL sources for a goood while too.

Kevin Buettner (3):
  Documentation for Python -P commandline support
  Add -P command line switch for executing Python scripts
  Tests for Python -P commandline support

 gdb/doc/gdb.texinfo                    | 10 +++++
 gdb/main.c                             | 48 +++++++++++++++++++++---
 gdb/python/python.c                    | 49 +++++++++++++++++++++++++
 gdb/python/python.h                    |  2 +
 gdb/testsuite/gdb.python/py-script.c   | 27 ++++++++++++++
 gdb/testsuite/gdb.python/py-script.exp | 51 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-script.py  | 25 +++++++++++++
 7 files changed, 206 insertions(+), 6 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-script.c
 create mode 100644 gdb/testsuite/gdb.python/py-script.exp
 create mode 100644 gdb/testsuite/gdb.python/py-script.py

-- 
2.21.0

[PATCH 1/3] Documentation for Python -P commandline support

[Kevin Buettner]

gdb/doc/ChangeLog:

	* gdb.texinfo (Mode Options): Add documentation for -P.
---
 gdb/doc/gdb.texinfo | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index eddd939869..868a1734de 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1252,6 +1252,16 @@ for remote debugging.
 Run using @var{device} for your program's standard input and output.
 @c FIXME: kingdon thinks there is more to -tty.  Investigate.
 
+@item -P
+@cindex @code{-P}
+@itemx --python
+@cindex @code{--python}
+Change interpretation of command line so that the argument immediately
+following this switch is taken to be the name of a Python script file.
+This option stops option processing; subsequent options are passed to
+Python as @code{sys.argv}.  This option is only available if Python
+scripting support was enabled when @value{GDBN} was configured.
+
 @c resolve the situation of these eventually
 @item -tui
 @cindex @code{--tui}
-- 
2.21.0

Re: [PATCH 1/3] Documentation for Python -P commandline support

[Eli Zaretskii]

> From: Kevin Buettner <kevinb@redhat.com>
> Cc: Kevin Buettner <kevinb@redhat.com>
> Date: Sun, 21 Jul 2019 16:54:25 -0700
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Mode Options): Add documentation for -P.

Thanks.

> +Python as @code{sys.argv}.  This option is only available if Python
> +scripting support was enabled when @value{GDBN} was configured.

I'd say "when @value{GDBN} was built".  It's more clear, I think.

Otherwise, this part is OK, but we should also mention this in NEWS.

Re: [PATCH 1/3] Documentation for Python -P commandline support

[Simon Marchi]

On 2019-07-21 7:54 p.m., Kevin Buettner wrote:
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Mode Options): Add documentation for -P.
> ---
>  gdb/doc/gdb.texinfo | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index eddd939869..868a1734de 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -1252,6 +1252,16 @@ for remote debugging.
>  Run using @var{device} for your program's standard input and output.
>  @c FIXME: kingdon thinks there is more to -tty.  Investigate.
>  
> +@item -P
> +@cindex @code{-P}
> +@itemx --python
> +@cindex @code{--python}
> +Change interpretation of command line so that the argument immediately
> +following this switch is taken to be the name of a Python script file.
> +This option stops option processing; subsequent options are passed to
> +Python as @code{sys.argv}.  This option is only available if Python
> +scripting support was enabled when @value{GDBN} was configured.
> +
>  @c resolve the situation of these eventually
>  @item -tui
>  @cindex @code{--tui}
> 

I think having an example would be helpful to understand how to use this
feature, and how it differs from -x.  It wasn't really obvious to me at first,
so I suspect it won't be for other people less familiar with GDB.

Simon

[PATCH 2/3] Add -P command line switch for executing Python scripts

[Kevin Buettner]

This commit introduces a new command line switch to GDB, a -P /
--python switch which is used for executing a python script.
Encountering -P curtails normal argument processing by GDB; any
remaining arguments (after the script name) are passed to the
script.

This is work that was originally written as part of the Archer
project.  The original/primary author is Tom Tromey, but it has
been maintained for the Fedora project by Jan Kratochvil, Sergio
Durigan Junior, and perhaps others too.

In its original form, and even in the form found within the Fedora
sources, the code implementing the -P switch had several properties
which I found to be surprising:

1) After script execution, exit() was called which (obviously)
   caused GDB to exit.
2) The printing of GDB's banner (copyright info, bug reporting
   instructions, and help instructions) was suppressed.
3) Due to the exit() noted above, GDB's CLI was not (automatically)
   invoked.  If the CLI was desired, it could be run from the Python
   script via use of the gdb.cli method, which was added as part of
   that work.

I've changed things so that exit() is no longer called.  GDB's CLI
will be invoked after script execution.  Also, GDB's banner will be
printed (or not) as normal.  I.e, the banner will be printed unless
the -q switch is specified.

If the script doesn't want the CLI for some reason, it can explicitly
call exit().  It may be the case that the script would be better off
calling a (yet to be written) gdb.exit() method for doing this
instead.  Such a method could make sure that GDB shuts down properly.

gdb/ChangeLog:

	* main.c (python/python.h): Include.
	(captured_main_1): Add option processing and other support for -P
	switch.
	(captured_main): Add help messages for -P.
	* python/python.h (run_python_script): Declare.
	* python/python.c (run_python_script): New function.
---
 gdb/main.c          | 48 ++++++++++++++++++++++++++++++++++++++------
 gdb/python/python.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/python/python.h |  2 ++
 3 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 678c413021..bc8238e3ce 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -33,6 +33,7 @@
 
 #include "interps.h"
 #include "main.h"
+#include "python/python.h"
 #include "source.h"
 #include "cli/cli-cmds.h"
 #include "objfiles.h"
@@ -440,7 +441,7 @@ struct cmdarg
 };
 
 static void
-captured_main_1 (struct captured_main_args *context)
+captured_main_1 (struct captured_main_args *context, bool &python_script)
 {
   int argc = context->argc;
   char **argv = context->argv;
@@ -658,10 +659,14 @@ captured_main_1 (struct captured_main_args *context)
       {"args", no_argument, &set_args, 1},
       {"l", required_argument, 0, 'l'},
       {"return-child-result", no_argument, &return_child_result, 1},
+#if HAVE_PYTHON
+      {"python", no_argument, 0, 'P'},
+      {"P", no_argument, 0, 'P'},
+#endif
       {0, no_argument, 0, 0}
     };
 
-    while (1)
+    while (!python_script)
       {
 	int option_index;
 
@@ -679,6 +684,9 @@ captured_main_1 (struct captured_main_args *context)
 	  case 0:
 	    /* Long option that just sets a flag.  */
 	    break;
+	  case 'P':
+	    python_script = true;
+	    break;
 	  case OPT_SE:
 	    symarg = optarg;
 	    execarg = optarg;
@@ -858,7 +866,20 @@ captured_main_1 (struct captured_main_args *context)
 
   /* Now that gdb_init has created the initial inferior, we're in
      position to set args for that inferior.  */
-  if (set_args)
+  if (python_script)
+    {
+      /* The first argument is a python script to evaluate, and
+	 subsequent arguments are passed to the script for
+	 processing there.  */
+      if (optind >= argc)
+	{
+	  fprintf_unfiltered (gdb_stderr,
+			      _("%s: Python script file name required\n"),
+			      argv[0]);
+	  exit (1);
+	}
+    }
+  else if (set_args)
     {
       /* The remaining options are the command-line options for the
 	 inferior.  The first one is the sym/exec file, and the rest
@@ -1157,9 +1178,14 @@ static void
 captured_main (void *data)
 {
   struct captured_main_args *context = (struct captured_main_args *) data;
+  bool python_script = false;
 
-  captured_main_1 (context);
+  captured_main_1 (context, python_script);
 
+#if HAVE_PYTHON
+  if (python_script)
+    run_python_script (context->argc - optind, &context->argv[optind]);
+#endif
   /* NOTE: cagney/1999-11-07: There is probably no reason for not
      moving this loop and the code found in captured_command_loop()
      into the command_loop() proper.  The main thing holding back that
@@ -1215,9 +1241,12 @@ print_gdb_help (struct ui_file *stream)
   fputs_unfiltered (_("\
 This is the GNU debugger.  Usage:\n\n\
     gdb [options] [executable-file [core-file or process-id]]\n\
-    gdb [options] --args executable-file [inferior-arguments ...]\n\n\
-"), stream);
+    gdb [options] --args executable-file [inferior-arguments ...]\n"), stream);
+#if HAVE_PYTHON
   fputs_unfiltered (_("\
+    gdb [options] [--python|-P] script-file [script-arguments ...]\n"), stream);
+#endif
+  fputs_unfiltered (_("\n\
 Selection of debuggee and its files:\n\n\
   --args             Arguments after executable-file are passed to inferior\n\
   --core=COREFILE    Analyze the core dump COREFILE.\n\
@@ -1260,6 +1289,13 @@ Output and user interface control:\n\n\
 #endif
   fputs_unfiltered (_("\
   --dbx              DBX compatibility mode.\n\
+"), stream);
+#if HAVE_PYTHON
+  fputs_unfiltered (_("\
+  --python, -P       Following argument is Python script file; remaining\n\
+                     arguments are passed to script.\n"), stream);
+#endif
+  fputs_unfiltered (_("\
   -q, --quiet, --silent\n\
                      Do not print version number on startup.\n\n\
 "), stream);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 96bee7c3b0..7bd4d1684f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1276,6 +1276,55 @@ gdbpy_print_stack_or_quit ()
 
 \f
 
+/* Set up the Python argument vector and evaluate a script.  This is
+   used to implement 'gdb -P'.  */
+
+void
+run_python_script (int argc, char **argv)
+{
+  if (!gdb_python_initialized)
+    return;
+
+  gdbpy_enter enter_py (get_current_arch (), current_language);
+
+#if PYTHON_ABI_VERSION < 3
+  PySys_SetArgv (argc - 1, argv + 1);
+#else
+  {
+    wchar_t **wargv = (wchar_t **) alloca (sizeof (*wargv) * (argc + 1));
+    int i;
+
+    for (i = 1; i < argc; i++)
+      {
+	size_t len = mbstowcs (NULL, argv[i], 0);
+
+	if (len == (size_t) -1)
+	  {
+	    fprintf (stderr, "Invalid multibyte argument #%d \"%s\"\n",
+		     i, argv[i]);
+	    exit (1);
+	  }
+	wargv[i] = (wchar_t *) alloca (sizeof (**wargv) * (len + 1));
+	size_t len2 = mbstowcs (wargv[i], argv[i], len + 1);
+	assert (len2 == len);
+      }
+    wargv[argc] = NULL;
+    PySys_SetArgv (argc - 1, wargv + 1);
+  }
+#endif
+
+  FILE *input = fopen (argv[0], "r");
+  if (! input)
+    {
+      fprintf (stderr, "could not open %s: %s\n", argv[0], strerror (errno));
+      exit (1);
+    }
+  PyRun_SimpleFile (input, argv[0]);
+  fclose (input);
+}
+
+\f
+
 /* Return a sequence holding all the Progspaces.  */
 
 static PyObject *
diff --git a/gdb/python/python.h b/gdb/python/python.h
index 10cd90d00e..2af0b2934d 100644
--- a/gdb/python/python.h
+++ b/gdb/python/python.h
@@ -28,4 +28,6 @@ extern const struct extension_language_defn extension_language_python;
 /* Command element for the 'python' command.  */
 extern cmd_list_element *python_cmd_element;
 
+extern void run_python_script (int argc, char **argv);
+
 #endif /* PYTHON_PYTHON_H */
-- 
2.21.0

Re: [PATCH 2/3] Add -P command line switch for executing Python scripts

[Simon Marchi]

Hi Kevin,

That sounds like a very useful feature.  See below for comments.

On 2019-07-21 7:54 p.m., Kevin Buettner wrote:
> This commit introduces a new command line switch to GDB, a -P /
> --python switch which is used for executing a python script.
> Encountering -P curtails normal argument processing by GDB; any
> remaining arguments (after the script name) are passed to the
> script.
> 
> This is work that was originally written as part of the Archer
> project.  The original/primary author is Tom Tromey, but it has
> been maintained for the Fedora project by Jan Kratochvil, Sergio
> Durigan Junior, and perhaps others too.
> 
> In its original form, and even in the form found within the Fedora
> sources, the code implementing the -P switch had several properties
> which I found to be surprising:
> 
> 1) After script execution, exit() was called which (obviously)
>    caused GDB to exit.
> 2) The printing of GDB's banner (copyright info, bug reporting
>    instructions, and help instructions) was suppressed.
> 3) Due to the exit() noted above, GDB's CLI was not (automatically)
>    invoked.  If the CLI was desired, it could be run from the Python
>    script via use of the gdb.cli method, which was added as part of
>    that work.
> 
> I've changed things so that exit() is no longer called.  GDB's CLI
> will be invoked after script execution.  Also, GDB's banner will be
> printed (or not) as normal.  I.e, the banner will be printed unless
> the -q switch is specified.
> 
> If the script doesn't want the CLI for some reason, it can explicitly
> call exit().  It may be the case that the script would be better off
> calling a (yet to be written) gdb.exit() method for doing this
> instead.  Such a method could make sure that GDB shuts down properly.

1. Since it's closely related to "-x", it would be nice if it was possible
to use -P with --batch, just like we can with -x.  For example, with this
Python script:

import sys
print('args:', sys.argv)

$ ./gdb --data-directory=data-directory --batch -x test.py
args: ['']
$ ./gdb --data-directory=data-directory --batch -P test.py
$

If you want your execution to be completely unattended, it sounds more fragile
to rely on calling exit at the end of your script.  If there's an error and it
ends with an exception, your exit won't be called.  So I'd prefer if --batch
worked for this case.

2. When using "--batch -x" with a gdb script, gdb's exit code will reflect if we
sourced the script successfully.  Unfortunately, it doesn't work the same way with
"--batch -x" and a Python script.  I think it would be useful if it did (a Python
script would be considered as failing if it ends by raising an exception).  If we
make that work, then it would be nice for "--batch -P" to work the same way.  To
be clear, since it's not an existing feature even for -x, I am not asking you to
implement that as part of this patch.

3. When you run a program under the standalone Python interpreter, sys.argv[0]
is the script name.  Perhaps people will expect it to be the same here?  It
would be confusing if for GDB Python scripts, it's different.

Also, the setting of sys.argv remains for the rest of the Python interpreter's
lifetime:

$ ./gdb --data-directory=data-directory -q -P test.py Hello
args: ['Hello']
(gdb) pi
>>> import sys
>>> sys.argv
['Hello']

I think it's not very likely that people's script would rely on the fact that
sys.argv was initially empty, but if we could reset sys.argv to [''] when we
are done executing that Python script, it would reduce the chances that we break
something.

> gdb/ChangeLog:
> 
> 	* main.c (python/python.h): Include.
> 	(captured_main_1): Add option processing and other support for -P
> 	switch.
> 	(captured_main): Add help messages for -P.
> 	* python/python.h (run_python_script): Declare.
> 	* python/python.c (run_python_script): New function.
> ---
>  gdb/main.c          | 48 ++++++++++++++++++++++++++++++++++++++------
>  gdb/python/python.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
>  gdb/python/python.h |  2 ++
>  3 files changed, 93 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/main.c b/gdb/main.c
> index 678c413021..bc8238e3ce 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -33,6 +33,7 @@
>  
>  #include "interps.h"
>  #include "main.h"
> +#include "python/python.h"
>  #include "source.h"
>  #include "cli/cli-cmds.h"
>  #include "objfiles.h"
> @@ -440,7 +441,7 @@ struct cmdarg
>  };
>  
>  static void
> -captured_main_1 (struct captured_main_args *context)
> +captured_main_1 (struct captured_main_args *context, bool &python_script)
>  {
>    int argc = context->argc;
>    char **argv = context->argv;
> @@ -658,10 +659,14 @@ captured_main_1 (struct captured_main_args *context)
>        {"args", no_argument, &set_args, 1},
>        {"l", required_argument, 0, 'l'},
>        {"return-child-result", no_argument, &return_child_result, 1},
> +#if HAVE_PYTHON
> +      {"python", no_argument, 0, 'P'},
> +      {"P", no_argument, 0, 'P'},
> +#endif
>        {0, no_argument, 0, 0}
>      };
>  
> -    while (1)
> +    while (!python_script)
>        {
>  	int option_index;
> > @@ -679,6 +684,9 @@ captured_main_1 (struct captured_main_args *context)
>  	  case 0:
>  	    /* Long option that just sets a flag.  */
>  	    break;
> +	  case 'P':
> +	    python_script = true;
> +	    break;
>  	  case OPT_SE:
>  	    symarg = optarg;
>  	    execarg = optarg;
> @@ -858,7 +866,20 @@ captured_main_1 (struct captured_main_args *context)
>  
>    /* Now that gdb_init has created the initial inferior, we're in
>       position to set args for that inferior.  */
> -  if (set_args)
> +  if (python_script)
> +    {
> +      /* The first argument is a python script to evaluate, and
> +	 subsequent arguments are passed to the script for
> +	 processing there.  */
> +      if (optind >= argc)
> +	{
> +	  fprintf_unfiltered (gdb_stderr,
> +			      _("%s: Python script file name required\n"),
> +			      argv[0]);
> +	  exit (1);
> +	}
> +    }
> +  else if (set_args)
>      {
>        /* The remaining options are the command-line options for the
>  	 inferior.  The first one is the sym/exec file, and the rest
> @@ -1157,9 +1178,14 @@ static void
>  captured_main (void *data)
>  {
>    struct captured_main_args *context = (struct captured_main_args *) data;
> +  bool python_script = false;
>  
> -  captured_main_1 (context);
> +  captured_main_1 (context, python_script);

I know it's a heated debate (well, not really but still), but I would prefer the
reference to the variable was passed as a pointer, &python_script.  I really
find it confusing to have it this way, since it looks like you are just constantly
passing false to the function (so the variable looks unnecessary).

>  
> +#if HAVE_PYTHON
> +  if (python_script)
> +    run_python_script (context->argc - optind, &context->argv[optind]);
> +#endif
>    /* NOTE: cagney/1999-11-07: There is probably no reason for not
>       moving this loop and the code found in captured_command_loop()
>       into the command_loop() proper.  The main thing holding back that
> @@ -1215,9 +1241,12 @@ print_gdb_help (struct ui_file *stream)
>    fputs_unfiltered (_("\
>  This is the GNU debugger.  Usage:\n\n\
>      gdb [options] [executable-file [core-file or process-id]]\n\
> -    gdb [options] --args executable-file [inferior-arguments ...]\n\n\
> -"), stream);
> +    gdb [options] --args executable-file [inferior-arguments ...]\n"), stream);
> +#if HAVE_PYTHON
>    fputs_unfiltered (_("\
> +    gdb [options] [--python|-P] script-file [script-arguments ...]\n"), stream);
> +#endif
> +  fputs_unfiltered (_("\n\
>  Selection of debuggee and its files:\n\n\
>    --args             Arguments after executable-file are passed to inferior\n\
>    --core=COREFILE    Analyze the core dump COREFILE.\n\
> @@ -1260,6 +1289,13 @@ Output and user interface control:\n\n\
>  #endif
>    fputs_unfiltered (_("\
>    --dbx              DBX compatibility mode.\n\
> +"), stream);
> +#if HAVE_PYTHON
> +  fputs_unfiltered (_("\
> +  --python, -P       Following argument is Python script file; remaining\n\
> +                     arguments are passed to script.\n"), stream);
> +#endif
> +  fputs_unfiltered (_("\
>    -q, --quiet, --silent\n\
>                       Do not print version number on startup.\n\n\
>  "), stream);
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index 96bee7c3b0..7bd4d1684f 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -1276,6 +1276,55 @@ gdbpy_print_stack_or_quit ()
>  
>  \f
>  
> +/* Set up the Python argument vector and evaluate a script.  This is
> +   used to implement 'gdb -P'.  */
> +
> +void
> +run_python_script (int argc, char **argv)

Even though the surrounding code is not like that, I would suggest following
our current conventions, putting /* See python.h.  */ here and put the doc
in the .h.

> +{
> +  if (!gdb_python_initialized)
> +    return;
> +
> +  gdbpy_enter enter_py (get_current_arch (), current_language);
> +
> +#if PYTHON_ABI_VERSION < 3

We is the IS_PY3K macro throughout.

> +  PySys_SetArgv (argc - 1, argv + 1);
> +#else
> +  {
> +    wchar_t **wargv = (wchar_t **) alloca (sizeof (*wargv) * (argc + 1));

I'd suggest using XALLOCAVEC:

  XALLOCAVEC (wchar_t *, argc + 1)

> +    int i;

You can inline this declaration in the for loop.

> +
> +    for (i = 1; i < argc; i++)
> +      {
> +	size_t len = mbstowcs (NULL, argv[i], 0);
> +
> +	if (len == (size_t) -1)
> +	  {
> +	    fprintf (stderr, "Invalid multibyte argument #%d \"%s\"\n",
> +		     i, argv[i]);
> +	    exit (1);
> +	  }

I think it would be more gdb-ish to call error () instead of plain exiting.

> +	wargv[i] = (wchar_t *) alloca (sizeof (**wargv) * (len + 1));

Suggest using XALLOCAVEC.  Or even dynamic allocation to be safer, given that
args can be actually quite long for alloca.

> +	size_t len2 = mbstowcs (wargv[i], argv[i], len + 1);
> +	assert (len2 == len);
> +      }
> +    wargv[argc] = NULL;
> +    PySys_SetArgv (argc - 1, wargv + 1);
> +  }
> +#endif
> +
> +  FILE *input = fopen (argv[0], "r");

Do we want to use gdb_fopen_cloexec?

> +  if (! input)

if (input == nullptr)

> +    {
> +      fprintf (stderr, "could not open %s: %s\n", argv[0], strerror (errno));
> +      exit (1);

error () instead of exiting?

> +    }> +  PyRun_SimpleFile (input, argv[0]);
> +  fclose (input);

If using gdb_fopen_cloexec, the file will get closed automatically as we leave the
scope, reducing chances that we leak an open file.

> +}
> +
> +\f
> +
>  /* Return a sequence holding all the Progspaces.  */
>  
>  static PyObject *
> diff --git a/gdb/python/python.h b/gdb/python/python.h
> index 10cd90d00e..2af0b2934d 100644
> --- a/gdb/python/python.h
> +++ b/gdb/python/python.h
> @@ -28,4 +28,6 @@ extern const struct extension_language_defn extension_language_python;
>  /* Command element for the 'python' command.  */
>  extern cmd_list_element *python_cmd_element;
>  
> +extern void run_python_script (int argc, char **argv);
> +
>  #endif /* PYTHON_PYTHON_H */
> 

Simon

Re: [PATCH 2/3] Add -P command line switch for executing Python scripts

[Tom Tromey]

>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:

Kevin> This commit introduces a new command line switch to GDB, a -P /
Kevin> --python switch which is used for executing a python script.
Kevin> Encountering -P curtails normal argument processing by GDB; any
Kevin> remaining arguments (after the script name) are passed to the
Kevin> script.

Kevin> 1) After script execution, exit() was called which (obviously)
Kevin>    caused GDB to exit.
Kevin> 2) The printing of GDB's banner (copyright info, bug reporting
Kevin>    instructions, and help instructions) was suppressed.
Kevin> 3) Due to the exit() noted above, GDB's CLI was not (automatically)
Kevin>    invoked.  If the CLI was desired, it could be run from the Python
Kevin>    script via use of the gdb.cli method, which was added as part of
Kevin>    that work.

Kevin> I've changed things so that exit() is no longer called.  GDB's CLI
Kevin> will be invoked after script execution.  Also, GDB's banner will be
Kevin> printed (or not) as normal.  I.e, the banner will be printed unless
Kevin> the -q switch is specified.

Kevin> If the script doesn't want the CLI for some reason, it can explicitly
Kevin> call exit().  It may be the case that the script would be better off
Kevin> calling a (yet to be written) gdb.exit() method for doing this
Kevin> instead.  Such a method could make sure that GDB shuts down properly.

The intent of -P was to provide a way to run gdb in a "Python script"
mode.  I think this changes defeat this goal.

For (2), printing the banner is just not great for an interpreter.  The
default for interpreters should be silence, with the script being
interpreted choosing what to emit.  "-q" isn't sufficient because it is
common for #! handling to only allow a single argument.

I think if -P is going to just mean "start gdb, but interpret some
Python script at startup", then it might as well not exist -- an
invocation like '-ex "source blah.py"' is just as good, and can be used
on older gdbs as well.

(1) and (3) seem linked.  Here the idea was to have a way to script gdb
where the command line was completely optional and under control of the
script.

Maybe -P isn't the right vehicle for this.  Later on, I was looking at
having a "Python" gdb interpreter -- not to actually interpret gdb
input, but to at least control gdb output.  And, I looked into compiling
gdb PIE so that one could just "import gdb" from an ordinary Python
interpreter.

thanks,
Tom

Re: [PATCH 2/3] Add -P command line switch for executing Python scripts

[Kevin Buettner]

On Tue, 23 Jul 2019 14:25:06 -0600
Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:  
> 
> Kevin> This commit introduces a new command line switch to GDB, a -P /
> Kevin> --python switch which is used for executing a python script.
> Kevin> Encountering -P curtails normal argument processing by GDB; any
> Kevin> remaining arguments (after the script name) are passed to the
> Kevin> script.  
> 
> Kevin> 1) After script execution, exit() was called which (obviously)
> Kevin>    caused GDB to exit.
> Kevin> 2) The printing of GDB's banner (copyright info, bug reporting
> Kevin>    instructions, and help instructions) was suppressed.
> Kevin> 3) Due to the exit() noted above, GDB's CLI was not (automatically)
> Kevin>    invoked.  If the CLI was desired, it could be run from the Python
> Kevin>    script via use of the gdb.cli method, which was added as part of
> Kevin>    that work.  
> 
> Kevin> I've changed things so that exit() is no longer called.  GDB's CLI
> Kevin> will be invoked after script execution.  Also, GDB's banner will be
> Kevin> printed (or not) as normal.  I.e, the banner will be printed unless
> Kevin> the -q switch is specified.  
> 
> Kevin> If the script doesn't want the CLI for some reason, it can explicitly
> Kevin> call exit().  It may be the case that the script would be better off
> Kevin> calling a (yet to be written) gdb.exit() method for doing this
> Kevin> instead.  Such a method could make sure that GDB shuts down properly.  
> 
> The intent of -P was to provide a way to run gdb in a "Python script"
> mode.  I think this changes defeat this goal.
> 
> For (2), printing the banner is just not great for an interpreter.  The
> default for interpreters should be silence, with the script being
> interpreted choosing what to emit.  "-q" isn't sufficient because it is
> common for #! handling to only allow a single argument.
> 
> I think if -P is going to just mean "start gdb, but interpret some
> Python script at startup", then it might as well not exist -- an
> invocation like '-ex "source blah.py"' is just as good, and can be used
> on older gdbs as well.
> 
> (1) and (3) seem linked.  Here the idea was to have a way to script gdb
> where the command line was completely optional and under control of the
> script.
> 
> Maybe -P isn't the right vehicle for this.  Later on, I was looking at
> having a "Python" gdb interpreter -- not to actually interpret gdb
> input, but to at least control gdb output.  And, I looked into compiling
> gdb PIE so that one could just "import gdb" from an ordinary Python
> interpreter.

Thanks for that critique / explanation.

I'm going to mull this over some more...

Kevin

Re: [PATCH 2/3] Add -P command line switch for executing Python scripts

[Pedro Alves]

On 7/23/19 9:25 PM, Tom Tromey wrote:
>>>>>> "Kevin" == Kevin Buettner <kevinb@redhat.com> writes:
> 
> Kevin> This commit introduces a new command line switch to GDB, a -P /
> Kevin> --python switch which is used for executing a python script.
> Kevin> Encountering -P curtails normal argument processing by GDB; any
> Kevin> remaining arguments (after the script name) are passed to the
> Kevin> script.
> 
> Kevin> 1) After script execution, exit() was called which (obviously)
> Kevin>    caused GDB to exit.
> Kevin> 2) The printing of GDB's banner (copyright info, bug reporting
> Kevin>    instructions, and help instructions) was suppressed.
> Kevin> 3) Due to the exit() noted above, GDB's CLI was not (automatically)
> Kevin>    invoked.  If the CLI was desired, it could be run from the Python
> Kevin>    script via use of the gdb.cli method, which was added as part of
> Kevin>    that work.
> 
> Kevin> I've changed things so that exit() is no longer called.  GDB's CLI
> Kevin> will be invoked after script execution.  Also, GDB's banner will be
> Kevin> printed (or not) as normal.  I.e, the banner will be printed unless
> Kevin> the -q switch is specified.
> 
> Kevin> If the script doesn't want the CLI for some reason, it can explicitly
> Kevin> call exit().  It may be the case that the script would be better off
> Kevin> calling a (yet to be written) gdb.exit() method for doing this
> Kevin> instead.  Such a method could make sure that GDB shuts down properly.
> 
> The intent of -P was to provide a way to run gdb in a "Python script"
> mode.  I think this changes defeat this goal.
> 
> For (2), printing the banner is just not great for an interpreter.  The
> default for interpreters should be silence, with the script being
> interpreted choosing what to emit.  "-q" isn't sufficient because it is
> common for #! handling to only allow a single argument.

I'm probably being dense, but I don't really understand this remark.
Can you expand on what does
 "#! handling to only allow a single argument."
mean?

> 
> I think if -P is going to just mean "start gdb, but interpret some
> Python script at startup", then it might as well not exist -- an
> invocation like '-ex "source blah.py"' is just as good, and can be used
> on older gdbs as well.

A selling point that I recalled about the patch when we started
discussing it internally was that you can pass arguments directly to
the script from the command line, like:

 $ gdb -P blah.py py-arg1 py-arg2

I wonder if people find that feature itself useful, regardless of
the original motivation for the patch?

> (1) and (3) seem linked.  Here the idea was to have a way to script gdb
> where the command line was completely optional and under control of the
> script.
> 
> Maybe -P isn't the right vehicle for this.  Later on, I was looking at
> having a "Python" gdb interpreter -- not to actually interpret gdb
> input, but to at least control gdb output.  And, I looked into compiling
> gdb PIE so that one could just "import gdb" from an ordinary Python
> interpreter.

FYI, and FWIW, we ended up dropping this patch from Fedora GDB.

Thanks,
Pedro Alves

[PATCH 3/3] Tests for Python -P commandline support

[Kevin Buettner]

gdb/testsuite/ChangeLog:

	* gdb.python/py-script.c: New file.
	* gdb.python/py-script.exp: New file.
	* gdb.python/py-script.py: New file.
---
 gdb/testsuite/gdb.python/py-script.c   | 27 ++++++++++++++
 gdb/testsuite/gdb.python/py-script.exp | 51 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.python/py-script.py  | 25 +++++++++++++
 3 files changed, 103 insertions(+)
 create mode 100644 gdb/testsuite/gdb.python/py-script.c
 create mode 100644 gdb/testsuite/gdb.python/py-script.exp
 create mode 100644 gdb/testsuite/gdb.python/py-script.py

diff --git a/gdb/testsuite/gdb.python/py-script.c b/gdb/testsuite/gdb.python/py-script.c
new file mode 100644
index 0000000000..bff3a5edf5
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-script.c
@@ -0,0 +1,27 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This program is intended to be started outside of gdb, and then
+   attached to by gdb.  It loops for a while, but not forever.  */
+
+#include <unistd.h>
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-script.exp b/gdb/testsuite/gdb.python/py-script.exp
new file mode 100644
index 0000000000..fc5b87cb34
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-script.exp
@@ -0,0 +1,51 @@
+# Copyright (C) 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests the loading
+# and execution of scripts specified on the GDB command line.
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " -P $pyfile"
+    clean_restart ${binfile}
+}
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint"
+    continue
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+gdb_test "python print(gdb.script_var1)" "script var 1"
+gdb_test "python print(gdb.script_var2)" "script var 2"
+
+save_vars { GDBFLAGS } {
+    append GDBFLAGS " -P $pyfile first_arg second_arg third_arg"
+    clean_restart ${binfile}
+}
+
+gdb_test "python print(gdb.script_args\[0\])" "first_arg"
+gdb_test "python print(gdb.script_args\[1\])" "second_arg"
+gdb_test "python print(gdb.script_args\[2\])" "third_arg"
diff --git a/gdb/testsuite/gdb.python/py-script.py b/gdb/testsuite/gdb.python/py-script.py
new file mode 100644
index 0000000000..4bbd5ba593
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-script.py
@@ -0,0 +1,25 @@
+# Copyright (C) 2019 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests the loading
+# and execution of a Python script specified on the GDB command line.
+
+import gdb
+import sys
+
+gdb.script_var1 = "script var 1"
+gdb.script_var2 = "script var 2"
+
+gdb.script_args = list(sys.argv)
-- 
2.21.0