Message ID | 96198491-96D8-42F0-9956-1C2BC9277050@dell.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 30011 invoked by alias); 31 May 2018 17:12:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 29990 invoked by uid 89); 31 May 2018 17:12:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_NUMSUBJECT, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: esa3.dell-outbound.iphmx.com Received: from esa3.dell-outbound.iphmx.com (HELO esa3.dell-outbound.iphmx.com) (68.232.153.94) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 31 May 2018 17:12:18 +0000 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A2FfAQCnKxBbh8qZ6ERcHAEBAQQBAQoBA?= =?us-ascii?q?YQYgTUKmFWBWIEwX5JdgXgLhnMhNRcBAgEBAQEBAQIBAQIQAQEBCgsJCCgvgjU?= =?us-ascii?q?igxRRAT5pBBODIoIBqg+IQIFoCQGIN4ITgTMMiByCZIIkAockkUUHAo5hjRIBk?= =?us-ascii?q?RWBQwOCBnBQKgGCGIIgDgmOF2+OSIEZAQE?= X-IPAS-Result: =?us-ascii?q?A2FfAQCnKxBbh8qZ6ERcHAEBAQQBAQoBAYQYgTUKmFWBWIE?= =?us-ascii?q?wX5JdgXgLhnMhNRcBAgEBAQEBAQIBAQIQAQEBCgsJCCgvgjUigxRRAT5pBBODI?= =?us-ascii?q?oIBqg+IQIFoCQGIN4ITgTMMiByCZIIkAockkUUHAo5hjRIBkRWBQwOCBnBQKgG?= =?us-ascii?q?CGIIgDgmOF2+OSIEZAQE?= Received: from esa2.dell-outbound2.iphmx.com ([68.232.153.202]) by esa3.dell-outbound.iphmx.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 May 2018 12:10:35 -0500 From: <Paul.Koning@dell.com> Received: from ausxippc101.us.dell.com ([143.166.85.207]) by esa2.dell-outbound2.iphmx.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 May 2018 23:03:01 +0600 X-LoopCount0: from 10.166.136.214 X-DLP: DLP_GlobalPCIDSS To: <gdb-patches@sourceware.org> Subject: [PATCH] fix build failure with Python 3.7 Date: Thu, 31 May 2018 17:12:14 +0000 Message-ID: <96198491-96D8-42F0-9956-1C2BC9277050@dell.com> Content-Type: text/plain; charset="us-ascii" Content-ID: <BF86B9E49DCC08468DCD4274E2C3540B@dell.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 |
Commit Message
Koning, Paul
May 31, 2018, 5:12 p.m. UTC
This cures PR gdb/33470, build failure due to calling an internal API in Python that changed in V3.7. The code now uses the "inittab" mechanism in Python, which is the approved way to make compiled-in modules known to Python. Gdb starts properly and I can see the _gdb module. On my test system (a Mac) it's hard to get gdb to run at all, so the test suite is a bit problematic. What other tests are needed? paul gdb/ChangeLog: 2018-05-31 Paul Koning <paul_koning@dell.com> PR gdb/33470 * python/python.c (do_start_initialization): Avoid call to internal Python API. (PyInit__gdb): New function.
Comments
On Thursday, May 31 2018, Paul Koning wrote: > This cures PR gdb/33470, build failure due to calling an internal > API in Python that changed in V3.7. The code now uses the > "inittab" mechanism in Python, which is the approved way to > make compiled-in modules known to Python. > > Gdb starts properly and I can see the _gdb module. On my test > system (a Mac) it's hard to get gdb to run at all, so the > test suite is a bit problematic. What other tests are needed? Thanks for the patch, Paul. I'll run this on BuildBot and see what comes out of it. Meanwhile... > paul > > gdb/ChangeLog: > > 2018-05-31 Paul Koning <paul_koning@dell.com> > > PR gdb/33470 This bug number is actually from Python's bugzilla, not GDB's. So it's not correct to mention it here in the ChangeLog/commit message. AFAIK, there's no correspondent GDB bug filed for this issue. > * python/python.c (do_start_initialization): > Avoid call to internal Python API. > (PyInit__gdb): New function. > > diff --git a/gdb/python/python.c b/gdb/python/python.c > index c29e7d7a6b..89443aee25 100644 > --- a/gdb/python/python.c > +++ b/gdb/python/python.c > @@ -1667,6 +1667,14 @@ finalize_python (void *ignore) > restore_active_ext_lang (previous_active); > } > > +#ifdef IS_PY3K > +PyMODINIT_FUNC > +PyInit__gdb (void) > +{ > + return PyModule_Create (&python_GdbModuleDef); > +} > +#endif I think it's a good idea to add a comment to this function. > + > static bool > do_start_initialization () > { > @@ -1707,6 +1715,9 @@ do_start_initialization () > remain alive for the duration of the program's execution, so > it is not freed after this call. */ > Py_SetProgramName (progname_copy); > + > + /* Define _gdb as a built-in module. */ > + PyImport_AppendInittab ("_gdb", PyInit__gdb); > #else > Py_SetProgramName (progname.release ()); > #endif > @@ -1716,9 +1727,7 @@ do_start_initialization () > PyEval_InitThreads (); > > #ifdef IS_PY3K > - gdb_module = PyModule_Create (&python_GdbModuleDef); > - /* Add _gdb module to the list of known built-in modules. */ > - _PyImport_FixupBuiltin (gdb_module, "_gdb"); > + gdb_module = PyImport_ImportModule ("_gdb"); > #else > gdb_module = Py_InitModule ("_gdb", python_GdbMethods); > #endif
> On May 31, 2018, at 4:10 PM, Sergio Durigan Junior <sergiodj@redhat.com> wrote: > >> >> gdb/ChangeLog: >> >> 2018-05-31 Paul Koning <paul_koning@dell.com> >> >> PR gdb/33470 > > This bug number is actually from Python's bugzilla, not GDB's. So it's > not correct to mention it here in the ChangeLog/commit message. AFAIK, > there's no correspondent GDB bug filed for this issue. Ok, I removed that. > >> * python/python.c (do_start_initialization): >> Avoid call to internal Python API. >> (PyInit__gdb): New function. >> >> diff --git a/gdb/python/python.c b/gdb/python/python.c >> index c29e7d7a6b..89443aee25 100644 >> --- a/gdb/python/python.c >> +++ b/gdb/python/python.c >> @@ -1667,6 +1667,14 @@ finalize_python (void *ignore) >> restore_active_ext_lang (previous_active); >> } >> >> +#ifdef IS_PY3K >> +PyMODINIT_FUNC >> +PyInit__gdb (void) >> +{ >> + return PyModule_Create (&python_GdbModuleDef); >> +} >> +#endif > > I think it's a good idea to add a comment to this function. I added this (after the #ifdef): /* This is called via the PyImport_AppendInittab mechanism called during initialization, to make the built-in _gdb module known to Python. */ paul
On Thursday, May 31 2018, Paul Koning wrote: >> On May 31, 2018, at 4:10 PM, Sergio Durigan Junior <sergiodj@redhat.com> wrote: >> >>> >>> gdb/ChangeLog: >>> >>> 2018-05-31 Paul Koning <paul_koning@dell.com> >>> >>> PR gdb/33470 >> >> This bug number is actually from Python's bugzilla, not GDB's. So it's >> not correct to mention it here in the ChangeLog/commit message. AFAIK, >> there's no correspondent GDB bug filed for this issue. > > Ok, I removed that. > >> >>> * python/python.c (do_start_initialization): >>> Avoid call to internal Python API. >>> (PyInit__gdb): New function. >>> >>> diff --git a/gdb/python/python.c b/gdb/python/python.c >>> index c29e7d7a6b..89443aee25 100644 >>> --- a/gdb/python/python.c >>> +++ b/gdb/python/python.c >>> @@ -1667,6 +1667,14 @@ finalize_python (void *ignore) >>> restore_active_ext_lang (previous_active); >>> } >>> >>> +#ifdef IS_PY3K >>> +PyMODINIT_FUNC >>> +PyInit__gdb (void) >>> +{ >>> + return PyModule_Create (&python_GdbModuleDef); >>> +} >>> +#endif >> >> I think it's a good idea to add a comment to this function. > > I added this (after the #ifdef): > > /* This is called via the PyImport_AppendInittab mechanism called > during initialization, to make the built-in _gdb module known to > Python. */ Thanks. FWIW, I ran the full testsuite here and no regressions were found.
>>>>> <Paul.Koning@dell.com> writes: >> This bug number is actually from Python's bugzilla, not GDB's. So it's >> not correct to mention it here in the ChangeLog/commit message. AFAIK, >> there's no correspondent GDB bug filed for this issue. > Ok, I removed that. There may be other cases where we stick a URL of some non-gdb bug into the ChangeLog (or at least the commit message), and that seems like it would save some time for some future archaeologist, so I'd suggest doing that. Tom
On 31/05/18 22:20, Sergio Durigan Junior wrote: > On Thursday, May 31 2018, Paul Koning wrote: > >>> On May 31, 2018, at 4:10 PM, Sergio Durigan Junior <sergiodj@redhat.com> wrote: >>> >>>> >>>> gdb/ChangeLog: >>>> >>>> 2018-05-31 Paul Koning <paul_koning@dell.com> >>>> >>>> PR gdb/33470 >>> >>> This bug number is actually from Python's bugzilla, not GDB's. So it's >>> not correct to mention it here in the ChangeLog/commit message. AFAIK, >>> there's no correspondent GDB bug filed for this issue. >> >> Ok, I removed that. >> >>> >>>> * python/python.c (do_start_initialization): >>>> Avoid call to internal Python API. >>>> (PyInit__gdb): New function. >>>> >>>> diff --git a/gdb/python/python.c b/gdb/python/python.c >>>> index c29e7d7a6b..89443aee25 100644 >>>> --- a/gdb/python/python.c >>>> +++ b/gdb/python/python.c >>>> @@ -1667,6 +1667,14 @@ finalize_python (void *ignore) >>>> restore_active_ext_lang (previous_active); >>>> } >>>> >>>> +#ifdef IS_PY3K >>>> +PyMODINIT_FUNC >>>> +PyInit__gdb (void) >>>> +{ >>>> + return PyModule_Create (&python_GdbModuleDef); >>>> +} >>>> +#endif >>> >>> I think it's a good idea to add a comment to this function. >> >> I added this (after the #ifdef): >> >> /* This is called via the PyImport_AppendInittab mechanism called >> during initialization, to make the built-in _gdb module known to >> Python. */ > > Thanks. > > FWIW, I ran the full testsuite here and no regressions were found. > Hey Sergio, Thanks for doing this. Did you test it against a GDB built against 3.7 and a GDB build with a 2.x version of Python? Cheers Phil
On 01/06/18 02:27, Tom Tromey wrote: >>>>>> <Paul.Koning@dell.com> writes: > >>> This bug number is actually from Python's bugzilla, not GDB's. So it's >>> not correct to mention it here in the ChangeLog/commit message. AFAIK, >>> there's no correspondent GDB bug filed for this issue. > >> Ok, I removed that. > > There may be other cases where we stick a URL of some non-gdb bug into > the ChangeLog (or at least the commit message), and that seems like it > would save some time for some future archaeologist, so I'd suggest doing > that. > > Tom > Hi all, I think the best thing to do would be to create a GDB bug that then has a reference to the Python bug tracking system. There is a Red Hat bugzilla entry for this, but from an upstream perspective, that's neither here nor there. Cheers, Phil
On 05/31/2018 09:45 PM, Paul.Koning@dell.com wrote: >>> @@ -1667,6 +1667,14 @@ finalize_python (void *ignore) >>> restore_active_ext_lang (previous_active); >>> } >>> >>> +#ifdef IS_PY3K >>> +PyMODINIT_FUNC >>> +PyInit__gdb (void) >>> +{ >>> + return PyModule_Create (&python_GdbModuleDef); >>> +} >>> +#endif >> >> I think it's a good idea to add a comment to this function. > > I added this (after the #ifdef): > > /* This is called via the PyImport_AppendInittab mechanism called > during initialization, to make the built-in _gdb module known to > Python. */ Can the function be made static? I'm a little surprised to see the function being named "Py...", since that kind of looks like stepping in Python's namespace. Thanks, Pedro Alves
> On Jun 1, 2018, at 8:15 AM, Pedro Alves <palves@redhat.com> wrote: > > On 05/31/2018 09:45 PM, Paul.Koning@dell.com wrote: > >>>> @@ -1667,6 +1667,14 @@ finalize_python (void *ignore) >>>> restore_active_ext_lang (previous_active); >>>> } >>>> >>>> +#ifdef IS_PY3K >>>> +PyMODINIT_FUNC >>>> +PyInit__gdb (void) >>>> +{ >>>> + return PyModule_Create (&python_GdbModuleDef); >>>> +} >>>> +#endif >>> >>> I think it's a good idea to add a comment to this function. >> >> I added this (after the #ifdef): >> >> /* This is called via the PyImport_AppendInittab mechanism called >> during initialization, to make the built-in _gdb module known to >> Python. */ > > Can the function be made static? No; I did that first but PyMODINIT_FUNC is a #define that conflicts with "static". > I'm a little surprised to see the function being named "Py...", since > that kind of looks like stepping in Python's namespace. True. How about "init__gdb_module"? paul
On 06/01/2018 01:54 PM, Paul.Koning@dell.com wrote: >> Can the function be made static? > > No; I did that first but PyMODINIT_FUNC is a #define that conflicts with "static". I see. >> I'm a little surprised to see the function being named "Py...", since >> that kind of looks like stepping in Python's namespace. > > True. How about "init__gdb_module"? Sounds fine to me. Thanks, Pedro Alves
diff --git a/gdb/python/python.c b/gdb/python/python.c index c29e7d7a6b..89443aee25 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -1667,6 +1667,14 @@ finalize_python (void *ignore) restore_active_ext_lang (previous_active); } +#ifdef IS_PY3K +PyMODINIT_FUNC +PyInit__gdb (void) +{ + return PyModule_Create (&python_GdbModuleDef); +} +#endif + static bool do_start_initialization () { @@ -1707,6 +1715,9 @@ do_start_initialization () remain alive for the duration of the program's execution, so it is not freed after this call. */ Py_SetProgramName (progname_copy); + + /* Define _gdb as a built-in module. */ + PyImport_AppendInittab ("_gdb", PyInit__gdb); #else Py_SetProgramName (progname.release ()); #endif @@ -1716,9 +1727,7 @@ do_start_initialization () PyEval_InitThreads (); #ifdef IS_PY3K - gdb_module = PyModule_Create (&python_GdbModuleDef); - /* Add _gdb module to the list of known built-in modules. */ - _PyImport_FixupBuiltin (gdb_module, "_gdb"); + gdb_module = PyImport_ImportModule ("_gdb"); #else gdb_module = Py_InitModule ("_gdb", python_GdbMethods); #endif