[patch] Do not open Python scripts twice #2 [Re: [RFC] Crash sourcing Python script on Windows]

[Pedro Alves]

On 01/23/2012 06:11 PM, Jan Kratochvil wrote:
> Hello Joel,
> 
> On Thu, 29 Sep 2011 01:19:56 +0200, Joel Brobecker wrote:
>> 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.
> 
> follow-up to:
> 	[patch] Code cleanup-like: Do not open Python scripts twice
> 	http://sourceware.org/ml/gdb-patches/2012-01/msg00774.html
> 
> While I agree it was not such a big degradation of GDB I do not find
> acceptable degrading normal platforms just due to MS-Windows defects.

Agreed.  To be honest, it's not a defect, it's a personality trait...

> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -1566,6 +1566,46 @@ aix*)
>    ;;
>  esac
>  
> +# MS-Windows platform may have Python compiled with different libc
> +# having the FILE structure layout incompatible between libraries.

It is not just about the layout.  The layout of both libc's could be the
same, but you'd still (maybe) crash.  It's more about the private global
state maintained by each of the libc's.  E.g., file descriptor N of
gdb's libc has no meaning the python's libc -- each libc has its own
file descriptor table.


> +if test "${have_libpython}" != no; then
> +  AC_CACHE_CHECK([whether FILE * is compatible with Python],
> +		 [gdb_cv_python_filep_compat], [
> +    old_CFLAGS="$CFLAGS"
> +    CFLAGS="$CFLAGS $PYTHON_CFLAGS"
> +    old_CPPFLAGS="$CPPFLAGS"
> +    CPPFLAGS="$CPPFLAGS $PYTHON_CPPFLAGS"
> +    old_LIBS="$LIBS"
> +    LIBS="$LIBS $PYTHON_LIBS"
> +    : >conftest.empty
> +    gdb_cv_python_filep_compat=no
> +    AC_RUN_IFELSE(
> +      AC_LANG_PROGRAM(
> +	[#include "]${have_libpython}[/Python.h"
> +	 #include <stdio.h>],
> +	[const char *filename = "conftest.empty";
> +	 FILE *f = fopen (filename, "r");
> +	 if (f == NULL)
> +	  return 1;
> +	 Py_Initialize ();
> +	 PyRun_File (f, filename, Py_file_input, PyDict_New (), PyDict_New ());

This is undefined behavior in the supposedly crashing case.  Hopefully this
doesn't happen to succeed by running some other random program.  :-)

The patch looks good to me.

-- 
Pedro Alves