From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26118 invoked by alias); 28 Sep 2011 23:20:21 -0000 Received: (qmail 26107 invoked by uid 22791); 28 Sep 2011 23:20:20 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,TW_BJ X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 28 Sep 2011 23:20:06 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 6766D2BB48B; Wed, 28 Sep 2011 19:20:05 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id QJG69sY+YWAK; Wed, 28 Sep 2011 19:20:05 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id B077D2BB425; Wed, 28 Sep 2011 19:20:03 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id ABC3C145615; Wed, 28 Sep 2011 19:19:57 -0400 (EDT) From: Joel Brobecker To: gdb-patches@sourceware.org Cc: Joel Brobecker Subject: [RFC] Crash sourcing Python script on Windows Date: Wed, 28 Sep 2011 23:24:00 -0000 Message-Id: <1317251996-12146-1-git-send-email-brobecker@adacore.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-09/txt/msg00483.txt.bz2 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