Rewrite "python" command exception handling

The "python" command (and the Python implementation of the gdb
"source" command) does not handle Python exceptions in the same way as
other gdb-facing Python code.  In particular, exceptions are turned
into a generic error rather than being routed through
gdbpy_handle_exception, which takes care of converting to 'quit' as
appropriate.

I think this was done this way because PyRun_SimpleFile and friends do
not propagate the Python exception -- they simply indicate that one
occurred.

This patch reimplements these functions to respect the general gdb
convention here.  As a bonus, some Windows-specific code can be
removed, as can the _execute_file function.

The bulk of this change is tweaking the test suite to match the new
way that exceptions are displayed.  These changes are largely
uninteresting.  However, it's worth pointing out the py-error.exp
change.  Here, the failure changes because the test changes the host
charset to something that isn't supported by Python.  This then
results in a weird error in the new setup.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31354
Acked-By: Tom de Vries <tdevries@suse.de>
Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Author: tromey

gdb/doc/python.texi

@@ -721,9 +721,9 @@ When executing the @code{python} command, Python exceptions
 uncaught within the Python code are translated to calls to
 @value{GDBN} error-reporting mechanism.  If the command that called
 @code{python} does not handle the error, @value{GDBN} will
-terminate it and print an error message containing the Python
-exception name, the associated value, and the Python call stack
-backtrace at the point where the exception was raised.  Example:
+terminate it and print an error message.  Exactly what will be printed
+depends on @code{set python print-stack} (@pxref{Python Commands}).
+Example:
 
 @smallexample
 (@value{GDBP}) python print foo

gdb/python/lib/gdb/__init__.py

@@ -127,33 +127,6 @@ def _execute_unwinders(pending_frame):
     return None
 
 
-def _execute_file(filepath):
-    """This function is used to replace Python 2's PyRun_SimpleFile.
-
-    Loads and executes the given file.
-
-    We could use the runpy module, but its documentation says:
-    "Furthermore, any functions and classes defined by the executed code are
-    not guaranteed to work correctly after a runpy function has returned."
-    """
-    globals = sys.modules["__main__"].__dict__
-    set_file = False
-    # Set file (if not set) so that the imported file can use it (e.g. to
-    # access file-relative paths). This matches what PyRun_SimpleFile does.
-    if not hasattr(globals, "__file__"):
-        globals["__file__"] = filepath
-        set_file = True
-    try:
-        with open(filepath, "rb") as file:
-            # We pass globals also as locals to match what Python does
-            # in PyRun_SimpleFile.
-            compiled = compile(file.read(), filepath, "exec")
-            exec(compiled, globals, globals)
-    finally:
-        if set_file:
-            del globals["__file__"]
-
-
 # Convenience variable to GDB's python directory
 PYTHONDIR = os.path.dirname(os.path.dirname(__file__))
 

gdb/python/python.c

@@ -284,12 +284,14 @@ gdbpy_check_quit_flag (const struct extension_language_defn *extlang)
   return PyOS_InterruptOccurred ();
 }
 
-/* Evaluate a Python command like PyRun_SimpleString, but uses
-   Py_single_input which prints the result of expressions, and does
-   not automatically print the stack on errors.  */
+/* Evaluate a Python command like PyRun_SimpleString, but takes a
+   Python start symbol, and does not automatically print the stack on
+   errors.  FILENAME is used to set the file name in error
+   messages.  */
 
 static int
-eval_python_command (const char *command)
+eval_python_command (const char *command, int start_symbol,
+		     const char *filename = "<string>")
 {
   PyObject *m, *d;
 
@@ -300,8 +302,15 @@ eval_python_command (const char *command)
   d = PyModule_GetDict (m);
   if (d == NULL)
     return -1;
-  gdbpy_ref<> v (PyRun_StringFlags (command, Py_single_input, d, d, NULL));
-  if (v == NULL)
+
+  /* Use this API because it is in Python 3.2.  */
+  gdbpy_ref<> code (Py_CompileStringExFlags (command, filename, start_symbol,
+					     nullptr, -1));
+  if (code == nullptr)
+    return -1;
+
+  gdbpy_ref<> result (PyEval_EvalCode (code.get (), d, d));
+  if (result == nullptr)
     return -1;
 
   return 0;
@@ -324,7 +333,8 @@ python_interactive_command (const char *arg, int from_tty)
   if (arg && *arg)
     {
       std::string script = std::string (arg) + "\n";
-      err = eval_python_command (script.c_str ());
+      /* Py_single_input causes the result to be displayed.  */
+      err = eval_python_command (script.c_str (), Py_single_input);
     }
   else
     {
@@ -333,14 +343,12 @@ python_interactive_command (const char *arg, int from_tty)
     }
 
   if (err)
-    {
-      gdbpy_print_stack ();
-      error (_("Error while executing Python code."));
-    }
+    gdbpy_handle_exception ();
 }
 
-/* A wrapper around PyRun_SimpleFile.  FILE is the Python script to run
-   named FILENAME.
+/* Like PyRun_SimpleFile, but if there is an exception, it is not
+   automatically displayed.  FILE is the Python script to run named
+   FILENAME.
 
    On Windows hosts few users would build Python themselves (this is no
    trivial task on this platform), and thus use binaries built by
@@ -349,39 +357,13 @@ python_interactive_command (const char *arg, int from_tty)
    library.  Python, being built with VC, would use one version of the
    msvcr DLL (Eg. msvcr100.dll), while MinGW uses msvcrt.dll.
    A FILE * from one runtime does not necessarily operate correctly in
-   the other runtime.
+   the other runtime.  */
 
-   To work around this potential issue, we run code in Python to load
-   the script.  */
-
-static void
+static int
 python_run_simple_file (FILE *file, const char *filename)
 {
-#ifndef _WIN32
-
-  PyRun_SimpleFile (file, filename);
-
-#else /* _WIN32 */
-
-  /* Because we have a string for a filename, and are using Python to
-     open the file, we need to expand any tilde in the path first.  */
-  gdb::unique_xmalloc_ptr<char> full_path (tilde_expand (filename));
-
-  if (gdb_python_module == nullptr
-      || ! PyObject_HasAttrString (gdb_python_module, "_execute_file"))
-    error (_("Installation error: gdb._execute_file function is missing"));
-
-  gdbpy_ref<> return_value
-    (PyObject_CallMethod (gdb_python_module, "_execute_file", "s",
-			  full_path.get ()));
-  if (return_value == nullptr)
-    {
-      /* Use PyErr_PrintEx instead of gdbpy_print_stack to better match the
-	 behavior of the non-Windows codepath.  */
-      PyErr_PrintEx(0);
-    }
-
-#endif /* _WIN32 */
+  std::string contents = read_remainder_of_file (file);
+  return eval_python_command (contents.c_str (), Py_file_input, filename);
 }
 
 /* Given a command_line, return a command string suitable for passing
@@ -408,17 +390,15 @@ static void
 gdbpy_eval_from_control_command (const struct extension_language_defn *extlang,
 				 struct command_line *cmd)
 {
-  int ret;
-
   if (cmd->body_list_1 != nullptr)
     error (_("Invalid \"python\" block structure."));
 
   gdbpy_enter enter_py;
 
   std::string script = compute_python_string (cmd->body_list_0.get ());
-  ret = PyRun_SimpleString (script.c_str ());
-  if (ret)
-    error (_("Error while executing Python code."));
+  int ret = eval_python_command (script.c_str (), Py_file_input);
+  if (ret != 0)
+    gdbpy_handle_exception ();
 }
 
 /* Implementation of the gdb "python" command.  */
@@ -433,8 +413,9 @@ python_command (const char *arg, int from_tty)
   arg = skip_spaces (arg);
   if (arg && *arg)
     {
-      if (PyRun_SimpleString (arg))
-	error (_("Error while executing Python code."));
+      int ret = eval_python_command (arg, Py_file_input);
+      if (ret != 0)
+	gdbpy_handle_exception ();
     }
   else
     {
@@ -1050,7 +1031,9 @@ gdbpy_source_script (const struct extension_language_defn *extlang,
 		     FILE *file, const char *filename)
 {
   gdbpy_enter enter_py;
-  python_run_simple_file (file, filename);
+  int result = python_run_simple_file (file, filename);
+  if (result != 0)
+    gdbpy_handle_exception ();
 }
 
 
@@ -1682,7 +1665,9 @@ gdbpy_source_objfile_script (const struct extension_language_defn *extlang,
   scoped_restore restire_current_objfile
     = make_scoped_restore (&gdbpy_current_objfile, objfile);
 
-  python_run_simple_file (file, filename);
+  int result = python_run_simple_file (file, filename);
+  if (result != 0)
+    gdbpy_print_stack ();
 }
 
 /* Set the current objfile to OBJFILE and then execute SCRIPT
@@ -1703,7 +1688,9 @@ gdbpy_execute_objfile_script (const struct extension_language_defn *extlang,
   scoped_restore restire_current_objfile
     = make_scoped_restore (&gdbpy_current_objfile, objfile);
 
-  PyRun_SimpleString (script);
+  int ret = eval_python_command (script, Py_file_input);
+  if (ret != 0)
+    gdbpy_print_stack ();
 }
 
 /* Return the current Objfile, or None if there isn't one.  */
@@ -2361,21 +2348,15 @@ test_python ()
       {
 	CMD (output);
       }
-    catch (const gdb_exception &e)
+    catch (const gdb_exception_quit &e)
       {
 	saw_exception = true;
-	SELF_CHECK (e.reason == RETURN_ERROR);
-	SELF_CHECK (e.error == GENERIC_ERROR);
-	SELF_CHECK (*e.message == "Error while executing Python code.");
+	SELF_CHECK (e.reason == RETURN_QUIT);
+	SELF_CHECK (e.error == GDB_NO_ERROR);
+	SELF_CHECK (*e.message == "Quit");
       }
     SELF_CHECK (saw_exception);
-    std::string ref_output_0 ("Traceback (most recent call last):\n"
-			      "  File \"<string>\", line 0, in <module>\n"
-			      "KeyboardInterrupt\n");
-    std::string ref_output_1 ("Traceback (most recent call last):\n"
-			      "  File \"<string>\", line 1, in <module>\n"
-			      "KeyboardInterrupt\n");
-    SELF_CHECK (output == ref_output_0 || output == ref_output_1);
+    SELF_CHECK (output.empty ());
   }
 
 #undef CMD

gdb/testsuite/gdb.ada/tasks.exp

@@ -98,15 +98,15 @@ if {[allow_python_tests]} {
     gdb_test "python print(bp.thread)" "None"
     gdb_test "python bp.thread = 1" \
 	[multi_line \
-	     "RuntimeError: Cannot set both task and thread attributes\\." \
-	     "Error while executing Python code\\."] \
+	     "RuntimeError.*: Cannot set both task and thread attributes\\." \
+	     "Error occurred in Python.*"] \
 	"try setting the thread, but expect an error"
     gdb_test_no_output "python bp.task = None"
     gdb_test_no_output "python bp.thread = 1"
     gdb_test "python bp.task = 3" \
 	[multi_line \
-	     "RuntimeError: Cannot set both task and thread attributes\\." \
-	     "Error while executing Python code\\."] \
+	     "RuntimeError.*: Cannot set both task and thread attributes\\." \
+	     "Error occurred in Python.*"] \
 	"try setting the task, but expect an error"
 
     # Reset the breakpoint to the state required for the rest of this

gdb/testsuite/gdb.python/py-arch.exp

@@ -91,7 +91,7 @@ foreach size {0 1 2 3 4 8 16} {
 }
 
 gdb_test "python arch.integer_type(95)" \
-    ".*ValueError: no integer type of that size is available.*" \
+    ".*ValueError.* no integer type of that size is available.*" \
     "call integer_type with invalid size"
 
 # Test for gdb.architecture_names().  First we're going to grab the

gdb/testsuite/gdb.python/py-block.exp

@@ -44,9 +44,9 @@ gdb_test "python print (block.function)" "None" "first anonymous block"
 gdb_test "python print (block.start)" "${decimal}" "check start not None"
 gdb_test "python print (block.end)" "${decimal}" "check end not None"
 gdb_test "python print (block\['f'\].name == 'f')" "True" "check variable access"
-gdb_test "python print (block\['nonexistent'\])" ".*KeyError: 'nonexistent'.*" \
+gdb_test "python print (block\['nonexistent'\])" ".*KeyError.*: nonexistent.*" \
          "check nonexistent variable"
-gdb_test "python print (block\[42\])" ".*TypeError: Expected a string.*" \
+gdb_test "python print (block\[42\])" ".*TypeError.*: Expected a string.*" \
          "check non-string key"
 
 # Test global/static blocks

gdb/testsuite/gdb.python/py-breakpoint.exp

@@ -190,7 +190,7 @@ proc_with_prefix test_bkpt_deletion { } {
     gdb_py_test_silent_cmd  "python dp1.delete()" \
 	"Delete Breakpoint" 0
     gdb_test "python print (dp1.number)" \
-	"RuntimeError: Breakpoint 2 is invalid.*" \
+	"RuntimeError.*: Breakpoint 2 is invalid.*" \
 	"Check breakpoint invalidated"
     gdb_py_test_silent_cmd "python del_list = gdb.breakpoints()" \
 	"Get Breakpoint List" 0
@@ -275,7 +275,7 @@ proc_with_prefix test_bkpt_thread_and_inferior { } {
 	gdb_test "python print(bp.thread)" "1"
 	gdb_test "python print(bp.inferior)" "None"
 	gdb_test "python bp.inferior = 1" \
-	    "RuntimeError: Cannot have both 'thread' and 'inferior' conditions on a breakpoint.*"
+	    "RuntimeError.*: Cannot have both 'thread' and 'inferior' conditions on a breakpoint.*"
 	gdb_test_no_output "python bp.thread = None"
 	gdb_test_no_output "python bp.inferior = 1" \
 	    "set the inferior now the thread has been cleared"
@@ -289,7 +289,7 @@ proc_with_prefix test_bkpt_thread_and_inferior { } {
 	gdb_test "python print(bp.thread)" "None"
 	gdb_test "python print(bp.inferior)" "1"
 	gdb_test "python bp.thread = 1" \
-	    "RuntimeError: Cannot have both 'thread' and 'inferior' conditions on a breakpoint.*"
+	    "RuntimeError.*: Cannot have both 'thread' and 'inferior' conditions on a breakpoint.*"
 	gdb_test_no_output "python bp.inferior = None"
 	gdb_test_no_output "python bp.thread = 1" \
 	    "set the thread now the inferior has been cleared"
@@ -522,7 +522,7 @@ proc_with_prefix test_bkpt_eval_funcs { } {
 	"end" ""
 
     gdb_test "python eval_bp2.stop = stop_func"  \
-	"RuntimeError: Only one stop condition allowed.  There is currently a GDB.*" \
+	"RuntimeError.*: Only one stop condition allowed.  There is currently a GDB.*" \
 	"assign stop function to a breakpoint that has a condition"
 
     delete_breakpoints
@@ -589,7 +589,7 @@ proc_with_prefix test_bkpt_temporary { } {
 	".*$srcfile:$ibp_location.*"
     gdb_test "python print (ibp.count)" "1" \
 	"Check temporary stop callback executed before deletion."
-    gdb_test "python print (ibp.temporary)" "RuntimeError: Breakpoint 2 is invalid.*" \
+    gdb_test "python print (ibp.temporary)" "RuntimeError.*: Breakpoint 2 is invalid.*" \
 	"Check temporary breakpoint is deleted after being hit"
     gdb_test "info breakpoints" "No breakpoints, watchpoints, tracepoints, or catchpoints.*" \
 	"Check info breakpoints shows temporary breakpoint is deleted"
@@ -707,7 +707,7 @@ proc_with_prefix test_bkpt_explicit_loc {} {
 
     delete_breakpoints
     gdb_test "python bp1 = gdb.Breakpoint (line=bp1)" \
-	"RuntimeError: Line keyword should be an integer or a string.*" \
+	"RuntimeError.*: Line keyword should be an integer or a string.*" \
 	"set explicit breakpoint by invalid line type"
 
     delete_breakpoints
@@ -736,7 +736,7 @@ proc_with_prefix test_bkpt_explicit_loc {} {
 
     delete_breakpoints
     gdb_test "python bp1 = gdb.Breakpoint (source=\"$srcfile\")" \
-	"RuntimeError: Specifying a source must also include a line, label or function.*" \
+	"RuntimeError.*: Specifying a source must also include a line, label or function.*" \
 	"set invalid explicit breakpoint by source only"
 
     gdb_test "python bp1 = gdb.Breakpoint (source=\"foo.c\", line=\"5\")" \
@@ -859,8 +859,8 @@ proc_with_prefix test_catchpoints {} {
     # the python api.
     gdb_test "python gdb.Breakpoint (\"syscall\", type=gdb.BP_CATCHPOINT)" \
 	[multi_line \
-	     "gdb.error: BP_CATCHPOINT not supported" \
-	     "Error while executing Python code\\."] \
+	     "gdb.error.*: BP_CATCHPOINT not supported" \
+	     "Error occurred in Python:.*"] \
 	"create a catchpoint via the api"
 
     # Setup a catchpoint.

gdb/testsuite/gdb.python/py-connection.exp

@@ -65,10 +65,10 @@ gdb_test "python print(conn.is_valid())" "False" "is_valid returns False"
 # Now check that accessing properties of the invalid connection cases
 # an error.
 gdb_test "python print(conn.num)" \
-    "RuntimeError: Connection no longer exists\\.\r\n.*"
+    "RuntimeError.*: Connection no longer exists\\.\r\n.*"
 gdb_test "python print(conn.type)" \
-    "RuntimeError: Connection no longer exists\\.\r\n.*"
+    "RuntimeError.*: Connection no longer exists\\.\r\n.*"
 gdb_test "python print(conn.description)" \
-    "RuntimeError: Connection no longer exists\\.\r\n.*"
+    "RuntimeError.*: Connection no longer exists\\.\r\n.*"
 gdb_test "python print(conn.details)" \
-    "RuntimeError: Connection no longer exists\\.\r\n.*"
+    "RuntimeError.*: Connection no longer exists\\.\r\n.*"