From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 45542 invoked by alias); 15 Aug 2019 17:15:25 -0000 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 Received: (qmail 38488 invoked by uid 89); 15 Aug 2019 17:15:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-30.6 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy= X-HELO: mail-ot1-f68.google.com Received: from mail-ot1-f68.google.com (HELO mail-ot1-f68.google.com) (209.85.210.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 15 Aug 2019 17:15:15 +0000 Received: by mail-ot1-f68.google.com with SMTP id f17so7151683otq.4 for ; Thu, 15 Aug 2019 10:15:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=GzWaK9XCpmGDYfXrAbjI4lMC01WKBN3+4K2kkAffxno=; b=mLnEcd2cA8mfTo2Kv7EUThxv78yhLTEaO1ThYeJYlBS4qzJ8D3f61qkeheaZUvtLCf nsfJEIXoGPqg1qz1zAJueGIL5MzyBryHLykcBsyzg+/aobHeKn6JGNmVvhPyAU2qlYgb cA8RHgPj4svvItoywOITjYxzxI3bZ3PUtLpeyoP43QOz8d6qkFB3ZuYmhs9S+WUgNWRY qXocB2TAldslrU7OiFuESd9wADmRt3VO4wyucpKur0MAk9ZIRsw8MOVr9e0HqIWDykWg t/LrqwNtu4APjYQkaEkbzvmn0V/lBDltoWTtoBHK7dBzb7HHYdAMFFV2Mu+wUmYKc8Ot nfaA== MIME-Version: 1.0 References: <20190814000511.140810-1-cbiesinger@google.com> <20190814160611.104374-1-cbiesinger@google.com> <7c69bd9a-8fc3-f82b-4e68-57788151fb16@simark.ca> In-Reply-To: <7c69bd9a-8fc3-f82b-4e68-57788151fb16@simark.ca> From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Thu, 15 Aug 2019 17:15:00 -0000 Message-ID: Subject: Re: [PATCH v2] Make GDB compile with Python 3 on MinGW To: Simon Marchi Cc: Christian Biesinger via gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-08/txt/msg00334.txt.bz2 On Wed, Aug 14, 2019 at 9:58 PM Simon Marchi wrote: > > On 2019-08-14 12:06 p.m., Christian Biesinger via gdb-patches wrote: > > [Now with the comment updated too] > > > > PyFile_FromString and PyFile_AsFile have been removed in Python 3. > > There is no obvious replacement that works here, and we can't just > > pass our FILE* to a DLL in Windows because it may use a different > > C runtime. > > > > So we just call a Python function which reads and executes file > > contents. Care must be taken to execute it in the context of > > __main__. > > > > Tested by inverting the ifdef and running the testsuite on Debian > > Linux (even without the patch, I failed at running the testsuite > > on Windows). > > I haven't deciphered the Python part yet, but here are some early comments. > > Did you test with both Python 2 and 3? I did, yeah. > > gdb/ChangeLog: > > > > 2019-08-13 Christian Biesinger > > > > * python/lib/gdb/__init__.py: Add an execute_file function. > > * python/python.c (python_run_simple_file): Call gdb.execute_file > > on Windows. > > --- > > gdb/python/lib/gdb/__init__.py | 23 +++++++++++++++++++++++ > > gdb/python/python.c | 23 +++++++++++++++-------- > > 2 files changed, 38 insertions(+), 8 deletions(-) > > > > diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py > > index af74df80c8..f1adc5ffbe 100644 > > --- a/gdb/python/lib/gdb/__init__.py > > +++ b/gdb/python/lib/gdb/__init__.py > > @@ -106,6 +106,29 @@ def execute_unwinders(pending_frame): > > > > return None > > > > +def execute_file(filepath): > > I would suggest naming this "_execute_file", to emphasize that's internal, > not to be used by users. Done (and sent a separate patch to rename other internal functions) > > + """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 > > + if not hasattr(globals, '__file__'): > > + globals['__file__'] = filepath > > + set_file = True > > What does setting __file__ help with? A comment explaining why this is needed > would be nice. Done > > + try: > > + with open(filepath, 'rb') as file: > > + # We pass globals also as locals to match what Python does > > + # in PyRun_SimpleFile. > > + exec(compile(file.read(), filepath, 'exec'), globals, globals) > > Could you split this line (the exec and compile) in two? If there is an error > coming out of either of them, and we get a backtrace, it will be easier to know > which one it is if they are on separate lines. Done > > + finally: > > + if set_file: > > + del globals['__file__'] > > + > > > > # Convenience variable to GDB's python directory > > PYTHONDIR = os.path.dirname(os.path.dirname(__file__)) > > diff --git a/gdb/python/python.c b/gdb/python/python.c > > index 162470dcc0..617bc0b84c 100644 > > --- a/gdb/python/python.c > > +++ b/gdb/python/python.c > > @@ -323,9 +323,8 @@ python_interactive_command (const char *arg, int from_tty) > > A FILE * from one runtime does not necessarily operate correctly in > > the other runtime. > > > > - To work around this potential issue, we create on Windows hosts the > > - FILE object using Python routines, thus making sure that it is > > - compatible with the Python library. */ > > + To work around this potential issue, we run code in Python to load > > + the script. */ > > > > static void > > python_run_simple_file (FILE *file, const char *filename) > > @@ -339,14 +338,22 @@ python_run_simple_file (FILE *file, const char *filename) > > /* 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 full_path (tilde_expand (filename)); > > - gdbpy_ref<> python_file (PyFile_FromString (full_path.get (), (char *) "r")); > > - if (python_file == NULL) > > + > > + if (gdb_python_module == nullptr > > + || ! PyObject_HasAttrString (gdb_python_module, "execute_file")) > > { > > - gdbpy_print_stack (); > > - error (_("Error while opening file: %s"), full_path.get ()); > > + error (_("Installation error: gdb.execute_file function is missing")); > > + return; > > } > > > > - PyRun_SimpleFile (PyFile_AsFile (python_file.get ()), filename); > > + gdbpy_ref<> return_value > > + (PyObject_CallMethod (gdb_python_module, "execute_file", "s", full_path.get ())); > > Wrap the line above to fit in 80 columns: > > gdbpy_ref<> return_value > (PyObject_CallMethod (gdb_python_module, "execute_file", "s", > full_path.get ())); Oops, done. > > + 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); > > I don't think PyErr_PrintEx clears the Python error indicator, and I think we want > it cleared when we exit this function, so should we call PyErr_Clear here? It does, per https://docs.python.org/3/c-api/exceptions.html and https://docs.python.org/2/c-api/exceptions.html Christian