Re: [RFC] Crash sourcing Python script on Windows

[Joel Brobecker <brobecker@adacore.com>]

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

> Joel>         * python/python.c (python_run_simple_file): New function.
> Joel>         (source_python_script, source_python_script_for_objfile):
> Joel>         Replace call to PyRun_SimpleFile by call to
> Joel>         python_run_simple_file.
> 
> It seems good to me.  Some trivial nits follow.

Thanks for the review.

> It would be good if this mentioned the specific case of linking against
> different versions of the C runtime.

Attached is what I checked in. The paragraph on Windows is plagiarism
on what Kai said, so credits to him.  Let me know if you see some nits,
or if this wasn't what you had in mind, and I will fix them.

-- 
Joel

[-- Attachment #2: source-python-windows.diff --]
[-- Type: text/x-diff, Size: 3596 bytes --]

commit b783e81f56f6e4d4eeec839cd83084db83b8a787
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Mon Sep 26 17:55:04 2011 -0700

    Crash sourcing Python script on Windows
    
    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.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 625bbbb..de4fdc8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2011-10-03  Joel Brobecker  <brobecker@adacore.com>
+
+	* 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.
+
 2011-10-03  Paul Koning  <paul_koning@dell.com>
 
 	* python/py-value.c (valpy_get_address): Use Py_XINCREF.
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 4ef5715..61c5420 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -134,6 +134,45 @@ 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.
+
+   There is also the situation where the Python library and GDB
+   are using two different versions of the C runtime library.
+   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. 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.
+
+   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 +612,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 +956,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;