Make the "python" command resemble the standard Python interpreter

Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

Hi,

I'd like to contribute a patch to improve the "python" command by making it resemble the standard Python interpreter in behavior.

For "python" with arguments, this prints the results of expressions, e.g., "python 1 + 2" will print "3" (previously, it will not print anything).

For "python" without arguments, this uses Python's built-in interactive loop (PyRun_InteractiveLoop) so that individual statements/expressions are immediately evaluated and results of expressions are printed. It also hooks GDB's readline wrappers (command_line_input) to Python so that line editing and history editing works. Additionally, Python's standard readline module is stubbed out because it conflicts with GDB's use of readline.

This patch depends on a patch to handle SIGINT that I previously submitted (http://sourceware.org/bugzilla/show_bug.cgi?id=13265) to handle SIGINT in the interactive loop.

Thanks!

Yit
January 10, 2012


[-- Attachment #2: python-interactive --]
[-- Type: application/octet-stream, Size: 10984 bytes --]

# HG changeset patch
# Parent 9f7d0a74b37f443edf5a7446fcadc806aea32f21
Make the "python" command resemble the standard Python interpreter.
- Use Py_single_input mode to interpret "python" with arguments, so that results of expressions are printed.
- Use Python's built-in interactive loop for "python" without arguments.
- Hook PyOS_ReadlineFunctionPointer to command_line_input to provide readline support.
- Install a dummy readline module to prevent conflicts that arise from using the standard Python readline module.

gdb/ChangeLog:

2012-01-10  Khoo Yit Phang <khooyp@cs.umd.edu>

	Make the "python" command resemble the standard Python interpreter.
	* Makefile.in (SUBDIR_PYTHON_OBS): Add py-gdb-readline.o.
	(SUBDIR_PYTHON_SRCS): Add python/py-gdb-readline.c.
	(py-gdb-readline.o): Add rule to compile python/py-gdb-readline.c.
	* python/py-gdb-readline.c: New file.
	* python/python-internal.h (gdbpy_initialize_gdb_readline): New
	prototype.
	* python/python.c (eval_python_command): New function.
	(eval_python_from_control_command): Call eval_python_command instead
	of PyRun_SimpleString.
	(python_command): For "python" with arguments, call
	eval_python_command.  For "python" without arguments, call
	PyRun_InteractiveLoop if from_tty is true, other call
	execute_control_command_untraced.

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -270,6 +270,7 @@
 #
 SUBDIR_PYTHON_OBS = \
 	python.o \
+	py-gdb-readline.o \
 	py-auto-load.o \
 	py-block.o \
 	py-bpevent.o \
@@ -302,6 +303,7 @@
 
 SUBDIR_PYTHON_SRCS = \
 	python/python.c \
+	python/py-gdb-readline.c \
 	python/py-auto-load.c \
 	python/py-block.c \
 	python/py-bpevent.c \
@@ -2019,6 +2021,10 @@
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/python.c
 	$(POSTCOMPILE)
 
+py-gdb-readline.o: $(srcdir)/python/py-gdb-readline.c
+	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-gdb-readline.c
+	$(POSTCOMPILE)
+
 py-auto-load.o: $(srcdir)/python/py-auto-load.c
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-auto-load.c
 	$(POSTCOMPILE)
diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
new file mode 100644
--- /dev/null
+++ b/gdb/python/py-gdb-readline.c
@@ -0,0 +1,168 @@
+/* Readline support for Python.
+
+   Copyright (C) 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#include "defs.h"
+#include "python-internal.h"
+#include "../exceptions.h"
+#include "../top.h"
+
+#include <stddef.h>
+#include <string.h>
+
+/* Python's readline module conflicts with GDB's use of readline
+   since readline is not reentrant. To prevent conflicts, a dummy
+   readline module is set up which simply fails for any readline
+   method. */
+
+/* TODO: Ideally, this module should implement a reentrant wrapper
+   to readline, so that the readline module can be used without
+   conflicting with GDB. */
+
+static PyObject *
+unimplemented_command (PyObject *self, PyObject *args, PyObject *kw)
+{
+  PyErr_SetString (PyExc_NotImplementedError,
+		   "readline module disabled under GDB");
+  return NULL;
+}
+
+#define UNIMPLEMENTED(cmd) \
+  { cmd, (PyCFunction)unimplemented_command, \
+    METH_VARARGS | METH_KEYWORDS, "" }
+
+static PyMethodDef GdbReadlineMethods[] =
+{
+  /* readline module methods as of Python 2.7.2. */
+  UNIMPLEMENTED("add_history"),
+  UNIMPLEMENTED("clear_history"),
+  UNIMPLEMENTED("get_begidx"),
+  UNIMPLEMENTED("get_completer"),
+  UNIMPLEMENTED("get_completer_delims"),
+  UNIMPLEMENTED("get_completion_type"),
+  UNIMPLEMENTED("get_current_history_length"),
+  UNIMPLEMENTED("get_endidx"),
+  UNIMPLEMENTED("get_history_item"),
+  UNIMPLEMENTED("get_history_length"),
+  UNIMPLEMENTED("get_line_buffer"),
+  UNIMPLEMENTED("insert_text"),
+  UNIMPLEMENTED("parse_and_bind"),
+  UNIMPLEMENTED("read_history_file"),
+  UNIMPLEMENTED("read_init_file"),
+  UNIMPLEMENTED("redisplay"),
+  UNIMPLEMENTED("remove_history_item"),
+  UNIMPLEMENTED("replace_history_item"),
+  UNIMPLEMENTED("set_completer"),
+  UNIMPLEMENTED("set_completer_delims"),
+  UNIMPLEMENTED("set_completion_display_matches_hook"),
+  UNIMPLEMENTED("set_history_length"),
+  UNIMPLEMENTED("set_pre_input_hook"),
+  UNIMPLEMENTED("set_startup_hook"),
+  UNIMPLEMENTED("write_history_file"),
+  { NULL, NULL, 0, NULL }
+};
+
+/* Readline function suitable for PyOS_ReadlineFunctionPointer, which
+   is used for Python's interactive parser and raw_input. In both
+   cases, sys_stdin and sys_stdout are always stdin and stdout
+   respectively, as far as I can tell; they are ignored and
+   command_line_input is used instead. */
+
+static char *
+gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
+			char *prompt)
+{
+  int n;
+  char *p = NULL, *p_start, *p_end, *q;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
+      p = command_line_input (prompt, 0, "python");
+      do_cleanups (cleanup);
+    }
+
+  /* Detect Ctrl-C and treat as KeyboardInterrupt. */
+  if (except.reason == RETURN_QUIT)
+    return NULL;
+
+  /* Handle errors by raising Python exceptions. */
+  if (except.reason < 0)
+    {
+      /* The thread state is nulled during gdbpy_readline_wrapper,
+	 with the original value saved in the following undocumented
+	 variable (see Python's Parser/myreadline.c and
+	 Modules/readline.c). */
+      PyEval_RestoreThread (_PyOS_ReadlineTState);
+      gdbpy_convert_exception (except);
+      PyEval_SaveThread ();
+      return NULL;
+    }
+
+  /* Detect EOF (Ctrl-D). */
+  if (p == NULL)
+    {
+      q = PyMem_Malloc (1);
+      if (q != NULL)
+	q[0] = '\0';
+      return q;
+    }
+
+  n = strlen (p);
+
+  /* Detect "end" like process_next_line in cli/cli-script.c. */
+  /* Strip trailing whitespace.  */
+  p_end = p + n;
+  while (p_end > p && (p_end[-1] == ' ' || p_end[-1] == '\t'))
+    p_end--;
+
+  /* Strip leading whitespace.  */
+  p_start = p;
+  while (p_start < p_end && (*p_start == ' ' || *p_start == '\t'))
+    p_start++;
+
+  /* Treat "end" as EOF. */
+  if (p_end - p_start == 3 && !strncmp (p_start, "end", 3))
+    {
+      q = PyMem_Malloc (1);
+      if (q != NULL)
+	q[0] = '\0';
+      return q;
+    }
+
+  /* Copy the line to Python and return. */
+  q = PyMem_Malloc (n + 2);
+  if (q != NULL)
+    {
+      strncpy (q, p, n);
+      q[n] = '\n';
+      q[n + 1] = '\0';
+    }
+  return q;
+}
+
+void
+gdbpy_initialize_gdb_readline (void)
+{
+  Py_InitModule3 ("readline", GdbReadlineMethods,
+		  "GDB-provided dummy readline module to prevent conflicts with the standard readline module.");
+
+  PyOS_ReadlineFunctionPointer = gdbpy_readline_wrapper;
+}
+
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -234,6 +234,7 @@
 struct symtab_and_line *sal_object_to_symtab_and_line (PyObject *obj);
 struct frame_info *frame_object_to_frame_info (PyObject *frame_obj);
 
+void gdbpy_initialize_gdb_readline (void);
 void gdbpy_initialize_auto_load (void);
 void gdbpy_initialize_values (void);
 void gdbpy_initialize_frames (void);
diff --git a/gdb/python/python.c b/gdb/python/python.c
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -279,6 +279,43 @@
   return script;
 }
 
+/* Evaluate a Python command like PyRun_SimpleString, but uses
+   Py_single_input which prints the result of expressions if the input
+   is from tty, and Py_file_input otherwise. */
+
+static void
+eval_python_command (const char *command, int from_tty)
+{
+  struct cleanup *cleanup;
+  PyObject *m, *d, *v;
+
+  cleanup = ensure_python_env (get_current_arch (), current_language);
+
+  m = PyImport_AddModule ("__main__");
+  if (m == NULL)
+    error (_("Error while executing Python code."));
+
+  d = PyModule_GetDict (m);
+  v = PyRun_StringFlags (command,
+			 from_tty ? Py_single_input : Py_file_input,
+			 d, d, NULL);
+  if (v == NULL)
+    {
+      int interrupt = PyErr_ExceptionMatches (PyExc_KeyboardInterrupt);
+      PyErr_Print ();
+      if (! interrupt)
+	error (_("Error while executing Python code."));
+    }
+  else
+    {
+      Py_DECREF (v);
+      if (Py_FlushLine ())
+	PyErr_Clear ();
+    }
+
+  do_cleanups (cleanup);
+}
+
 /* Take a command line structure representing a 'python' command, and
    evaluate its body using the Python interpreter.  */
 
@@ -292,13 +329,10 @@
   if (cmd->body_count != 1)
     error (_("Invalid \"python\" block structure."));
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  script = compute_python_string (cmd->body_list[0]);
+  cleanup = make_cleanup (xfree, script);
 
-  script = compute_python_string (cmd->body_list[0]);
-  ret = PyRun_SimpleString (script);
-  xfree (script);
-  if (ret)
-    error (_("Error while executing Python code."));
+  eval_python_command (script, 0);
 
   do_cleanups (cleanup);
 }
@@ -316,18 +350,26 @@
   while (arg && *arg && isspace (*arg))
     ++arg;
   if (arg && *arg)
-    {
-      ensure_python_env (get_current_arch (), current_language);
-
-      if (PyRun_SimpleString (arg))
-	error (_("Error while executing Python code."));
-    }
+    eval_python_command (arg, from_tty);
   else
     {
-      struct command_line *l = get_command_line (python_control, "");
-
-      make_cleanup_free_command_lines (&l);
-      execute_control_command_untraced (l);
+      if (from_tty)
+	{
+	  int err;
+	  ensure_python_env (get_current_arch (), current_language);
+	  err = PyRun_InteractiveLoop(instream, "<stdin>");
+	  dont_repeat ();
+	  if (err && ! PyErr_ExceptionMatches (PyExc_KeyboardInterrupt))
+	    error(_("Error while executing Python code."));
+	}
+      else
+	{
+	  /* TODO: perhaps PyRun_FileExFlags can be used, which would
+	     simplify cli/cli-script.c. */
+	  struct command_line *l = get_command_line (python_control, "");
+	  make_cleanup_free_command_lines (&l);
+	  execute_control_command_untraced (l);
+	}
     }
 
   do_cleanups (cleanup);
@@ -1280,6 +1322,7 @@
   gdbpy_gdberror_exc = PyErr_NewException ("gdb.GdbError", NULL, NULL);
   PyModule_AddObject (gdb_module, "GdbError", gdbpy_gdberror_exc);
 
+  gdbpy_initialize_gdb_readline ();
   gdbpy_initialize_auto_load ();
   gdbpy_initialize_values ();
   gdbpy_initialize_frames ();

Re: Make the "python" command resemble the standard Python interpreter

[Joel Brobecker]

> I'd like to contribute a patch to improve the "python" command by
> making it resemble the standard Python interpreter in behavior.

I don't know about the patch, but the feature sounds great!

-- 
Joel

Re: Make the "python" command resemble the standard Python interpreter

[Kevin Pouget]

On Wed, Jan 11, 2012 at 1:18 AM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> Hi,
>
> I'd like to contribute a patch to improve the "python" command by making it resemble the standard Python interpreter in behavior.
>
> For "python" with arguments, this prints the results of expressions, e.g., "python 1 + 2" will print "3" (previously, it will not print anything).
>
> For "python" without arguments, this uses Python's built-in interactive loop (PyRun_InteractiveLoop) so that individual statements/expressions are immediately evaluated and results of expressions are printed. It also hooks GDB's readline wrappers (command_line_input) to Python so that line editing and history editing works. Additionally, Python's standard readline module is stubbed out because it conflicts with GDB's use of readline.
>
> This patch depends on a patch to handle SIGINT that I previously submitted (http://sourceware.org/bugzilla/show_bug.cgi?id=13265) to handle SIGINT in the interactive loop.
>
> Thanks!
>
> Yit
> January 10, 2012
>

Hello,

it sounds interesting indeed,
I dropped below a few comments.  I'm not a maintainer, so that's
nothing more than suggestions


> gdb/ChangeLog:
>
> 2012-01-10  Khoo Yit Phang <khooyp@cs.umd.edu>
>
> Make the "python" command resemble the standard Python interpreter.
> * Makefile.in (SUBDIR_PYTHON_OBS): Add py-gdb-readline.o.
> (SUBDIR_PYTHON_SRCS): Add python/py-gdb-readline.c.
> (py-gdb-readline.o): Add rule to compile python/py-gdb-readline.c.
> * python/py-gdb-readline.c: New file.
> * python/python-internal.h (gdbpy_initialize_gdb_readline): New
> prototype.
> * python/python.c (eval_python_command): New function.
> (eval_python_from_control_command): Call eval_python_command instead
> of PyRun_SimpleString.
> (python_command): For "python" with arguments, call
> eval_python_command.  For "python" without arguments, call
> PyRun_InteractiveLoop if from_tty is true, other call
> execute_control_command_untraced.
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -270,6 +270,7 @@
> #
> SUBDIR_PYTHON_OBS = \
> python.o \
> + py-gdb-readline.o \
> py-auto-load.o \
> py-block.o \
> py-bpevent.o \
> @@ -302,6 +303,7 @@
>
> SUBDIR_PYTHON_SRCS = \
> python/python.c \
> + python/py-gdb-readline.c \
> python/py-auto-load.c \
> python/py-block.c \
> python/py-bpevent.c \

name should be kept in alphabetical order, please. The same applies
for the entries below.

> @@ -2019,6 +2021,10 @@
> $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/python.c
> $(POSTCOMPILE)
>
> +py-gdb-readline.o: $(srcdir)/python/py-gdb-readline.c
> + $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-gdb-readline.c
> + $(POSTCOMPILE)
> +
> py-auto-load.o: $(srcdir)/python/py-auto-load.c
> $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-auto-load.c
> $(POSTCOMPILE)
> diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
> new file mode 100644
> --- /dev/null
> +++ b/gdb/python/py-gdb-readline.c
> @@ -0,0 +1,168 @@
> +/* Readline support for Python.
> +
> +   Copyright (C) 2012 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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/>.  */
> +
> +#include "defs.h"
> +#include "python-internal.h"
> +#include "../exceptions.h"
> +#include "../top.h"

according to other py-*.c files, you don't need the '../'

> +#include <stddef.h>
> +#include <string.h>
> +
> +/* Python's readline module conflicts with GDB's use of readline
> +   since readline is not reentrant. To prevent conflicts, a dummy
> +   readline module is set up which simply fails for any readline
> +   method. */

Two spaces after the dots "reentrant.  To prevent", "method.  */", and
the same in further comments

> +/* TODO: Ideally, this module should implement a reentrant wrapper
> +   to readline, so that the readline module can be used without
> +   conflicting with GDB. */

I think TODO will not be accepted in the final patch.

> +static PyObject *
> +unimplemented_command (PyObject *self, PyObject *args, PyObject *kw)
> +{
> +  PyErr_SetString (PyExc_NotImplementedError,
> +   "readline module disabled under GDB");
> +  return NULL;
> +}
> +
> +#define UNIMPLEMENTED(cmd) \
> +  { cmd, (PyCFunction)unimplemented_command, \
> +    METH_VARARGS | METH_KEYWORDS, "" }

space after the '('. The same applies below, and for all the function calls:
> function (...);
> (cast) var;

> +static PyMethodDef GdbReadlineMethods[] =
> +{
> +  /* readline module methods as of Python 2.7.2. */
> +  UNIMPLEMENTED("add_history"),
> +  UNIMPLEMENTED("clear_history"),
> +  UNIMPLEMENTED("get_begidx"),
> +  UNIMPLEMENTED("get_completer"),
> +  UNIMPLEMENTED("get_completer_delims"),
> +  UNIMPLEMENTED("get_completion_type"),
> +  UNIMPLEMENTED("get_current_history_length"),
> +  UNIMPLEMENTED("get_endidx"),
> +  UNIMPLEMENTED("get_history_item"),
> +  UNIMPLEMENTED("get_history_length"),
> +  UNIMPLEMENTED("get_line_buffer"),
> +  UNIMPLEMENTED("insert_text"),
> +  UNIMPLEMENTED("parse_and_bind"),
> +  UNIMPLEMENTED("read_history_file"),
> +  UNIMPLEMENTED("read_init_file"),
> +  UNIMPLEMENTED("redisplay"),
> +  UNIMPLEMENTED("remove_history_item"),
> +  UNIMPLEMENTED("replace_history_item"),
> +  UNIMPLEMENTED("set_completer"),
> +  UNIMPLEMENTED("set_completer_delims"),
> +  UNIMPLEMENTED("set_completion_display_matches_hook"),
> +  UNIMPLEMENTED("set_history_length"),
> +  UNIMPLEMENTED("set_pre_input_hook"),
> +  UNIMPLEMENTED("set_startup_hook"),
> +  UNIMPLEMENTED("write_history_file"),
> +  { NULL, NULL, 0, NULL }
> +};
> +
> +/* Readline function suitable for PyOS_ReadlineFunctionPointer, which
> +   is used for Python's interactive parser and raw_input. In both
> +   cases, sys_stdin and sys_stdout are always stdin and stdout
> +   respectively, as far as I can tell; they are ignored and
> +   command_line_input is used instead. */
> +
> +static char *
> +gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
> + char *prompt)

I think that 'char *prompt' whould be aligned with FILE *sys_stdin'

> +{
> +  int n;
> +  char *p = NULL, *p_start, *p_end, *q;
> +  volatile struct gdb_exception except;
> +
> +  TRY_CATCH (except, RETURN_MASK_ALL)
> +    {
> +      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();

new line between declarations and the code

> +      p = command_line_input (prompt, 0, "python");
> +      do_cleanups (cleanup);
> +    }

I'm not sure about that, but isn't the clean up supposed to be
executed even if an exception is thrown? it seems not to be the case
here

> +  /* Detect Ctrl-C and treat as KeyboardInterrupt. */
> +  if (except.reason == RETURN_QUIT)
> +    return NULL;
> +
> +  /* Handle errors by raising Python exceptions. */
> +  if (except.reason < 0)
> +    {
> +      /* The thread state is nulled during gdbpy_readline_wrapper,
> + with the original value saved in the following undocumented
> + variable (see Python's Parser/myreadline.c and
> + Modules/readline.c). */

comment lines should be aligned with "The thread" (or maybe my tabs
are not displayed properly)

> +      PyEval_RestoreThread (_PyOS_ReadlineTState);
> +      gdbpy_convert_exception (except);
> +      PyEval_SaveThread ();
> +      return NULL;
> +    }
> +
> +  /* Detect EOF (Ctrl-D). */
> +  if (p == NULL)
> +    {
> +      q = PyMem_Malloc (1);
> +      if (q != NULL)
> + q[0] = '\0';
> +      return q;
> +    }
> +
> +  n = strlen (p);
> +
> +  /* Detect "end" like process_next_line in cli/cli-script.c. */
> +  /* Strip trailing whitespace.  */
> +  p_end = p + n;
> +  while (p_end > p && (p_end[-1] == ' ' || p_end[-1] == '\t'))
> +    p_end--;
> +
> +  /* Strip leading whitespace.  */
> +  p_start = p;
> +  while (p_start < p_end && (*p_start == ' ' || *p_start == '\t'))
> +    p_start++;

maybe clu-utils.h remove_trailing_whitespace and skip_spaces already
do the same?

> +  /* Treat "end" as EOF. */
> +  if (p_end - p_start == 3 && !strncmp (p_start, "end", 3))
> +    {
> +      q = PyMem_Malloc (1);
> +      if (q != NULL)
> + q[0] = '\0';
> +      return q;
> +    }
> +
> +  /* Copy the line to Python and return. */
> +  q = PyMem_Malloc (n + 2);
> +  if (q != NULL)
> +    {
> +      strncpy (q, p, n);
> +      q[n] = '\n';
> +      q[n + 1] = '\0';
> +    }
> +  return q;
> +}
> +
> +void
> +gdbpy_initialize_gdb_readline (void)

all functions should have a comment, but I'm not sure whether it
should be in the C or H file

> +{
> +  Py_InitModule3 ("readline", GdbReadlineMethods,
> +  "GDB-provided dummy readline module to prevent conflicts with the standard readline module.");

This line is too long, should be < 80 chars

> +
> +  PyOS_ReadlineFunctionPointer = gdbpy_readline_wrapper;
> +}
> +
> diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
> --- a/gdb/python/python-internal.h
> +++ b/gdb/python/python-internal.h
> @@ -234,6 +234,7 @@
> struct symtab_and_line *sal_object_to_symtab_and_line (PyObject *obj);
> struct frame_info *frame_object_to_frame_info (PyObject *frame_obj);
>
> +void gdbpy_initialize_gdb_readline (void);
> void gdbpy_initialize_auto_load (void);
> void gdbpy_initialize_values (void);
> void gdbpy_initialize_frames (void);
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -279,6 +279,43 @@
>   return script;
> }
>
> +/* Evaluate a Python command like PyRun_SimpleString, but uses
> +   Py_single_input which prints the result of expressions if the input
> +   is from tty, and Py_file_input otherwise. */
> +
> +static void
> +eval_python_command (const char *command, int from_tty)
> +{
> +  struct cleanup *cleanup;
> +  PyObject *m, *d, *v;
> +
> +  cleanup = ensure_python_env (get_current_arch (), current_language);
> +
> +  m = PyImport_AddModule ("__main__");
> +  if (m == NULL)
> +    error (_("Error while executing Python code."));
> +
> +  d = PyModule_GetDict (m);
> +  v = PyRun_StringFlags (command,
> + from_tty ? Py_single_input : Py_file_input,
> + d, d, NULL);
> +  if (v == NULL)
> +    {
> +      int interrupt = PyErr_ExceptionMatches (PyExc_KeyboardInterrupt);
blank line

> +      PyErr_Print ();
> +      if (! interrupt)
> + error (_("Error while executing Python code."));
> +    }
> +  else
> +    {
> +      Py_DECREF (v);
> +      if (Py_FlushLine ())
> + PyErr_Clear ();
> +    }
> +
> +  do_cleanups (cleanup);
> +}
> +
> /* Take a command line structure representing a 'python' command, and
>    evaluate its body using the Python interpreter.  */
>
> @@ -292,13 +329,10 @@
>   if (cmd->body_count != 1)
>     error (_("Invalid \"python\" block structure."));
>
> -  cleanup = ensure_python_env (get_current_arch (), current_language);
> +  script = compute_python_string (cmd->body_list[0]);
> +  cleanup = make_cleanup (xfree, script);
>
> -  script = compute_python_string (cmd->body_list[0]);
> -  ret = PyRun_SimpleString (script);
> -  xfree (script);
> -  if (ret)
> -    error (_("Error while executing Python code."));
> +  eval_python_command (script, 0);
>
>   do_cleanups (cleanup);
> }
> @@ -316,18 +350,26 @@
>   while (arg && *arg && isspace (*arg))
>     ++arg;
>   if (arg && *arg)
> -    {
> -      ensure_python_env (get_current_arch (), current_language);
> -
> -      if (PyRun_SimpleString (arg))
> - error (_("Error while executing Python code."));
> -    }
> +    eval_python_command (arg, from_tty);
>   else
>     {
> -      struct command_line *l = get_command_line (python_control, "");
> -
> -      make_cleanup_free_command_lines (&l);
> -      execute_control_command_untraced (l);
> +      if (from_tty)
> + {
> +  int err;
> +  ensure_python_env (get_current_arch (), current_language);
> +  err = PyRun_InteractiveLoop(instream, "<stdin>");
> +  dont_repeat ();
> +  if (err && ! PyErr_ExceptionMatches (PyExc_KeyboardInterrupt))
> +    error(_("Error while executing Python code."));
> + }
> +      else
> + {
> +  /* TODO: perhaps PyRun_FileExFlags can be used, which would
> +     simplify cli/cli-script.c. */
> +  struct command_line *l = get_command_line (python_control, "");
> +  make_cleanup_free_command_lines (&l);
> +  execute_control_command_untraced (l);
> + }
>     }
>
>   do_cleanups (cleanup);
> @@ -1280,6 +1322,7 @@
>   gdbpy_gdberror_exc = PyErr_NewException ("gdb.GdbError", NULL, NULL);
>   PyModule_AddObject (gdb_module, "GdbError", gdbpy_gdberror_exc);
>
> +  gdbpy_initialize_gdb_readline ();
>   gdbpy_initialize_auto_load ();
>   gdbpy_initialize_values ();
>   gdbpy_initialize_frames ();

and you can maybe add some regression tests, if there's a way to test
it and update the 'python' documentation


Cordially,

Kevin

Re: Make the "python" command resemble the standard Python interpreter

[Joel Brobecker]

Just another minor comment:

> > +#include <stddef.h>
> > +#include <string.h>

Use "gdb_string" instead of string.h. Not all platforms have
string.h.

-- 
Joel

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

Thanks for the review, I'll attach an updated patch in a moment. I have a few questions:

On Jan 11, 2012, at 5:26 AM, Kevin Pouget wrote:

>> +static char *
>> +gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
>> + char *prompt)
> 
> I think that 'char *prompt' whould be aligned with FILE *sys_stdin'

It is properly tabbed originally and it seems to be the case when I download the attachment too. Perhaps a email formatting glitch?

>> +{
>> +  int n;
>> +  char *p = NULL, *p_start, *p_end, *q;
>> +  volatile struct gdb_exception except;
>> +
>> +  TRY_CATCH (except, RETURN_MASK_ALL)
>> +    {
>> +      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
> 
> new line between declarations and the code

Do you mean there should not be a new line?

>> +      p = command_line_input (prompt, 0, "python");
>> +      do_cleanups (cleanup);
>> +    }
> 
> I'm not sure about that, but isn't the clean up supposed to be
> executed even if an exception is thrown? it seems not to be the case
> here

Are you referring to do_cleanups? If I understand correctly, it's to handle the case where an exception is not thrown (see, e.g., py-value.c).

>> +  /* Detect Ctrl-C and treat as KeyboardInterrupt. */
>> +  if (except.reason == RETURN_QUIT)
>> +    return NULL;
>> +
>> +  /* Handle errors by raising Python exceptions. */
>> +  if (except.reason < 0)
>> +    {
>> +      /* The thread state is nulled during gdbpy_readline_wrapper,
>> + with the original value saved in the following undocumented
>> + variable (see Python's Parser/myreadline.c and
>> + Modules/readline.c). */
> 
> comment lines should be aligned with "The thread" (or maybe my tabs
> are not displayed properly)

That's might be the case.

>> +{
>> +  Py_InitModule3 ("readline", GdbReadlineMethods,
>> +  "GDB-provided dummy readline module to prevent conflicts with the standard readline module.");
> 
> This line is too long, should be < 80 chars

I'll shorten the line, but separately, how do you format lines containing strings that themselves can be up to 80 chars (e.g., for printing)?


Yit
January 11, 2012

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 73 bytes --]

Hi,

Here's my updated patch based on the review.

Yit
January 11, 2012


[-- Attachment #2: python-interactive.diff --]
[-- Type: application/octet-stream, Size: 10838 bytes --]

# HG changeset patch
# Parent 9f7d0a74b37f443edf5a7446fcadc806aea32f21
Make the "python" command resemble the standard Python interpreter.
- Use Py_single_input mode to interpret "python" with arguments, so that results of expressions are printed.
- Use Python's built-in interactive loop for "python" without arguments.
- Hook PyOS_ReadlineFunctionPointer to command_line_input to provide readline support.
- Install a dummy readline module to prevent conflicts that arise from using the standard Python readline module.

gdb/ChangeLog:

2012-01-10  Khoo Yit Phang <khooyp@cs.umd.edu>

	Make the "python" command resemble the standard Python interpreter.
	* Makefile.in (SUBDIR_PYTHON_OBS): Add py-gdb-readline.o.
	(SUBDIR_PYTHON_SRCS): Add python/py-gdb-readline.c.
	(py-gdb-readline.o): Add rule to compile python/py-gdb-readline.c.
	* python/py-gdb-readline.c: New file.
	* python/python-internal.h (gdbpy_initialize_gdb_readline): New
	prototype.
	* python/python.c (eval_python_command): New function.
	(eval_python_from_control_command): Call eval_python_command instead
	of PyRun_SimpleString.
	(python_command): For "python" with arguments, call
	eval_python_command.  For "python" without arguments, call
	PyRun_InteractiveLoop if from_tty is true, other call
	execute_control_command_untraced.

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -283,6 +283,7 @@
 	py-finishbreakpoint.o \
 	py-frame.o \
 	py-function.o \
+	py-gdb-readline.o \
 	py-inferior.o \
 	py-infthread.o \
 	py-lazy-string.o \
@@ -315,6 +316,7 @@
 	python/py-finishbreakpoint.c \
 	python/py-frame.c \
 	python/py-function.c \
+	python/py-gdb-readline.c \
 	python/py-inferior.c \
 	python/py-infthread.c \
 	python/py-lazy-string.c \
@@ -2071,6 +2073,10 @@
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-function.c
 	$(POSTCOMPILE)
 
+py-gdb-readline.o: $(srcdir)/python/py-gdb-readline.c
+	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-gdb-readline.c
+	$(POSTCOMPILE)
+
 py-inferior.o: $(srcdir)/python/py-inferior.c
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-inferior.c
 	$(POSTCOMPILE)
diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
new file mode 100644
--- /dev/null
+++ b/gdb/python/py-gdb-readline.c
@@ -0,0 +1,161 @@
+/* Readline support for Python.
+
+   Copyright (C) 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#include "defs.h"
+#include "python-internal.h"
+#include "exceptions.h"
+#include "top.h"
+#include "cli/cli-utils.h"
+#include "gdb_string.h"
+
+#include <stddef.h>
+
+/* Python's readline module conflicts with GDB's use of readline
+   since readline is not reentrant.  Ideally, a reentrant wrapper to
+   GDB's readline should be implemented to replace Python's readline
+   and prevent conflicts.  For now, this file implements a dummy
+   readline module which simply fails for any readline method.  */
+
+static PyObject *
+unimplemented_command (PyObject *self, PyObject *args, PyObject *kw)
+{
+  PyErr_SetString (PyExc_NotImplementedError,
+		   "readline module disabled under GDB");
+  return NULL;
+}
+
+#define UNIMPLEMENTED(cmd) \
+  { cmd, (PyCFunction)unimplemented_command, \
+    METH_VARARGS | METH_KEYWORDS, "" }
+
+static PyMethodDef GdbReadlineMethods[] =
+{
+  /* readline module methods as of Python 2.7.2.  */
+  UNIMPLEMENTED ("add_history"),
+  UNIMPLEMENTED ("clear_history"),
+  UNIMPLEMENTED ("get_begidx"),
+  UNIMPLEMENTED ("get_completer"),
+  UNIMPLEMENTED ("get_completer_delims"),
+  UNIMPLEMENTED ("get_completion_type"),
+  UNIMPLEMENTED ("get_current_history_length"),
+  UNIMPLEMENTED ("get_endidx"),
+  UNIMPLEMENTED ("get_history_item"),
+  UNIMPLEMENTED ("get_history_length"),
+  UNIMPLEMENTED ("get_line_buffer"),
+  UNIMPLEMENTED ("insert_text"),
+  UNIMPLEMENTED ("parse_and_bind"),
+  UNIMPLEMENTED ("read_history_file"),
+  UNIMPLEMENTED ("read_init_file"),
+  UNIMPLEMENTED ("redisplay"),
+  UNIMPLEMENTED ("remove_history_item"),
+  UNIMPLEMENTED ("replace_history_item"),
+  UNIMPLEMENTED ("set_completer"),
+  UNIMPLEMENTED ("set_completer_delims"),
+  UNIMPLEMENTED ("set_completion_display_matches_hook"),
+  UNIMPLEMENTED ("set_history_length"),
+  UNIMPLEMENTED ("set_pre_input_hook"),
+  UNIMPLEMENTED ("set_startup_hook"),
+  UNIMPLEMENTED ("write_history_file"),
+  { NULL, NULL, 0, NULL }
+};
+
+/* Readline function suitable for PyOS_ReadlineFunctionPointer, which
+   is used for Python's interactive parser and raw_input.  In both
+   cases, sys_stdin and sys_stdout are always stdin and stdout
+   respectively, as far as I can tell; they are ignored and
+   command_line_input is used instead.  */
+
+static char *
+gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
+			char *prompt)
+{
+  int n;
+  char *p = NULL, *p_start, *p_end, *q;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
+      p = command_line_input (prompt, 0, "python");
+      do_cleanups (cleanup);
+    }
+
+  /* Detect Ctrl-C and treat as KeyboardInterrupt.  */
+  if (except.reason == RETURN_QUIT)
+    return NULL;
+
+  /* Handle errors by raising Python exceptions.  */
+  if (except.reason < 0)
+    {
+      /* The thread state is nulled during gdbpy_readline_wrapper,
+	 with the original value saved in the following undocumented
+	 variable (see Python's Parser/myreadline.c and
+	 Modules/readline.c).  */
+      PyEval_RestoreThread (_PyOS_ReadlineTState);
+      gdbpy_convert_exception (except);
+      PyEval_SaveThread ();
+      return NULL;
+    }
+
+  /* Detect EOF (Ctrl-D).  */
+  if (p == NULL)
+    {
+      q = PyMem_Malloc (1);
+      if (q != NULL)
+	q[0] = '\0';
+      return q;
+    }
+
+  n = strlen (p);
+
+  /* Detect "end" like process_next_line in cli/cli-script.c.  */
+  p_start = remove_trailing_whitespace (p, p + n);
+  p_end = skip_spaces (p_start);
+  while (p_start < p_end && (*p_start == ' ' || *p_start == '\t'))
+    p_start++;
+
+  /* Treat "end" as EOF.  */
+  if (p_end - p_start == 3 && !strncmp (p_start, "end", 3))
+    {
+      q = PyMem_Malloc (1);
+      if (q != NULL)
+	q[0] = '\0';
+      return q;
+    }
+
+  /* Copy the line to Python and return.  */
+  q = PyMem_Malloc (n + 2);
+  if (q != NULL)
+    {
+      strncpy (q, p, n);
+      q[n] = '\n';
+      q[n + 1] = '\0';
+    }
+  return q;
+}
+
+void
+gdbpy_initialize_gdb_readline (void)
+{
+  Py_InitModule3 ("readline", GdbReadlineMethods,
+		  "Dummy readline module to prevent conflicts with GDB.");
+
+  PyOS_ReadlineFunctionPointer = gdbpy_readline_wrapper;
+}
+
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -234,6 +234,7 @@
 struct symtab_and_line *sal_object_to_symtab_and_line (PyObject *obj);
 struct frame_info *frame_object_to_frame_info (PyObject *frame_obj);
 
+void gdbpy_initialize_gdb_readline (void);
 void gdbpy_initialize_auto_load (void);
 void gdbpy_initialize_values (void);
 void gdbpy_initialize_frames (void);
diff --git a/gdb/python/python.c b/gdb/python/python.c
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -279,6 +279,43 @@
   return script;
 }
 
+/* Evaluate a Python command like PyRun_SimpleString, but uses
+   Py_single_input which prints the result of expressions if the input
+   is from tty, and Py_file_input otherwise.  */
+
+static void
+eval_python_command (const char *command, int from_tty)
+{
+  struct cleanup *cleanup;
+  PyObject *m, *d, *v;
+
+  cleanup = ensure_python_env (get_current_arch (), current_language);
+
+  m = PyImport_AddModule ("__main__");
+  if (m == NULL)
+    error (_("Error while executing Python code."));
+
+  d = PyModule_GetDict (m);
+  v = PyRun_StringFlags (command,
+			 from_tty ? Py_single_input : Py_file_input,
+			 d, d, NULL);
+  if (v == NULL)
+    {
+      int interrupt = PyErr_ExceptionMatches (PyExc_KeyboardInterrupt);
+      PyErr_Print ();
+      if (! interrupt)
+	error (_("Error while executing Python code."));
+    }
+  else
+    {
+      Py_DECREF (v);
+      if (Py_FlushLine ())
+	PyErr_Clear ();
+    }
+
+  do_cleanups (cleanup);
+}
+
 /* Take a command line structure representing a 'python' command, and
    evaluate its body using the Python interpreter.  */
 
@@ -292,13 +329,10 @@
   if (cmd->body_count != 1)
     error (_("Invalid \"python\" block structure."));
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  script = compute_python_string (cmd->body_list[0]);
+  cleanup = make_cleanup (xfree, script);
 
-  script = compute_python_string (cmd->body_list[0]);
-  ret = PyRun_SimpleString (script);
-  xfree (script);
-  if (ret)
-    error (_("Error while executing Python code."));
+  eval_python_command (script, 0);
 
   do_cleanups (cleanup);
 }
@@ -316,18 +350,24 @@
   while (arg && *arg && isspace (*arg))
     ++arg;
   if (arg && *arg)
-    {
-      ensure_python_env (get_current_arch (), current_language);
-
-      if (PyRun_SimpleString (arg))
-	error (_("Error while executing Python code."));
-    }
+    eval_python_command (arg, from_tty);
   else
     {
-      struct command_line *l = get_command_line (python_control, "");
-
-      make_cleanup_free_command_lines (&l);
-      execute_control_command_untraced (l);
+      if (from_tty)
+	{
+	  int err;
+	  ensure_python_env (get_current_arch (), current_language);
+	  err = PyRun_InteractiveLoop(instream, "<stdin>");
+	  dont_repeat ();
+	  if (err && ! PyErr_ExceptionMatches (PyExc_KeyboardInterrupt))
+	    error(_("Error while executing Python code."));
+	}
+      else
+	{
+	  struct command_line *l = get_command_line (python_control, "");
+	  make_cleanup_free_command_lines (&l);
+	  execute_control_command_untraced (l);
+	}
     }
 
   do_cleanups (cleanup);
@@ -1280,6 +1320,7 @@
   gdbpy_gdberror_exc = PyErr_NewException ("gdb.GdbError", NULL, NULL);
   PyModule_AddObject (gdb_module, "GdbError", gdbpy_gdberror_exc);
 
+  gdbpy_initialize_gdb_readline ();
   gdbpy_initialize_auto_load ();
   gdbpy_initialize_values ();
   gdbpy_initialize_frames ();

Re: Make the "python" command resemble the standard Python interpreter

[Kevin Pouget]

On Wed, Jan 11, 2012 at 4:53 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> Hi,
>
> Thanks for the review, I'll attach an updated patch in a moment. I have a few questions:
>
> On Jan 11, 2012, at 5:26 AM, Kevin Pouget wrote:
>
>>> +static char *
>>> +gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
>>> + char *prompt)
>>
>> I think that 'char *prompt' whould be aligned with FILE *sys_stdin'
>
> It is properly tabbed originally and it seems to be the case when I download the attachment too. Perhaps a email formatting glitch?

yes, if you're sure about the indentation, forget what I said ;)

>>> +{
>>> +  int n;
>>> +  char *p = NULL, *p_start, *p_end, *q;
>>> +  volatile struct gdb_exception except;
>>> +
>>> +  TRY_CATCH (except, RETURN_MASK_ALL)
>>> +    {
>>> +      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
>>
>> new line between declarations and the code
>
> Do you mean there should not be a new line?

no, you _should_ have a line between these two lines:

> +      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
> +      p = command_line_input (prompt, 0, "python");

>>> +      p = command_line_input (prompt, 0, "python");
>>> +      do_cleanups (cleanup);
>>> +    }
>>
>> I'm not sure about that, but isn't the clean up supposed to be
>> executed even if an exception is thrown? it seems not to be the case
>> here
>
> Are you referring to do_cleanups? If I understand correctly, it's to handle the case where an exception is not thrown (see, e.g., py-value.c).

I think that's you're supposed to use the cleanup machinery when you
don't explicitely handle the exception. Here you code looks like:

> TRY_CATCH
> {
>   do_something_dangerous()
> }
> handle_exception_if_any()
> continue_anyway()

so I think it's safe to simply call "gdbpy_suspend_sigint_handler"
after the exception handling.

>>> +  /* Detect Ctrl-C and treat as KeyboardInterrupt. */
>>> +  if (except.reason == RETURN_QUIT)
>>> +    return NULL;
>>> +
>>> +  /* Handle errors by raising Python exceptions. */
>>> +  if (except.reason < 0)
>>> +    {
>>> +      /* The thread state is nulled during gdbpy_readline_wrapper,
>>> + with the original value saved in the following undocumented
>>> + variable (see Python's Parser/myreadline.c and
>>> + Modules/readline.c). */
>>
>> comment lines should be aligned with "The thread" (or maybe my tabs
>> are not displayed properly)
>
> That's might be the case.
>
>>> +{
>>> +  Py_InitModule3 ("readline", GdbReadlineMethods,
>>> +  "GDB-provided dummy readline module to prevent conflicts with the standard readline module.");
>>
>> This line is too long, should be < 80 chars
>
> I'll shorten the line, but separately, how do you format lines containing strings that themselves can be up to 80 chars (e.g., for printing)?

you can use the C line breaker \,  there're examples at the end of
many .c and py-*.c files, like

>  c = add_com ("tbreak", class_breakpoint, tbreak_command, _("\
> Set a temporary breakpoint.\n\
> Like \"break\" except the breakpoint is only temporary,\n\
> ...


Kevin

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

On Jan 11, 2012, at 1:31 PM, Kevin Pouget wrote:

> On Wed, Jan 11, 2012 at 4:53 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> 
>>>> +      p = command_line_input (prompt, 0, "python");
>>>> +      do_cleanups (cleanup);
>>>> +    }
>>> 
>>> I'm not sure about that, but isn't the clean up supposed to be
>>> executed even if an exception is thrown? it seems not to be the case
>>> here
>> 
>> Are you referring to do_cleanups? If I understand correctly, it's to handle the case where an exception is not thrown (see, e.g., py-value.c).
> 
> I think that's you're supposed to use the cleanup machinery when you
> don't explicitely handle the exception. Here you code looks like:
> 
>> TRY_CATCH
>> {
>>  do_something_dangerous()
>> }
>> handle_exception_if_any()
>> continue_anyway()
> 
> so I think it's safe to simply call "gdbpy_suspend_sigint_handler"
> after the exception handling.

I don't think that's right. I traced this, and the cleanup function isn't called if I don't use do_cleanup. In this case, it must be called to restore the SIGINT handler to Python. In many other places I've looked that use TRY_CATCH, do_cleanup is called at the end too.

Yit
January 11, 2012

Re: Make the "python" command resemble the standard Python interpreter

[Kevin Pouget]

On Wed, Jan 11, 2012 at 7:48 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> Hi,
>
> On Jan 11, 2012, at 1:31 PM, Kevin Pouget wrote:
>
>> On Wed, Jan 11, 2012 at 4:53 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
>>
>>>>> +      p = command_line_input (prompt, 0, "python");
>>>>> +      do_cleanups (cleanup);
>>>>> +    }
>>>>
>>>> I'm not sure about that, but isn't the clean up supposed to be
>>>> executed even if an exception is thrown? it seems not to be the case
>>>> here
>>>
>>> Are you referring to do_cleanups? If I understand correctly, it's to handle the case where an exception is not thrown (see, e.g., py-value.c).
>>
>> I think that's you're supposed to use the cleanup machinery when you
>> don't explicitely handle the exception. Here you code looks like:
>>
>>> TRY_CATCH
>>> {
>>>  do_something_dangerous()
>>> }
>>> handle_exception_if_any()
>>> continue_anyway()
>>
>> so I think it's safe to simply call "gdbpy_suspend_sigint_handler"
>> after the exception handling.
>
> I don't think that's right. I traced this, and the cleanup function isn't called if I don't use do_cleanup. In this case, it must be called to restore the SIGINT handler to Python. In many other places I've looked that use TRY_CATCH, do_cleanup is called at the end too.


okay, so I'll have to study it a bit more! A maintainer review will
confirm if you're doing it correctly


> It also hooks GDB's readline wrappers (command_line_input) to Python so that line editing > and history editing works. Additionally, Python's standard readline module is stubbed out
> because it conflicts with GDB's use of readline.

that's interesting! I noticed that there's conflict between Python and
GDB readline; do you thing that this hook could also be used to fix
PDB's commandline prompt? (when you run "python import
pdb;pdb.set_trace()", history and left/right arrow don't work
properly)


Cordially,

Kevin

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 1739 bytes --]

Hi,

On Jan 11, 2012, at 2:03 PM, Kevin Pouget wrote:

> On Wed, Jan 11, 2012 at 7:48 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
>> 
>> It also hooks GDB's readline wrappers (command_line_input) to Python so that line editing > and history editing works. Additionally, Python's standard readline module is stubbed out
>> because it conflicts with GDB's use of readline.
> 
> that's interesting! I noticed that there's conflict between Python and
> GDB readline; do you thing that this hook could also be used to fix
> PDB's commandline prompt? (when you run "python import
> pdb;pdb.set_trace()", history and left/right arrow don't work
> properly)

Unfortunately no, and actually, my previous patch causes pdb.set_trace() to not work at all since since my dummy readline methods just throws exceptions.

Attached is a new patch that uses another way to disable the readline module, which works with pdb.set_trace(). However, readline support still doesn't work with pdb. The reason is because pdb uses raw_input, which in turn uses sys.stdin/sys.stdout to determine whether to use readline, but GDB replaces sys.stdin/sys.stdout with it's own file-like objects that isn't recognized as a tty by Python.

It should be possible to replace raw_input with one that works with GDB, but that can be another patch.

Making Python's readline module work under GDB is not possible, since it re-initializes libreadline and generally assumes that libreadline is completely under Python's control (and libreadline itself has no support for nesting/reentrancy).

It should also be possible to write a wrapper module around readline that avoids conflicts with GDB, but that'll take some more work.

Yit
January 11, 2012


[-- Attachment #2: python-interactive.diff --]
[-- Type: application/octet-stream, Size: 9747 bytes --]

# HG changeset patch
# Parent 9f7d0a74b37f443edf5a7446fcadc806aea32f21
Make the "python" command resemble the standard Python interpreter.
- Use Py_single_input mode to interpret "python" with arguments, so that results of expressions are printed.
- Use Python's built-in interactive loop for "python" without arguments.
- Hook PyOS_ReadlineFunctionPointer to command_line_input to provide readline support.
- Block importing of the standard Python readline module using a sys.meta_path loader to prevent conflicts with GDB.

gdb/ChangeLog:

2012-01-11  Khoo Yit Phang <khooyp@cs.umd.edu>

	Make the "python" command resemble the standard Python interpreter.
	* Makefile.in (SUBDIR_PYTHON_OBS): Add py-gdb-readline.o.
	(SUBDIR_PYTHON_SRCS): Add python/py-gdb-readline.c.
	(py-gdb-readline.o): Add rule to compile python/py-gdb-readline.c.
	* python/py-gdb-readline.c: New file.
	* python/python-internal.h (gdbpy_initialize_gdb_readline): New
	prototype.
	* python/python.c (eval_python_command): New function.
	(eval_python_from_control_command): Call eval_python_command instead
	of PyRun_SimpleString.
	(python_command): For "python" with arguments, call
	eval_python_command.  For "python" without arguments, call
	PyRun_InteractiveLoop if from_tty is true, other call
	execute_control_command_untraced.

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -283,6 +283,7 @@
 	py-finishbreakpoint.o \
 	py-frame.o \
 	py-function.o \
+	py-gdb-readline.o \
 	py-inferior.o \
 	py-infthread.o \
 	py-lazy-string.o \
@@ -315,6 +316,7 @@
 	python/py-finishbreakpoint.c \
 	python/py-frame.c \
 	python/py-function.c \
+	python/py-gdb-readline.c \
 	python/py-inferior.c \
 	python/py-infthread.c \
 	python/py-lazy-string.c \
@@ -2071,6 +2073,10 @@
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-function.c
 	$(POSTCOMPILE)
 
+py-gdb-readline.o: $(srcdir)/python/py-gdb-readline.c
+	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-gdb-readline.c
+	$(POSTCOMPILE)
+
 py-inferior.o: $(srcdir)/python/py-inferior.c
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-inferior.c
 	$(POSTCOMPILE)
diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
new file mode 100644
--- /dev/null
+++ b/gdb/python/py-gdb-readline.c
@@ -0,0 +1,133 @@
+/* Readline support for Python.
+
+   Copyright (C) 2012 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#include "defs.h"
+#include "python-internal.h"
+#include "exceptions.h"
+#include "top.h"
+#include "cli/cli-utils.h"
+#include "gdb_string.h"
+
+#include <stddef.h>
+
+/* Readline function suitable for PyOS_ReadlineFunctionPointer, which
+   is used for Python's interactive parser and raw_input.  In both
+   cases, sys_stdin and sys_stdout are always stdin and stdout
+   respectively, as far as I can tell; they are ignored and
+   command_line_input is used instead.  */
+
+static char *
+gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
+			char *prompt)
+{
+  int n;
+  char *p = NULL, *p_start, *p_end, *q;
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
+
+      p = command_line_input (prompt, 0, "python");
+      do_cleanups (cleanup);
+    }
+
+  /* Detect Ctrl-C and treat as KeyboardInterrupt.  */
+  if (except.reason == RETURN_QUIT)
+    return NULL;
+
+  /* Handle errors by raising Python exceptions.  */
+  if (except.reason < 0)
+    {
+      /* The thread state is nulled during gdbpy_readline_wrapper,
+	 with the original value saved in the following undocumented
+	 variable (see Python's Parser/myreadline.c and
+	 Modules/readline.c).  */
+      PyEval_RestoreThread (_PyOS_ReadlineTState);
+      gdbpy_convert_exception (except);
+      PyEval_SaveThread ();
+      return NULL;
+    }
+
+  /* Detect EOF (Ctrl-D).  */
+  if (p == NULL)
+    {
+      q = PyMem_Malloc (1);
+      if (q != NULL)
+	q[0] = '\0';
+      return q;
+    }
+
+  n = strlen (p);
+
+  /* Detect "end" like process_next_line in cli/cli-script.c.  */
+  p_start = remove_trailing_whitespace (p, p + n);
+  p_end = skip_spaces (p_start);
+  while (p_start < p_end && (*p_start == ' ' || *p_start == '\t'))
+    p_start++;
+
+  /* Treat "end" as EOF.  */
+  if (p_end - p_start == 3 && !strncmp (p_start, "end", 3))
+    {
+      q = PyMem_Malloc (1);
+      if (q != NULL)
+	q[0] = '\0';
+      return q;
+    }
+
+  /* Copy the line to Python and return.  */
+  q = PyMem_Malloc (n + 2);
+  if (q != NULL)
+    {
+      strncpy (q, p, n);
+      q[n] = '\n';
+      q[n + 1] = '\0';
+    }
+  return q;
+}
+
+/* Initialize Python readline support.  */
+
+void
+gdbpy_initialize_gdb_readline (void)
+{
+  /* Python's readline module conflicts with GDB's use of readline
+     since readline is not reentrant.  Ideally, a reentrant wrapper to
+     GDB's readline should be implemented to replace Python's readline
+     and prevent conflicts.  For now, this file implements a
+     sys.meta_path finder that simply fails to import the readline
+     module.  */
+  PyRun_SimpleString ("\
+import sys\n\
+\n\
+class GdbRemoveReadlineFinder:\n\
+  def find_module(self, fullname, path=None):\n\
+    if fullname == 'readline' and path is None:\n\
+      return self\n\
+    return None\n\
+\n\
+  def load_module(self, fullname):\n\
+    raise ImportError('readline module disabled under GDB')\n\
+\n\
+sys.meta_path.append(GdbRemoveReadlineFinder())\n\
+");
+
+  PyOS_ReadlineFunctionPointer = gdbpy_readline_wrapper;
+}
+
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -234,6 +234,7 @@
 struct symtab_and_line *sal_object_to_symtab_and_line (PyObject *obj);
 struct frame_info *frame_object_to_frame_info (PyObject *frame_obj);
 
+void gdbpy_initialize_gdb_readline (void);
 void gdbpy_initialize_auto_load (void);
 void gdbpy_initialize_values (void);
 void gdbpy_initialize_frames (void);
diff --git a/gdb/python/python.c b/gdb/python/python.c
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -279,6 +279,44 @@
   return script;
 }
 
+/* Evaluate a Python command like PyRun_SimpleString, but uses
+   Py_single_input which prints the result of expressions if the input
+   is from tty, and Py_file_input otherwise.  */
+
+static void
+eval_python_command (const char *command, int from_tty)
+{
+  struct cleanup *cleanup;
+  PyObject *m, *d, *v;
+
+  cleanup = ensure_python_env (get_current_arch (), current_language);
+
+  m = PyImport_AddModule ("__main__");
+  if (m == NULL)
+    error (_("Error while executing Python code."));
+
+  d = PyModule_GetDict (m);
+  v = PyRun_StringFlags (command,
+			 from_tty ? Py_single_input : Py_file_input,
+			 d, d, NULL);
+  if (v == NULL)
+    {
+      int interrupt = PyErr_ExceptionMatches (PyExc_KeyboardInterrupt);
+
+      PyErr_Print ();
+      if (! interrupt)
+	error (_("Error while executing Python code."));
+    }
+  else
+    {
+      Py_DECREF (v);
+      if (Py_FlushLine ())
+	PyErr_Clear ();
+    }
+
+  do_cleanups (cleanup);
+}
+
 /* Take a command line structure representing a 'python' command, and
    evaluate its body using the Python interpreter.  */
 
@@ -292,13 +330,10 @@
   if (cmd->body_count != 1)
     error (_("Invalid \"python\" block structure."));
 
-  cleanup = ensure_python_env (get_current_arch (), current_language);
+  script = compute_python_string (cmd->body_list[0]);
+  cleanup = make_cleanup (xfree, script);
 
-  script = compute_python_string (cmd->body_list[0]);
-  ret = PyRun_SimpleString (script);
-  xfree (script);
-  if (ret)
-    error (_("Error while executing Python code."));
+  eval_python_command (script, 0);
 
   do_cleanups (cleanup);
 }
@@ -316,18 +351,26 @@
   while (arg && *arg && isspace (*arg))
     ++arg;
   if (arg && *arg)
-    {
-      ensure_python_env (get_current_arch (), current_language);
-
-      if (PyRun_SimpleString (arg))
-	error (_("Error while executing Python code."));
-    }
+    eval_python_command (arg, from_tty);
   else
     {
-      struct command_line *l = get_command_line (python_control, "");
+      if (from_tty)
+	{
+	  int err;
 
-      make_cleanup_free_command_lines (&l);
-      execute_control_command_untraced (l);
+	  ensure_python_env (get_current_arch (), current_language);
+	  err = PyRun_InteractiveLoop(instream, "<stdin>");
+	  dont_repeat ();
+	  if (err && ! PyErr_ExceptionMatches (PyExc_KeyboardInterrupt))
+	    error(_("Error while executing Python code."));
+	}
+      else
+	{
+	  struct command_line *l = get_command_line (python_control, "");
+
+	  make_cleanup_free_command_lines (&l);
+	  execute_control_command_untraced (l);
+	}
     }
 
   do_cleanups (cleanup);
@@ -1280,6 +1323,7 @@
   gdbpy_gdberror_exc = PyErr_NewException ("gdb.GdbError", NULL, NULL);
   PyModule_AddObject (gdb_module, "GdbError", gdbpy_gdberror_exc);
 
+  gdbpy_initialize_gdb_readline ();
   gdbpy_initialize_auto_load ();
   gdbpy_initialize_values ();
   gdbpy_initialize_frames ();

Re: Make the "python" command resemble the standard Python interpreter

[Tom Tromey]

>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:

Yit> Attached is a new patch that uses another way to disable the readline
Yit> module, which works with pdb.set_trace(). However, readline support
Yit> still doesn't work with pdb. The reason is because pdb uses raw_input,
Yit> which in turn uses sys.stdin/sys.stdout to determine whether to use
Yit> readline, but GDB replaces sys.stdin/sys.stdout with it's own
Yit> file-like objects that isn't recognized as a tty by Python.

This sounds like http://sourceware.org/bugzilla/show_bug.cgi?id=12150
Maybe we should try to fix that instead?

Yit> Making Python's readline module work under GDB is not possible, since
Yit> it re-initializes libreadline and generally assumes that libreadline
Yit> is completely under Python's control (and libreadline itself has no
Yit> support for nesting/reentrancy).

The initialization shouldn't be a problem.

Calling rl_initialize multiple times is ok -- readline() itself calls
it.

I don't know about the reentrancy though.  Also, IIRC, gdb uses the
unusual async interface.  Maybe that raises some issues.

Tom

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

On Jan 11, 2012, at 4:30 PM, Tom Tromey wrote:

>>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:
> 
> Yit> Attached is a new patch that uses another way to disable the readline
> Yit> module, which works with pdb.set_trace(). However, readline support
> Yit> still doesn't work with pdb. The reason is because pdb uses raw_input,
> Yit> which in turn uses sys.stdin/sys.stdout to determine whether to use
> Yit> readline, but GDB replaces sys.stdin/sys.stdout with it's own
> Yit> file-like objects that isn't recognized as a tty by Python.
> 
> This sounds like http://sourceware.org/bugzilla/show_bug.cgi?id=12150
> Maybe we should try to fix that instead?

Yes, it's related, if I understand correctly, sys.stdout/sys.stderr are overridden the way they are so that GDB can capture/filter the output. Another solution, besides overriding raw_input/input, would be to point sys.stdout/sys.stderr to a PTY, though I'm not sure what the trade-offs are.

> Yit> Making Python's readline module work under GDB is not possible, since
> Yit> it re-initializes libreadline and generally assumes that libreadline
> Yit> is completely under Python's control (and libreadline itself has no
> Yit> support for nesting/reentrancy).
> 
> The initialization shouldn't be a problem.
> 
> Calling rl_initialize multiple times is ok -- readline() itself calls
> it.

However, other configuration cannot be set repeatedly, e.g., rl_readline_name, rl_completion_*, various rl_*_hook, keybindings, and so on.

> I don't know about the reentrancy though.  Also, IIRC, gdb uses the
> unusual async interface.  Maybe that raises some issues.

That may explain another issue I've seen where inputs are delayed by one keystroke.

Yit
January 11, 2011

Re: Make the "python" command resemble the standard Python interpreter

[Tom Tromey]

>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:

>> The initialization shouldn't be a problem.
>> 
>> Calling rl_initialize multiple times is ok -- readline() itself calls
>> it.

Yit> However, other configuration cannot be set repeatedly, e.g.,
Yit> rl_readline_name, rl_completion_*, various rl_*_hook, keybindings, and
Yit> so on.

Ok, I see.  Thanks for the explanation.

Tom

Re: Make the "python" command resemble the standard Python interpreter

[Tom Tromey]

>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:

Yit> It should be possible to replace raw_input with one that works with
Yit> GDB, but that can be another patch.

Yit> Making Python's readline module work under GDB is not possible, since
Yit> it re-initializes libreadline and generally assumes that libreadline
Yit> is completely under Python's control (and libreadline itself has no
Yit> support for nesting/reentrancy).

Yit> It should also be possible to write a wrapper module around readline
Yit> that avoids conflicts with GDB, but that'll take some more work.

Would you mind filing bugs for these discoveries you've made?
That would be handy.

I didn't re-read the patches yet.  I plan to wait until your paperwork
is done; then please just re-post the latest versions with commentary.

thanks,
Tom

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

On Jan 20, 2012, at 4:25 PM, Tom Tromey wrote:

>>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:
> 
> Yit> It should be possible to replace raw_input with one that works with
> Yit> GDB, but that can be another patch.
> 
> Yit> Making Python's readline module work under GDB is not possible, since
> Yit> it re-initializes libreadline and generally assumes that libreadline
> Yit> is completely under Python's control (and libreadline itself has no
> Yit> support for nesting/reentrancy).
> 
> Yit> It should also be possible to write a wrapper module around readline
> Yit> that avoids conflicts with GDB, but that'll take some more work.
> 
> Would you mind filing bugs for these discoveries you've made?
> That would be handy.

Sure.

> I didn't re-read the patches yet.  I plan to wait until your paperwork
> is done; then please just re-post the latest versions with commentary.

Just to mention, I probably won't get to update my patches for the next couple of weeks.

Yit
January 22, 2012

Re: Make the "python" command resemble the standard Python interpreter

[Tom Tromey]

>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:

Yit> I'd like to contribute a patch to improve the "python" command by
Yit> making it resemble the standard Python interpreter in behavior.

Thanks.

Do you have a copyright assignment in place?
If not, contact me off-list and I will get you started.
We need this before we can incorporate this patch.

Yit> For "python" with arguments, this prints the results of expressions,
Yit> e.g., "python 1 + 2" will print "3" (previously, it will not print
Yit> anything).

Did you run the test suite?  I'm curious if this caused any failures.

On balance, I'm ok with this idea, since I don't think we make many
promises about CLI output remaining the same across versions.

Yit> For "python" without arguments, this uses Python's built-in
Yit> interactive loop (PyRun_InteractiveLoop) so that individual
Yit> statements/expressions are immediately evaluated and results of
Yit> expressions are printed.

Sounds nice.

This also seems like it could cause some (spurious) test failures.

Yit> It also hooks GDB's readline wrappers
Yit> (command_line_input) to Python so that line editing and history
Yit> editing works. Additionally, Python's standard readline module is
Yit> stubbed out because it conflicts with GDB's use of readline.

Can you expand more on how it conflicts?

I think it would be better to make it not conflict somehow.

Yit> +  TRY_CATCH (except, RETURN_MASK_ALL)
Yit> +    {
Yit> +      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
Yit> +      p = command_line_input (prompt, 0, "python");
Yit> +      do_cleanups (cleanup);
Yit> +    }
Yit> +
Yit> +  /* Detect Ctrl-C and treat as KeyboardInterrupt. */
Yit> +  if (except.reason == RETURN_QUIT)
Yit> +    return NULL;

Does this case need the Python exception to be set?  If not, I think it
would be good to expand this comment to explain the situation.

Yit> +  m = PyImport_AddModule ("__main__");
Yit> +  if (m == NULL)
Yit> +    error (_("Error while executing Python code."));

You have to do something with the Python exception here.
Usually we use gdbpy_print_stack, but sometimes other things are
appropriate.

Yit> +  d = PyModule_GetDict (m);

Do we need error checking?
I didn't look at the API docs.

Yit> +  v = PyRun_StringFlags (command,
Yit> +			 from_tty ? Py_single_input : Py_file_input,
Yit> +			 d, d, NULL);
Yit> +  if (v == NULL)
Yit> +    {
Yit> +      int interrupt = PyErr_ExceptionMatches (PyExc_KeyboardInterrupt);
Yit> +      PyErr_Print ();

gdbpy_print_stack.

Yit> +      if (! interrupt)
Yit> +	error (_("Error while executing Python code."));

Why the special case for interrupts here?

Tom

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

On Jan 11, 2012, at 3:51 PM, Tom Tromey wrote:

> Yit> For "python" with arguments, this prints the results of expressions,
> Yit> e.g., "python 1 + 2" will print "3" (previously, it will not print
> Yit> anything).
> 
> Did you run the test suite?  I'm curious if this caused any failures.
> 
> On balance, I'm ok with this idea, since I don't think we make many
> promises about CLI output remaining the same across versions.

I'm still trying to figure out how to run the testsuite and understand the output. Is it possible to just run the Python ones, to not take so long?

> Yit> It also hooks GDB's readline wrappers
> Yit> (command_line_input) to Python so that line editing and history
> Yit> editing works. Additionally, Python's standard readline module is
> Yit> stubbed out because it conflicts with GDB's use of readline.
> 
> Can you expand more on how it conflicts?
> 
> I think it would be better to make it not conflict somehow.

Both GDB and Python's readline module and methods attempt to configure libreadline, clobbering each others configuration. There doesn't seem to be any support in libreadline for reentrancy. It may be possible to implement a wrapper around readline for Python, but that's another patch, I think.

> Yit> +  TRY_CATCH (except, RETURN_MASK_ALL)
> Yit> +    {
> Yit> +      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
> Yit> +      p = command_line_input (prompt, 0, "python");
> Yit> +      do_cleanups (cleanup);
> Yit> +    }
> Yit> +
> Yit> +  /* Detect Ctrl-C and treat as KeyboardInterrupt. */
> Yit> +  if (except.reason == RETURN_QUIT)
> Yit> +    return NULL;
> 
> Does this case need the Python exception to be set?  If not, I think it
> would be good to expand this comment to explain the situation.

In this case, no, since PyOS_ReadlineFunctionPointer is supposed to return NULL upon Ctrl-C, from what I can gather. That comment is misleading, I'll change it.

> Yit> +  m = PyImport_AddModule ("__main__");
> Yit> +  if (m == NULL)
> Yit> +    error (_("Error while executing Python code."));
> 
> You have to do something with the Python exception here.
> Usually we use gdbpy_print_stack, but sometimes other things are
> appropriate.

Okay.

> Yit> +  d = PyModule_GetDict (m);
> 
> Do we need error checking?
> I didn't look at the API docs.

Nope, "this function never fails".

> Yit> +  v = PyRun_StringFlags (command,
> Yit> +			 from_tty ? Py_single_input : Py_file_input,
> Yit> +			 d, d, NULL);
> Yit> +  if (v == NULL)
> Yit> +    {
> Yit> +      int interrupt = PyErr_ExceptionMatches (PyExc_KeyboardInterrupt);
> Yit> +      PyErr_Print ();
> 
> gdbpy_print_stack.
> 
> Yit> +      if (! interrupt)
> Yit> +	error (_("Error while executing Python code."));
> 
> Why the special case for interrupts here?

This is because PyErr_Print clears the error, annoyingly enough, so I can't check whether it's a KeyboardInterrupt after PyErr_Print.

Yit
January 11, 2012

Re: Make the "python" command resemble the standard Python interpreter

[Tom Tromey]

>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:

Yit> I'm still trying to figure out how to run the testsuite and understand
Yit> the output. Is it possible to just run the Python ones, to not take so
Yit> long?

cd build/gdb
make check RUNTESTFLAGS=--directory=gdb.python

Tom

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

On Jan 11, 2012, at 3:51 PM, Tom Tromey wrote:

> Yit> +  m = PyImport_AddModule ("__main__");
> Yit> +  if (m == NULL)
> Yit> +    error (_("Error while executing Python code."));
> 
> You have to do something with the Python exception here.
> Usually we use gdbpy_print_stack, but sometimes other things are
> appropriate.
> 
> Yit> +  v = PyRun_StringFlags (command,
> Yit> +			 from_tty ? Py_single_input : Py_file_input,
> Yit> +			 d, d, NULL);
> Yit> +  if (v == NULL)
> Yit> +    {
> Yit> +      int interrupt = PyErr_ExceptionMatches (PyExc_KeyboardInterrupt);
> Yit> +      PyErr_Print ();
> 
> gdbpy_print_stack.

I've ran the testsuite, and of course, there were many test failures, probably spurious and I'll update. However, a number of errors turn out to be due to adding gdbpy_print_stack, because the original lines of code that I replaced (python:249 and python.c:273) simply called error like:

      if (PyRun_SimpleString (arg))
        error (_("Error while executing Python code."));

Should I defer your suggested change to a separate patch?

Yit
January 11, 2012

Re: Make the "python" command resemble the standard Python interpreter

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:

> I think it would be better to make it not conflict somehow.
>
> Yit> +  TRY_CATCH (except, RETURN_MASK_ALL)
> Yit> +    {
> Yit> +      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
> Yit> +      p = command_line_input (prompt, 0, "python");
> Yit> +      do_cleanups (cleanup);
> Yit> +    }
> Yit> +
> Yit> +  /* Detect Ctrl-C and treat as KeyboardInterrupt. */
> Yit> +  if (except.reason == RETURN_QUIT)
> Yit> +    return NULL;
>
> Does this case need the Python exception to be set?  If not, I think it
> would be good to expand this comment to explain the situation.

Yes, I think it does.  If command_line_input fails for a reason other
than the one case of Ctrl-C, the exception I believe with be eaten.

Also, the cleanup in the TRY_CATCH seems weird?

> Yit> +  m = PyImport_AddModule ("__main__");
> Yit> +  if (m == NULL)
> Yit> +    error (_("Error while executing Python code."));
>
> You have to do something with the Python exception here.
> Usually we use gdbpy_print_stack, but sometimes other things are
> appropriate.


> Yit> +  d = PyModule_GetDict (m);
>
> Do we need error checking?
> I didn't look at the API docs.

Nope, PyModule_GetDict never fails.  But given how things change in
Python (ie 3.x vs 2.x) it might be cautious just to add a check here
anyway.

Cheers,

Phil

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

(Sorry if you received a duplicate email, my previous reply was rejected by gdb-patches.)

Hi,

On Jan 13, 2012, at 8:56 AM, Phil Muldoon wrote:

> Tom Tromey <tromey@redhat.com> writes:
> 
>>>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:
>> 
>> Yit> +  TRY_CATCH (except, RETURN_MASK_ALL)
>> Yit> +    {
>> Yit> +      struct cleanup *cleanup = gdbpy_suspend_sigint_handler ();
>> Yit> +      p = command_line_input (prompt, 0, "python");
>> Yit> +      do_cleanups (cleanup);
>> Yit> +    }
>> Yit> +
>> Yit> +  /* Detect Ctrl-C and treat as KeyboardInterrupt. */
>> Yit> +  if (except.reason == RETURN_QUIT)
>> Yit> +    return NULL;
>> 
>> Does this case need the Python exception to be set?  If not, I think it
>> would be good to expand this comment to explain the situation.
> 
> Yes, I think it does.  If command_line_input fails for a reason other
> than the one case of Ctrl-C, the exception I believe with be eaten.

Tom's email did not include all the code: the other cases are handled via gdbpy_convert_exception.

> Also, the cleanup in the TRY_CATCH seems weird?

Can you clarify?

In this case, gdbpy_suspend_sigint_handler suspends Python's interrupt handler and reverts to GDB's. The cleanup handler resumes Python's interrupt handler, and additionally captures any changes to GDB's signal handler, and so has to be run before returning to Python.

>> Yit> +  d = PyModule_GetDict (m);
>> 
>> Do we need error checking?
>> I didn't look at the API docs.
> 
> Nope, PyModule_GetDict never fails.  But given how things change in
> Python (ie 3.x vs 2.x) it might be cautious just to add a check here
> anyway.

Sounds reasonable.

Yit
January 13, 2012

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Tue, Jan 10, 2012 at 4:18 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> Hi,
>
> I'd like to contribute a patch to improve the "python" command by making it resemble the standard Python interpreter in behavior.
>
> For "python" with arguments, this prints the results of expressions, e.g., "python 1 + 2" will print "3" (previously, it will not print anything).
>
> For "python" without arguments, this uses Python's built-in interactive loop (PyRun_InteractiveLoop) so that individual statements/expressions are immediately evaluated and results of expressions are printed. It also hooks GDB's readline wrappers (command_line_input) to Python so that line editing and history editing works. Additionally, Python's standard readline module is stubbed out because it conflicts with GDB's use of readline.
>
> This patch depends on a patch to handle SIGINT that I previously submitted (http://sourceware.org/bugzilla/show_bug.cgi?id=13265) to handle SIGINT in the interactive loop.
>
> Thanks!

There already is an established behaviour for python without arguments
(with or without a terminal).

(gdb) python
 > print 1 + 2
 > end
3
(gdb)

I've read the thread and I don't seen anyone raising this issue (could
have missed it though.  I saw Tom's message re: what output CLI will
generate, but this is about behaviour).
I've read the patch and I don't think using from_tty is ok to select
between the different behaviours.

Can we pick a different way of invoking the python interpreter?

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

On Jan 12, 2012, at 11:40 AM, Doug Evans wrote:

> There already is an established behaviour for python without arguments
> (with or without a terminal).
> 
> (gdb) python
>> print 1 + 2
>> end
> 3
> (gdb)
> 
> I've read the thread and I don't seen anyone raising this issue (could
> have missed it though.  I saw Tom's message re: what output CLI will
> generate, but this is about behaviour).
> I've read the patch and I don't think using from_tty is ok to select
> between the different behaviours.

Can you explain why?

> Can we pick a different way of invoking the python interpreter?

I had suggested in my original patch to use PyRun_FileExFlags when from_tty is false, which is how the Python interpreter picks the different behavior (Py_Main), so both from_tty and not from_tty would resemble Python. This would also eliminate the need for handling python in cli/cli-script.c and other places.

Yit
January 12, 2012

RE: Make the "python" command resemble the standard Python interpreter

[Paul_Koning]

>There already is an established behaviour for python without arguments (with or without a terminal).
>
>(gdb) python
> > print 1 + 2
> > end
>3
>(gdb)
>
>I've read the thread and I don't seen anyone raising this issue (could have missed it though.  I saw Tom's message re: what output CLI will generate, but this is about behaviour).
>I've read the patch and I don't think using from_tty is ok to select between the different behaviours.
>
>Can we pick a different way of invoking the python interpreter?

But "python" seems rather an intuitive way of doing it.  The existing behavior is quite surprising and unintuitive, because it accumulates an entire multi-line string without any parsing, and then hands it off all at once to Python.  I see no reason to preserve that behavior if we have a normal Python interactive interpreter loop.  That loop does everything the existing thing does and far more/better, so it should replace the old behavior.

For example, if you commit a syntax error, the existing machinery doesn't tell you until much later, when you say "end".  The interactive interpreter will tell you at the time you make the error.

	paul

Re: Make the "python" command resemble the standard Python interpreter

[Joel Brobecker]

> But "python" seems rather an intuitive way of doing it.  The existing
> behavior is quite surprising and unintuitive, because it accumulates
> an entire multi-line string without any parsing, and then hands it off
> all at once to Python.  I see no reason to preserve that behavior if
> we have a normal Python interactive interpreter loop.  That loop does
> everything the existing thing does and far more/better, so it should
> replace the old behavior.

I tend to agree. I have wished many times before that the "python"
command without argument would enter the Python interpreter.

-- 
Joel

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Thu, Jan 12, 2012 at 8:52 AM,  <Paul_Koning@dell.com> wrote:
>>There already is an established behaviour for python without arguments (with or without a terminal).
>>
>>(gdb) python
>> > print 1 + 2
>> > end
>>3
>>(gdb)
>>
>>I've read the thread and I don't seen anyone raising this issue (could have missed it though.  I saw Tom's message re: what output CLI will generate, but this is about behaviour).
>>I've read the patch and I don't think using from_tty is ok to select between the different behaviours.
>>
>>Can we pick a different way of invoking the python interpreter?
>
> But "python" seems rather an intuitive way of doing it.

I COMPLETELY AGREE!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
However, this changes things incompatibly ...

RE: Make the "python" command resemble the standard Python interpreter

[Paul_Koning]

Yes, but I think the benefits outweigh the small imcompatibility.

	paul

-----Original Message-----
From: Doug Evans [mailto:dje@google.com] 
Sent: Thursday, January 12, 2012 12:28 PM
To: Koning, Paul
Cc: khooyp@cs.umd.edu; gdb-patches@sourceware.org
Subject: Re: Make the "python" command resemble the standard Python interpreter

On Thu, Jan 12, 2012 at 8:52 AM,  <Paul_Koning@dell.com> wrote:
>>There already is an established behaviour for python without arguments (with or without a terminal).
>>
>>(gdb) python
>> > print 1 + 2
>> > end
>>3
>>(gdb)
>>
>>I've read the thread and I don't seen anyone raising this issue (could have missed it though.  I saw Tom's message re: what output CLI will generate, but this is about behaviour).
>>I've read the patch and I don't think using from_tty is ok to select between the different behaviours.
>>
>>Can we pick a different way of invoking the python interpreter?
>
> But "python" seems rather an intuitive way of doing it.

I COMPLETELY AGREE!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
However, this changes things incompatibly ...

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Thu, Jan 12, 2012 at 9:30 AM,  <Paul_Koning@dell.com> wrote:
> Yes, but I think the benefits outweigh the small imcompatibility.

Yes and no.
- I may want a script that invokes python interactively.
- How do I write a gdb macro that invokes the python repl?

Solve those problems, and provide a migration path away from the old
behaviour, and then you've got something.

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Thu, Jan 12, 2012 at 9:38 AM, Doug Evans <dje@google.com> wrote:
> On Thu, Jan 12, 2012 at 9:30 AM,  <Paul_Koning@dell.com> wrote:
>> Yes, but I think the benefits outweigh the small imcompatibility.
>
> Yes and no.
> - I may want a script that invokes python interactively.
> - How do I write a gdb macro that invokes the python repl?
>
> Solve those problems, and provide a migration path away from the old
> behaviour, and then you've got something.

As a strawman, a new command, python-foo, could be provided
[python-code?  python-script?] that had the "old" behaviour.

As for migration, we *could* add python-foo as an alias for python to
7.4 (seems safe enough) and say the "python" command will change it's
behaviour in 7.5 (or some such).
Or we could just skip the first step and just say python will change
incompatibly in 7.5.
[I don't have a preference, but I think we need to start doing this
kind of thing, and this would be good practice. :-)]

RE: Make the "python" command resemble the standard Python interpreter

[Paul_Koning]

>...
>> - I may want a script that invokes python interactively.
>> - How do I write a gdb macro that invokes the python repl?
>>
>> Solve those problems, and provide a migration path away from the old 
>> behaviour, and then you've got something.
>
>As a strawman, a new command, python-foo, could be provided [python-code?  python-script?] that had the "old" behaviour.

Nice solution.

>As for migration, we *could* add python-foo as an alias for python to
>7.4 (seems safe enough) and say the "python" command will change it's behaviour in 7.5 (or some such).
>Or we could just skip the first step and just say python will change incompatibly in 7.5.
>[I don't have a preference, but I think we need to start doing this kind of thing, and this would be good practice. :-)]

I'd say one step not two, given that the population of users of the scenario you mentioned is likely to be tiny.

	paul

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Thu, Jan 12, 2012 at 9:47 AM,  <Paul_Koning@dell.com> wrote:
>>...
>>> - I may want a script that invokes python interactively.
>>> - How do I write a gdb macro that invokes the python repl?
>>>
>>> Solve those problems, and provide a migration path away from the old
>>> behaviour, and then you've got something.
>>
>>As a strawman, a new command, python-foo, could be provided [python-code?  python-script?] that had the "old" behaviour.
>
> Nice solution.

btw, would we ever want to pass options to the python repl?
If that might ever occur, then we don't want python with arguments to
be the old behaviour.

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

On Jan 12, 2012, at 1:13 PM, Doug Evans wrote:

> On Jan 12, 2012 9:51 AM, "Doug Evans" <dje@google.com> wrote:
> >
> > On Thu, Jan 12, 2012 at 9:47 AM,  <Paul_Koning@dell.com> wrote:
> > >>...
> > >>> - I may want a script that invokes python interactively.
> > >>> - How do I write a gdb macro that invokes the python repl?
> > >>>
> > >>> Solve those problems, and provide a migration path away from the old
> > >>> behaviour, and then you've got something.
> > >>
> > >>As a strawman, a new command, python-foo, could be provided [python-code?  python-script?] that had the "old" behaviour.
> > >
> > > Nice solution.
> >
> > btw, would we ever want to pass options to the python repl?
> > If that might ever occur, then we don't want python with arguments to
> > be the old behaviour.
> 
> Or preferably have a new command should the need arise (I like python-repl, but I realize repl may be too obscure :-) ).
> OK, I think I'm OK with where this is going.

Instead of making a new command, we can add an option to, say "python /i", that forces the interpreter to start, so that you can define a GDB macro that starts a Python interpreter (when from_tty is false). That would retain compatibility with the current behavior.

Yit
January 12, 2012

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Thu, Jan 12, 2012 at 10:19 AM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> Hi,
>
> On Jan 12, 2012, at 1:13 PM, Doug Evans wrote:
>
>> On Jan 12, 2012 9:51 AM, "Doug Evans" <dje@google.com> wrote:
>> >
>> > On Thu, Jan 12, 2012 at 9:47 AM,  <Paul_Koning@dell.com> wrote:
>> > >>...
>> > >>> - I may want a script that invokes python interactively.
>> > >>> - How do I write a gdb macro that invokes the python repl?
>> > >>>
>> > >>> Solve those problems, and provide a migration path away from the old
>> > >>> behaviour, and then you've got something.
>> > >>
>> > >>As a strawman, a new command, python-foo, could be provided [python-code?  python-script?] that had the "old" behaviour.
>> > >
>> > > Nice solution.
>> >
>> > btw, would we ever want to pass options to the python repl?
>> > If that might ever occur, then we don't want python with arguments to
>> > be the old behaviour.
>>
>> Or preferably have a new command should the need arise (I like python-repl, but I realize repl may be too obscure :-) ).
>> OK, I think I'm OK with where this is going.
>
> Instead of making a new command, we can add an option to, say "python /i", that forces the interpreter to start, so that you can define a GDB macro that starts a Python interpreter (when from_tty is false). That would retain compatibility with the current behavior.

Heh.  / is for display options (e.g. x/i $pc), - is for other kinds of
options (e.g. symbol-file -readnow foo).
But yeah, that's another alternative.  [It feels more problematic,
e.g. the caveat you mention.]

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

On Jan 12, 2012, at 1:30 PM, Doug Evans wrote:

> On Thu, Jan 12, 2012 at 10:19 AM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
>> Hi,
>> 
>> On Jan 12, 2012, at 1:13 PM, Doug Evans wrote:
>> 
>>> On Jan 12, 2012 9:51 AM, "Doug Evans" <dje@google.com> wrote:
>>>> 
>>>> On Thu, Jan 12, 2012 at 9:47 AM,  <Paul_Koning@dell.com> wrote:
>>>>>> ...
>>>>>>> - I may want a script that invokes python interactively.
>>>>>>> - How do I write a gdb macro that invokes the python repl?
>>>>>>> 
>>>>>>> Solve those problems, and provide a migration path away from the old
>>>>>>> behaviour, and then you've got something.
>>>>>> 
>>>>>> As a strawman, a new command, python-foo, could be provided [python-code?  python-script?] that had the "old" behaviour.
>>>>> 
>>>>> Nice solution.
>>>> 
>>>> btw, would we ever want to pass options to the python repl?
>>>> If that might ever occur, then we don't want python with arguments to
>>>> be the old behaviour.
>>> 
>>> Or preferably have a new command should the need arise (I like python-repl, but I realize repl may be too obscure :-) ).
>>> OK, I think I'm OK with where this is going.
>> 
>> Instead of making a new command, we can add an option to, say "python /i", that forces the interpreter to start, so that you can define a GDB macro that starts a Python interpreter (when from_tty is false). That would retain compatibility with the current behavior.
> 
> Heh.  / is for display options (e.g. x/i $pc), - is for other kinds of
> options (e.g. symbol-file -readnow foo).
> But yeah, that's another alternative.  [It feels more problematic,
> e.g. the caveat you mention.]

For reference, the caveat I mentioned was in an email rejected by the mailing list server: that was for "python -i" which would disallow the python script "-i".

"python /i" would not have this caveat, since "/i" isn't a valid Python expression.

Yit
January 12, 2012

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Thu, Jan 12, 2012 at 10:36 AM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
>>> Instead of making a new command, we can add an option to, say "python /i", that forces the interpreter to start, so that you can define a GDB macro that starts a Python interpreter (when from_tty is false). That would retain compatibility with the current behavior.
>>
>> Heh.  / is for display options (e.g. x/i $pc), - is for other kinds of
>> options (e.g. symbol-file -readnow foo).
>> But yeah, that's another alternative.  [It feels more problematic,
>> e.g. the caveat you mention.]
>
> For reference, the caveat I mentioned was in an email rejected by the mailing list server: that was for "python -i" which would disallow the python script "-i".
>
> "python /i" would not have this caveat, since "/i" isn't a valid Python expression.

Yeah, but we shouldn't use /i.
[it also feels a bit too hacky]

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

[sorry, for the resend, didn't realize my mailer would pick rich html]

On Thu, Jan 12, 2012 at 9:51 AM, Doug Evans <dje@google.com> wrote:
> On Thu, Jan 12, 2012 at 9:47 AM,  <Paul_Koning@dell.com> wrote:
>>>...
>>>> - I may want a script that invokes python interactively.
>>>> - How do I write a gdb macro that invokes the python repl?
>>>>
>>>> Solve those problems, and provide a migration path away from the old
>>>> behaviour, and then you've got something.
>>>
>>>As a strawman, a new command, python-foo, could be provided [python-code?  python-script?] that had the "old" behaviour.
>>
>> Nice solution.
>
> btw, would we ever want to pass options to the python repl?
> If that might ever occur, then we don't want python with arguments to
> be the old behaviour.

Or preferably have a new command should the need arise (I like
python-repl, but I realize repl may be too obscure :-) ).
[so plain "python" would invoke the repl and "python foo" would
evaluate foo in python]
OK, I think I'm OK with where this is going.

Re: Make the "python" command resemble the standard Python interpreter

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Paul> Yes, but I think the benefits outweigh the small imcompatibility.

Doug> Yes and no.
Doug> - I may want a script that invokes python interactively.
Doug> - How do I write a gdb macro that invokes the python repl?

I think:

(gdb) python gdb.execute('python', from_tty = True)

This is sort of weird, but I think it is ok to require a weird command
to fill an unusual need.

Tom

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

On Jan 20, 2012, at 4:22 PM, Tom Tromey wrote:

>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
> 
> Paul> Yes, but I think the benefits outweigh the small imcompatibility.
> 
> Doug> Yes and no.
> Doug> - I may want a script that invokes python interactively.
> Doug> - How do I write a gdb macro that invokes the python repl?
> 
> I think:
> 
> (gdb) python gdb.execute('python', from_tty = True)
> 
> This is sort of weird, but I think it is ok to require a weird command
> to fill an unusual need.

To summarize the various proposals:
1. Make "python" start an interactive Python shell only, and introduce another command (say, "python-script") for non-interactive scripts.
2. Make "python" take a flag (say, "python -") that forces an interactive Python shell regardless of from_tty.
3. Start an interactive Python shell from a user-defined command by "python gdb.execute('python', from_tty = True)".

Personally, I prefer 2 or 3 since they retain compatibility with existing scripts.

Yit
January 22, 2012

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Sun, Jan 22, 2012 at 8:42 AM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> Hi,
>
> On Jan 20, 2012, at 4:22 PM, Tom Tromey wrote:
>
>>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>>
>> Paul> Yes, but I think the benefits outweigh the small imcompatibility.
>>
>> Doug> Yes and no.
>> Doug> - I may want a script that invokes python interactively.
>> Doug> - How do I write a gdb macro that invokes the python repl?
>>
>> I think:
>>
>> (gdb) python gdb.execute('python', from_tty = True)
>>
>> This is sort of weird, but I think it is ok to require a weird command
>> to fill an unusual need.
>
> To summarize the various proposals:
> 1. Make "python" start an interactive Python shell only, and introduce another command (say, "python-script") for non-interactive scripts.
> 2. Make "python" take a flag (say, "python -") that forces an interactive Python shell regardless of from_tty.
> 3. Start an interactive Python shell from a user-defined command by "python gdb.execute('python', from_tty = True)".
>
> Personally, I prefer 2 or 3 since they retain compatibility with existing scripts.

Hi.
I'm not that comfortable with having the python command having such
varying behaviours (especially based on the value of from_tty).

OTOH, if you want to treat it as "a lost cause", an "experiment gone
wrong", or whatever :-), and add commands that don't have that
problem, then I might be ok with it.

Re: Make the "python" command resemble the standard Python interpreter

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> I'm not that comfortable with having the python command having such
Doug> varying behaviours (especially based on the value of from_tty).

What bad effect do you think it will cause?

Tom

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Tue, Jan 24, 2012 at 9:30 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> I'm not that comfortable with having the python command having such
> Doug> varying behaviours (especially based on the value of from_tty).
>
> What bad effect do you think it will cause?

[for example]
Suppose I want to cut-n-paste some lines from a script into my session?
Maybe as a quick hack or maybe to test something out or whatever.

With this change I can't do that for something that contains:

python
foo
end

As I say, having "python" invoke the python repl *is* the more
intuitive thing to do,
so add a new command, python-foo, that retains/provides the old
"python ... end" behaviour and I'm ok.
[IWBN to also deprecate and eventually remove support for "python ...
end", but that's a teensy bit harder, 1/2 :-).]

OTOH, I don't mind just adding "python-foo" to invoke the python repl.
If someone doesn't want to type all that s/he can always do "alias
pyfoo = python-foo" (or whatever).
We could even provide our own alias (if we could come up with
something better than "python" :-)).

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

On Jan 26, 2012, at 12:43 PM, Doug Evans wrote:

> On Tue, Jan 24, 2012 at 9:30 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>> 
>> Doug> I'm not that comfortable with having the python command having such
>> Doug> varying behaviours (especially based on the value of from_tty).
>> 
>> What bad effect do you think it will cause?
> 
> [for example]
> Suppose I want to cut-n-paste some lines from a script into my session?
> Maybe as a quick hack or maybe to test something out or whatever.
> 
> With this change I can't do that for something that contains:
> 
> python
> foo
> end

This scenario works perfectly fine in my current implementation based on from_tty: "python\n" starts the Python REPL, "foo\n" runs in the interpreter, and "end\n" quits the interpreter ("end" by itself in a line is specially recognized as an alternative to Ctrl-D), which has the same outcome as in the script.

Yit
January 29, 2012

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Sun, Jan 29, 2012 at 7:10 PM, Khoo Yit Phang <khooyp@cs.umd.edu> wrote:
> Hi,
>
> On Jan 26, 2012, at 12:43 PM, Doug Evans wrote:
>
>> On Tue, Jan 24, 2012 at 9:30 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>>>
>>> Doug> I'm not that comfortable with having the python command having such
>>> Doug> varying behaviours (especially based on the value of from_tty).
>>>
>>> What bad effect do you think it will cause?
>>
>> [for example]
>> Suppose I want to cut-n-paste some lines from a script into my session?
>> Maybe as a quick hack or maybe to test something out or whatever.
>>
>> With this change I can't do that for something that contains:
>>
>> python
>> foo
>> end
>
> This scenario works perfectly fine in my current implementation based on from_tty: "python\n" starts the Python REPL, "foo\n" runs in the interpreter, and "end\n" quits the interpreter ("end" by itself in a line is specially recognized as an alternative to Ctrl-D), which has the same outcome as in the script.

Assuming the script didn't intentionally force some keyboard interaction.
[In emacs I sometimes just cut-n-paste a part of a script en masse.]

Plus, with some playing around I found this:

--- foo.gdb - snip ---
python
if 0 == 1:
  print "foo"
print "bar"
end
--- snip ---

(gdb) source foo.gdb
bar
(gdb)

But cut-n-paste that script into gdb and I get this:

(gdb) python
if 0 == 1:
  print "foo"
print "bar"
end
>>> ... ...   File "<stdin>", line 3
    print "bar"
        ^
SyntaxError: invalid syntax
>>>
(gdb)

[For reference sake, here's how I cut-n-pasted it in emacs:
C-x C-f foo.gdb RET C-space C-x ] C-b M-w C-x b RET C-y RET
I hope I transcribed that right.]

Python's repl expects a blank line to end the block.
I don't know if there's a way to work around this.  Maybe there is.
So now I'm even less comfortable.

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Mon, Jan 30, 2012 at 9:18 AM, Doug Evans <dje@google.com> wrote:
> Plus, with some playing around I found this:
>
> --- foo.gdb - snip ---
> python
> if 0 == 1:
>  print "foo"
> print "bar"
> end
> --- snip ---
>
> (gdb) source foo.gdb
> bar
> (gdb)
>
> But cut-n-paste that script into gdb and I get this:
>
> (gdb) python
> if 0 == 1:
>  print "foo"
> print "bar"
> end
>>>> ... ...   File "<stdin>", line 3
>    print "bar"
>        ^
> SyntaxError: invalid syntax
>>>>
> (gdb)
>
> [For reference sake, here's how I cut-n-pasted it in emacs:
> C-x C-f foo.gdb RET C-space C-x ] C-b M-w C-x b RET C-y RET
> I hope I transcribed that right.]
>
> Python's repl expects a blank line to end the block.
> I don't know if there's a way to work around this.  Maybe there is.
> So now I'm even less comfortable.

btw, that's with the latest python-interactive script (that I could
find) applied (+ the sigint patch too).

For grin's sake, there's another example:

--- snip ---
python
if 0 == 1:
  print "foo"
end
---

If I cut-n-paste that into gdb the "end" terminates the "if" block
(heh, didn't expect that :-)), and afterwards I'm still in python.
Maybe this can be fixed too.

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Mon, Jan 30, 2012 at 9:25 AM, Doug Evans <dje@google.com> wrote:
> On Mon, Jan 30, 2012 at 9:18 AM, Doug Evans <dje@google.com> wrote:
>> Plus, with some playing around I found this:
>>
>> --- foo.gdb - snip ---
>> python
>> if 0 == 1:
>>  print "foo"
>> print "bar"
>> end
>> --- snip ---
>>
>> (gdb) source foo.gdb
>> bar
>> (gdb)
>>
>> But cut-n-paste that script into gdb and I get this:
>>
>> (gdb) python
>> if 0 == 1:
>>  print "foo"
>> print "bar"
>> end
>>>>> ... ...   File "<stdin>", line 3
>>    print "bar"
>>        ^
>> SyntaxError: invalid syntax
>>>>>
>> (gdb)
>>
>> [For reference sake, here's how I cut-n-pasted it in emacs:
>> C-x C-f foo.gdb RET C-space C-x ] C-b M-w C-x b RET C-y RET
>> I hope I transcribed that right.]
>>
>> Python's repl expects a blank line to end the block.
>> I don't know if there's a way to work around this.  Maybe there is.
>> So now I'm even less comfortable.
>
> btw, that's with the latest python-interactive script (that I could
> find) applied (+ the sigint patch too).
>
> For grin's sake, there's another example:
>
> --- snip ---
> python
> if 0 == 1:
>  print "foo"
> end
> ---
>
> If I cut-n-paste that into gdb the "end" terminates the "if" block
> (heh, didn't expect that :-)), and afterwards I'm still in python.
> Maybe this can be fixed too.

btw, in an effort to keep things moving along, to repeat something I
mentioned earlier,
If you, for example, add a new command, say, "python-block" ('tis the
best I could come up with :-() that always behaved like python...end
in scripts, then I think that would be ok: Users that want the
existing script-like behaviour can switch to and use it instead of
"python". IOW, "python-block" always behaves like the existing
"python" command without arguments.

E.g.,

python-block
if 0 == 1:
  print "foo"
end

RE: Make the "python" command resemble the standard Python interpreter

[Paul_Koning]

I'm confused.  "Python's repl expects a blank line to end a block"?  I'm not sure what a repl is, but Python doesn't require blank lines for anything.  End of block is defined by smaller indent.

I don't understand that error message at all.

	paul

-----Original Message-----
From: Doug Evans [mailto:dje@google.com] 
Sent: Monday, February 06, 2012 3:09 PM
To: Khoo Yit Phang
Cc: Tom Tromey; Koning, Paul; gdb-patches@sourceware.org
Subject: Re: Make the "python" command resemble the standard Python interpreter

On Mon, Jan 30, 2012 at 9:25 AM, Doug Evans <dje@google.com> wrote:
> On Mon, Jan 30, 2012 at 9:18 AM, Doug Evans <dje@google.com> wrote:
>> Plus, with some playing around I found this:
>>
>> --- foo.gdb - snip ---
>> python
>> if 0 == 1:
>>  print "foo"
>> print "bar"
>> end
>> --- snip ---
>>
>> (gdb) source foo.gdb
>> bar
>> (gdb)
>>
>> But cut-n-paste that script into gdb and I get this:
>>
>> (gdb) python
>> if 0 == 1:
>>  print "foo"
>> print "bar"
>> end
>>>>> ... ...   File "<stdin>", line 3
>>    print "bar"
>>        ^
>> SyntaxError: invalid syntax
>>>>>
>> (gdb)
>>
>> [For reference sake, here's how I cut-n-pasted it in emacs:
>> C-x C-f foo.gdb RET C-space C-x ] C-b M-w C-x b RET C-y RET I hope I 
>> transcribed that right.]
>>
>> Python's repl expects a blank line to end the block.
>> I don't know if there's a way to work around this.  Maybe there is.
>> So now I'm even less comfortable.
>
> btw, that's with the latest python-interactive script (that I could
> find) applied (+ the sigint patch too).
>
> For grin's sake, there's another example:
>
> --- snip ---
> python
> if 0 == 1:
>  print "foo"
> end
> ---
>
> If I cut-n-paste that into gdb the "end" terminates the "if" block 
> (heh, didn't expect that :-)), and afterwards I'm still in python.
> Maybe this can be fixed too.

btw, in an effort to keep things moving along, to repeat something I mentioned earlier, If you, for example, add a new command, say, "python-block" ('tis the best I could come up with :-() that always behaved like python...end in scripts, then I think that would be ok: Users that want the existing script-like behaviour can switch to and use it instead of "python". IOW, "python-block" always behaves like the existing "python" command without arguments.

E.g.,

python-block
if 0 == 1:
  print "foo"
end

Re: Make the "python" command resemble the standard Python interpreter

[Khoo Yit Phang]

Hi,

Doug is right: Python's standard REPL (read-eval-print-loop), i.e., what you get when you run "python" from the shell or call the PyRun_InteractiveLoop function, has a slightly different behavior when defining the outermost block. (See some example transcripts at the end.)

So, copying and pasting in (non-GDB) Python has the hazards Doug is worried about. I suppose it's preferable to avoid this hazard in GDB's Python, probably by introducing another command such as "python-block".

Alternatively, I suppose it's possible to work around PyRun_InteractiveLoop by detecting a newline without indent, if we're willing to diverge from standard Python. 

Yit
February 6, 2012


 bash> python
 Python 2.7.2 (default, Nov 21 2011, 15:04:09) 
 [GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
 Type "help", "copyright", "credits" or "license" for more information.
 >>> if 1 == 1:
 ...   print 1
 ... if 2 == 2:
   File "<stdin>", line 3
     if 2 == 2:
      ^
 SyntaxError: invalid syntax
 >>> if 1 == 1:
 ...   print 1
 ... 
 1
 >>> def foo():
 ...  return 1
 ... def bar():
   File "<stdin>", line 3
     def bar():
       ^
 SyntaxError: invalid syntax
 >>> def foo():
 ...   return 1
 ... 


On Feb 6, 2012, at 3:13 PM, <Paul_Koning@Dell.com> wrote:

> I'm confused.  "Python's repl expects a blank line to end a block"?  I'm not sure what a repl is, but Python doesn't require blank lines for anything.  End of block is defined by smaller indent.
> 
> I don't understand that error message at all.
> 
> 	paul
> 
> -----Original Message-----
> From: Doug Evans [mailto:dje@google.com] 
> Sent: Monday, February 06, 2012 3:09 PM
> To: Khoo Yit Phang
> Cc: Tom Tromey; Koning, Paul; gdb-patches@sourceware.org
> Subject: Re: Make the "python" command resemble the standard Python interpreter
> 
> On Mon, Jan 30, 2012 at 9:25 AM, Doug Evans <dje@google.com> wrote:
>> On Mon, Jan 30, 2012 at 9:18 AM, Doug Evans <dje@google.com> wrote:
>>> Plus, with some playing around I found this:
>>> 
>>> --- foo.gdb - snip ---
>>> python
>>> if 0 == 1:
>>>  print "foo"
>>> print "bar"
>>> end
>>> --- snip ---
>>> 
>>> (gdb) source foo.gdb
>>> bar
>>> (gdb)
>>> 
>>> But cut-n-paste that script into gdb and I get this:
>>> 
>>> (gdb) python
>>> if 0 == 1:
>>>  print "foo"
>>> print "bar"
>>> end
>>>>>> ... ...   File "<stdin>", line 3
>>>    print "bar"
>>>        ^
>>> SyntaxError: invalid syntax
>>>>>> 
>>> (gdb)
>>> 
>>> [For reference sake, here's how I cut-n-pasted it in emacs:
>>> C-x C-f foo.gdb RET C-space C-x ] C-b M-w C-x b RET C-y RET I hope I 
>>> transcribed that right.]
>>> 
>>> Python's repl expects a blank line to end the block.
>>> I don't know if there's a way to work around this.  Maybe there is.
>>> So now I'm even less comfortable.
>> 
>> btw, that's with the latest python-interactive script (that I could
>> find) applied (+ the sigint patch too).
>> 
>> For grin's sake, there's another example:
>> 
>> --- snip ---
>> python
>> if 0 == 1:
>>  print "foo"
>> end
>> ---
>> 
>> If I cut-n-paste that into gdb the "end" terminates the "if" block 
>> (heh, didn't expect that :-)), and afterwards I'm still in python.
>> Maybe this can be fixed too.
> 
> btw, in an effort to keep things moving along, to repeat something I mentioned earlier, If you, for example, add a new command, say, "python-block" ('tis the best I could come up with :-() that always behaved like python...end in scripts, then I think that would be ok: Users that want the existing script-like behaviour can switch to and use it instead of "python". IOW, "python-block" always behaves like the existing "python" command without arguments.
> 
> E.g.,
> 
> python-block
> if 0 == 1:
>  print "foo"
> end

Re: Make the "python" command resemble the standard Python interpreter

[Doug Evans]

On Mon, Feb 6, 2012 at 12:13 PM,  <Paul_Koning@dell.com> wrote:
> End of block is defined by smaller indent.

Here's a cut-n-paste of an experiment I just did.

[dje@annie ~]$ python
Python 2.7 (r27:82500, Sep 16 2010, 18:02:00)
[GCC 4.5.1 20100907 (Red Hat 4.5.1-3)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> if 0 == 1:
...   print "foo"
... print "bar"
  File "<stdin>", line 3
    print "bar"
        ^
SyntaxError: invalid syntax
>>>

In a script Python can see the change in indentation.
I gather in interactive mode things work differently, otherwise how
can I terminate the `if' without invoking something new.

btw, "repl" == "Read Eval Print Loop".
http://en.wikipedia.org/wiki/REPL

RE: Make the "python" command resemble the standard Python interpreter

[Paul_Koning]

I see what you mean.  Thanks for the correction.

	paul

-----Original Message-----
From: Doug Evans [mailto:dje@google.com] 
Sent: Monday, February 06, 2012 3:34 PM
To: Koning, Paul
Cc: khooyp@cs.umd.edu; tromey@redhat.com; gdb-patches@sourceware.org
Subject: Re: Make the "python" command resemble the standard Python interpreter

On Mon, Feb 6, 2012 at 12:13 PM,  <Paul_Koning@dell.com> wrote:
> End of block is defined by smaller indent.

Here's a cut-n-paste of an experiment I just did.

[dje@annie ~]$ python
Python 2.7 (r27:82500, Sep 16 2010, 18:02:00) [GCC 4.5.1 20100907 (Red Hat 4.5.1-3)] on linux2 Type "help", "copyright", "credits" or "license" for more information.
>>> if 0 == 1:
...   print "foo"
... print "bar"
  File "<stdin>", line 3
    print "bar"
        ^
SyntaxError: invalid syntax
>>>

In a script Python can see the change in indentation.
I gather in interactive mode things work differently, otherwise how can I terminate the `if' without invoking something new.

btw, "repl" == "Read Eval Print Loop".
http://en.wikipedia.org/wiki/REPL

Re: Make the "python" command resemble the standard Python interpreter

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> --- snip ---
Doug> python
Doug> if 0 == 1:
Doug>   print "foo"
Doug> end
Doug> ---

Thanks for the examples.
I'm convinced that we need 2 commands.

Tom