[RFC] Crash sourcing Python script on Windows

[Joel Brobecker <brobecker at adacore dot com>]

Hello,

At long last, I forced myself to look into this. I had noticed
that "source script.py" did not work on Windows, and resulted
in the debugger crashing.  It turns out that the problem came
from the fact that I was using a binaries from python.org where
the definition of type FILE difers from the definition our
compiler uses.  The crash occurs when trying to call PyRun_SimpleFile
with a FILE created by GDB.

The approach we took is to create a new FILE descriptor using
Python routines, thus making sure that the FILE is compatible
with PyRun_SimpleFile.  There were two such calls, so we wrote
a wrapper to that function that only takes a filename, and creates
the FILE on the fly.  This avoids code duplication.

We typically build Python ourselves on all platforms, except
Windows, where the build setup is a lot different. That's why
we have only ever seen this problem there.  Still, this is
arguably a problem that could occur on other systems, even if
I somehow doubt it (this is a guess!). As a result, I decided
to apply the workaround unconditionally on all platforms.
I don't see this as a big performance impact in practice.

The internet if full of references to people hitting the same
problem.  So I think we should make an attempt at preventing
this from happening. As said above, building Python on Windows
is hard, so we shouldn't expect typical developers to do themselves.

The question, hence the RFC, is: can anyone see a better way?
I thought about reading the entire script from stream to a string,
and then feeding that to PyRun_SimpleString.  That would work too,
I think.   But reading a file into a string means that we'd have
to have the entire file in memory, whereas the python interpreter
might have a smarter strategy.

Anyways, open to other options...

gdb/ChangeLog:

        * python/python.c (python_run_simple_file): New function.
        (source_python_script, source_python_script_for_objfile):
        Replace call to PyRun_SimpleFile by call to
        python_run_simple_file.

Tested on x86_64-linux, and on x86-windows (with AdaCore's testsuite).

Thanks!
-- 
Joel

---
 gdb/python/python.c |   36 ++++++++++++++++++++++++++++++++++--
 1 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/gdb/python/python.c b/gdb/python/python.c
index 4ef5715..5fef62f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -134,6 +134,38 @@ ensure_python_env (struct gdbarch *gdbarch,
   return make_cleanup (restore_python_env, env);
 }
 
+/* A wrapper around PyRun_SimpleFile.  FILENAME is the name of
+   the Python script to run.
+
+   One of the parameters of PyRun_SimpleFile is a FILE *.
+   The problem is that type FILE is extremely system and compiler
+   dependent.  So, unless the Python library has been compiled using
+   the same build environment as GDB, we run the risk of getting
+   a crash due to inconsistencies between the definition used by GDB,
+   and the definition used by Python.  A mismatch can very likely
+   lead to a crash.  This is particularly visible on Windows, where
+   few users would build Python themselves (this is no trivial task
+   on this platform), and thus use binaries built by someone else
+   instead.
+
+   To work around this potential issue, we create the FILE object
+   using Python routines, thus making sure that it is compatible
+   with the Python library.  */
+
+static void
+python_run_simple_file (const char *filename)
+{
+  char *filename_copy;
+  PyObject* python_file;
+  struct cleanup *cleanup;
+
+  filename_copy = xstrdup (filename);
+  cleanup = make_cleanup (xfree, filename_copy);
+  python_file = PyFile_FromString(filename_copy, "r");
+  make_cleanup_py_decref (python_file);
+  PyRun_SimpleFile (PyFile_AsFile (python_file), filename);
+  do_cleanups (cleanup);
+}
 
 /* Given a command_line, return a command string suitable for passing
    to Python.  Lines in the string are separated by newlines.  The
@@ -573,7 +605,7 @@ source_python_script (FILE *stream, const char *file)
 
   /* Note: If an exception occurs python will print the traceback and
      clear the error indicator.  */
-  PyRun_SimpleFile (stream, file);
+  python_run_simple_file (file);
 
   do_cleanups (cleanup);
 }
@@ -917,7 +949,7 @@ source_python_script_for_objfile (struct objfile *objfile,
   cleanups = ensure_python_env (get_objfile_arch (objfile), current_language);
   gdbpy_current_objfile = objfile;
 
-  PyRun_SimpleFile (stream, file);
+  python_run_simple_file (file);
 
   do_cleanups (cleanups);
   gdbpy_current_objfile = NULL;
-- 
1.7.1