* [patch] Fix Python 3 build error on 32-bit hosts
@ 2015-02-04 17:43 Jan Kratochvil
2015-02-04 18:39 ` Paul_Koning
2015-02-04 18:58 ` Pedro Alves
0 siblings, 2 replies; 10+ messages in thread
From: Jan Kratochvil @ 2015-02-04 17:43 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2382 bytes --]
Hi,
on Fedora Rawhide (==22) i686 using --with-python=/usr/bin/python3 one gets:
gcc -g -I. -I. -I./common -I./config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -pthread -I/usr/include/guile/2.0 -I/usr/include/python3.4m -I/usr/include/python3.4m -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmissing-prototypes -Wdeclaration-after-statement -Wempty-body -Wmissing-parameter-type -Wold-style-declaration -Wold-style-definition -Wformat-nonliteral -Werror -c -o py-value.o -MT py-value.o -MMD -MP -MF .deps/py-value.Tpo -fno-strict-aliasing -DNDEBUG -fwrapv ./python/py-value.c
./python/py-value.c:1696:3: error: initialization from incompatible pointer type [-Werror]
valpy_hash, /*tp_hash*/
^
./python/py-value.c:1696:3: error: (near initialization for ‘value_object_type.tp_hash’) [-Werror]
cc1: all warnings being treated as errors
Makefile:2628: recipe for target 'py-value.o' failed
This is because in Python 2 tp_hash was:
typedef long (*hashfunc)(PyObject *);
while in Python 3 tp_hash is:
typedef Py_hash_t (*hashfunc)(PyObject *);
Py_hash_t is int for 32-bit hosts and long for 64-bit hosts. While on 32-bit
hosts sizeof(long)==sizeof(int) still the hashfunc type is formally
incompatible. As this patch should have no compiled code change it is not
really necessary for gdb-7.9, it would fix there just this non-fatal
compilation warning:
./python/py-value.c:1696:3: warning: initialization from incompatible pointer type
valpy_hash, /*tp_hash*/
^
./python/py-value.c:1696:3: warning: (near initialization for ‘value_object_type.tp_hash’)
A question is whether the autoconf test isn't an overkill. One could use
instead just:
#if PYTHON_ABI_VERSION >= 3
Also one could use that #if either just at the valpy_hash() definition or one
could provide Py_hash_t in gdb/defs.h or one could provide some GDB_Py_hash_t
in gdb/defs.h.
Tested compilation with:
python-devel-2.7.9-4.fc22.x86_64
python-devel-2.7.9-4.fc22.i686
python3-devel-3.4.2-4.fc22.x86_64
python3-devel-3.4.2-4.fc22.i686
Jan
[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 1724 bytes --]
gdb/
2015-02-04 Jan Kratochvil <jan.kratochvil@redhat.com>
* configure.ac <"${have_libpython}" != no>: Define Py_hash_t.
* python/py-value.c (valpy_fetch_lazy): Use it. Remove cast to the
return type.
* config.in: Regenerate.
* configure: Regenerate.
diff --git a/gdb/configure.ac b/gdb/configure.ac
index dfc6947..f335b7b 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1016,6 +1016,25 @@ if test "${have_libpython}" != no; then
]]), [python_has_threads=no], [python_has_threads=yes])
AC_MSG_RESULT(${python_has_threads})
CPPFLAGS="${saved_CPPFLAGS}"
+
+ AC_CACHE_CHECK([for Py_hash_t], gdb_cv_Py_hash_t,
+ old_CFLAGS="$CFLAGS"
+ CFLAGS="$CFLAGS $PYTHON_CFLAGS"
+ old_CPPFLAGS="$CPPFLAGS"
+ CPPFLAGS="$CPPFLAGS $PYTHON_CPPFLAGS"
+ AC_TRY_COMPILE(
+ [#include <Python.h>],
+ [Py_hash_t var;],
+ gdb_cv_Py_hash_t=yes,
+ gdb_cv_Py_hash_t=no
+ )
+ CPPFLAGS="$old_CPPFLAGS"
+ CFLAGS="$old_CFLAGS"
+ )
+ if test "x$gdb_cv_Py_hash_t" = "xno"; then
+ AC_DEFINE(Py_hash_t, long,
+ [Provide Python 3 Py_hash_t for Python 2.])
+ fi
else
# Even if Python support is not compiled in, we need to have this file
# included so that the "python" command, et.al., still exists.
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 4c4d36e..5a13777 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -895,10 +895,10 @@ valpy_fetch_lazy (PyObject *self, PyObject *args)
/* Calculate and return the address of the PyObject as the value of
the builtin __hash__ call. */
-static long
+static Py_hash_t
valpy_hash (PyObject *self)
{
- return (long) (intptr_t) self;
+ return (intptr_t) self;
}
enum valpy_opcode
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] Fix Python 3 build error on 32-bit hosts
2015-02-04 17:43 [patch] Fix Python 3 build error on 32-bit hosts Jan Kratochvil
@ 2015-02-04 18:39 ` Paul_Koning
2015-02-04 18:58 ` Pedro Alves
1 sibling, 0 replies; 10+ messages in thread
From: Paul_Koning @ 2015-02-04 18:39 UTC (permalink / raw)
To: jan.kratochvil; +Cc: gdb-patches
> On Feb 4, 2015, at 12:43 PM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
>
> Hi,
>
> on Fedora Rawhide (==22) i686 using --with-python=/usr/bin/python3 one gets:
>
> gcc -g -I. -I. -I./common -I./config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber -I./gnulib/import -Ibuild-gnulib/import -DTUI=1 -pthread -I/usr/include/guile/2.0 -I/usr/include/python3.4m -I/usr/include/python3.4m -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmissing-prototypes -Wdeclaration-after-statement -Wempty-body -Wmissing-parameter-type -Wold-style-declaration -Wold-style-definition -Wformat-nonliteral -Werror -c -o py-value.o -MT py-value.o -MMD -MP -MF .deps/py-value.Tpo -fno-strict-aliasing -DNDEBUG -fwrapv ./python/py-value.c
> ./python/py-value.c:1696:3: error: initialization from incompatible pointer type [-Werror]
> valpy_hash, /*tp_hash*/
> ^
> ./python/py-value.c:1696:3: error: (near initialization for ‘value_object_type.tp_hash’) [-Werror]
> cc1: all warnings being treated as errors
> Makefile:2628: recipe for target 'py-value.o' failed
>
> This is because in Python 2 tp_hash was:
> typedef long (*hashfunc)(PyObject *);
> while in Python 3 tp_hash is:
> typedef Py_hash_t (*hashfunc)(PyObject *);
>
> Py_hash_t is int for 32-bit hosts and long for 64-bit hosts. While on 32-bit
> hosts sizeof(long)==sizeof(int) still the hashfunc type is formally
> incompatible. As this patch should have no compiled code change it is not
> really necessary for gdb-7.9, it would fix there just this non-fatal
> compilation warning:
> ./python/py-value.c:1696:3: warning: initialization from incompatible pointer type
> valpy_hash, /*tp_hash*/
> ^
> ./python/py-value.c:1696:3: warning: (near initialization for ‘value_object_type.tp_hash’)
>
> A question is whether the autoconf test isn't an overkill. One could use
> instead just:
> #if PYTHON_ABI_VERSION >= 3
I would say yes. This has been done in other cases where types or signatures changed between the two versions. Given that it’s specifically a change in definition, it seems logical to test the Python version with a #if and use the old or the new definition accordingly.
paul
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] Fix Python 3 build error on 32-bit hosts
2015-02-04 17:43 [patch] Fix Python 3 build error on 32-bit hosts Jan Kratochvil
2015-02-04 18:39 ` Paul_Koning
@ 2015-02-04 18:58 ` Pedro Alves
2015-02-04 19:08 ` Pedro Alves
2015-02-04 19:20 ` [patchv2] " Jan Kratochvil
1 sibling, 2 replies; 10+ messages in thread
From: Pedro Alves @ 2015-02-04 18:58 UTC (permalink / raw)
To: Jan Kratochvil, gdb-patches
On 02/04/2015 06:43 PM, Jan Kratochvil wrote:
> A question is whether the autoconf test isn't an overkill.
I agree with Paul here. I think checking Python version is sufficient.
One could use
> instead just:
> #if PYTHON_ABI_VERSION >= 3
This
https://docs.python.org/3/c-api/object.html#PyObject_Hash
and a quick web search for Py_hash_t seems to indicate that we should
check for Python >=3.2, not just 3.
> Also one could use that #if either just at the valpy_hash() definition or one
> could provide Py_hash_t in gdb/defs.h or one could provide some GDB_Py_hash_t
> in gdb/defs.h.
I'd be fine with providing a fallback Py_hash_t in python/python-internal.h,
where we already do a series of fallback definitions and fixes for older
Python, such as e.g. Py_ssize_t.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] Fix Python 3 build error on 32-bit hosts
2015-02-04 18:58 ` Pedro Alves
@ 2015-02-04 19:08 ` Pedro Alves
2015-02-04 19:20 ` [patchv2] " Jan Kratochvil
1 sibling, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2015-02-04 19:08 UTC (permalink / raw)
To: Jan Kratochvil, gdb-patches
On 02/04/2015 07:58 PM, Pedro Alves wrote:
> On 02/04/2015 06:43 PM, Jan Kratochvil wrote:
>
>> A question is whether the autoconf test isn't an overkill.
>
> I agree with Paul here. I think checking Python version is sufficient.
>
> One could use
>> instead just:
>> #if PYTHON_ABI_VERSION >= 3
>
> This
>
> https://docs.python.org/3/c-api/object.html#PyObject_Hash
>
> and a quick web search for Py_hash_t seems to indicate that we should
> check for Python >=3.2, not just 3.
Seems like the easiest for that is PY_VERSION_HEX.
Supposedly this should work:
#if PY_VERSION_HEX >= 0x03020000
Thanks,
Pedro Alves
>
>> Also one could use that #if either just at the valpy_hash() definition or one
>> could provide Py_hash_t in gdb/defs.h or one could provide some GDB_Py_hash_t
>> in gdb/defs.h.
>
> I'd be fine with providing a fallback Py_hash_t in python/python-internal.h,
> where we already do a series of fallback definitions and fixes for older
> Python, such as e.g. Py_ssize_t.
>
> Thanks,
> Pedro Alves
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [patchv2] Fix Python 3 build error on 32-bit hosts
2015-02-04 18:58 ` Pedro Alves
2015-02-04 19:08 ` Pedro Alves
@ 2015-02-04 19:20 ` Jan Kratochvil
2015-02-04 19:27 ` Pedro Alves
2015-02-04 21:06 ` Paul_Koning
1 sibling, 2 replies; 10+ messages in thread
From: Jan Kratochvil @ 2015-02-04 19:20 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Paul_Koning
[-- Attachment #1: Type: text/plain, Size: 411 bytes --]
On Wed, 04 Feb 2015 19:58:18 +0100, Pedro Alves wrote:
> I agree with Paul here. I think checking Python version is sufficient.
I should point out that version checking is error prone to distro backports.
> I'd be fine with providing a fallback Py_hash_t in python/python-internal.h,
> where we already do a series of fallback definitions and fixes for older
> Python, such as e.g. Py_ssize_t.
Done.
Jan
[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 1238 bytes --]
gdb/
2015-02-04 Jan Kratochvil <jan.kratochvil@redhat.com>
* python/python-internal.h (Py_hash_t): Define it for Python <3.2.
* python/py-value.c (valpy_fetch_lazy): Use it. Remove cast to the
return type.
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 0ee8544..23d4755 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -169,6 +169,10 @@ typedef unsigned long gdb_py_ulongest;
#endif /* HAVE_LONG_LONG */
+#if PY_VERSION_HEX < 0x03020000
+typedef long Py_hash_t;
+#endif
+
/* Python 2.6 did not wrap Py_DECREF in 'do {...} while (0)', leading
to 'suggest explicit braces to avoid ambiguous ‘else’' gcc errors.
Wrap it ourselves, so that callers don't need to care. */
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 4c4d36e..5a13777 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -895,10 +895,10 @@ valpy_fetch_lazy (PyObject *self, PyObject *args)
/* Calculate and return the address of the PyObject as the value of
the builtin __hash__ call. */
-static long
+static Py_hash_t
valpy_hash (PyObject *self)
{
- return (long) (intptr_t) self;
+ return (intptr_t) self;
}
enum valpy_opcode
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patchv2] Fix Python 3 build error on 32-bit hosts
2015-02-04 19:20 ` [patchv2] " Jan Kratochvil
@ 2015-02-04 19:27 ` Pedro Alves
2015-02-04 19:34 ` [commit] " Jan Kratochvil
2015-02-04 21:06 ` Paul_Koning
1 sibling, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2015-02-04 19:27 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Paul_Koning
On 02/04/2015 08:20 PM, Jan Kratochvil wrote:
> On Wed, 04 Feb 2015 19:58:18 +0100, Pedro Alves wrote:
>> I agree with Paul here. I think checking Python version is sufficient.
>
> I should point out that version checking is error prone to distro backports.
Distros will be backporting Python fixes, not API/ABI breaking changes.
>> I'd be fine with providing a fallback Py_hash_t in python/python-internal.h,
>> where we already do a series of fallback definitions and fixes for older
>> Python, such as e.g. Py_ssize_t.
>
> Done.
OK.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* [commit] [patchv2] Fix Python 3 build error on 32-bit hosts
2015-02-04 19:27 ` Pedro Alves
@ 2015-02-04 19:34 ` Jan Kratochvil
2015-02-04 19:42 ` Pedro Alves
2015-02-04 19:51 ` Phil Muldoon
0 siblings, 2 replies; 10+ messages in thread
From: Jan Kratochvil @ 2015-02-04 19:34 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Paul_Koning
On Wed, 04 Feb 2015 20:27:16 +0100, Pedro Alves wrote:
> Distros will be backporting Python fixes, not API/ABI breaking changes.
Adding a new typedef (in the library namespace) I do not find as an API
breakage (for correctly written applications).
> OK.
Checked in:
881d5d5db08ee6b343e1f1fc560d785fed29428e
Thanks,
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [commit] [patchv2] Fix Python 3 build error on 32-bit hosts
2015-02-04 19:34 ` [commit] " Jan Kratochvil
@ 2015-02-04 19:42 ` Pedro Alves
2015-02-04 19:51 ` Phil Muldoon
1 sibling, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2015-02-04 19:42 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches, Paul_Koning
On 02/04/2015 08:34 PM, Jan Kratochvil wrote:
> On Wed, 04 Feb 2015 20:27:16 +0100, Pedro Alves wrote:
>> Distros will be backporting Python fixes, not API/ABI breaking changes.
>
> Adding a new typedef (in the library namespace) I do not find as an API
> breakage (for correctly written applications).
Changing the tp_hash's callback type is, as clearly evidenced by
the necessity of this fix.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [commit] [patchv2] Fix Python 3 build error on 32-bit hosts
2015-02-04 19:34 ` [commit] " Jan Kratochvil
2015-02-04 19:42 ` Pedro Alves
@ 2015-02-04 19:51 ` Phil Muldoon
1 sibling, 0 replies; 10+ messages in thread
From: Phil Muldoon @ 2015-02-04 19:51 UTC (permalink / raw)
To: Jan Kratochvil, Pedro Alves; +Cc: gdb-patches, Paul_Koning
On 04/02/15 19:34, Jan Kratochvil wrote:
> On Wed, 04 Feb 2015 20:27:16 +0100, Pedro Alves wrote:
>> Distros will be backporting Python fixes, not API/ABI breaking changes.
> Adding a new typedef (in the library namespace) I do not find as an API
> breakage (for correctly written applications).
It's not. The API promise only applies to explicitly exported APIs and objects from GDB Python
Cheers
Phil
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patchv2] Fix Python 3 build error on 32-bit hosts
2015-02-04 19:20 ` [patchv2] " Jan Kratochvil
2015-02-04 19:27 ` Pedro Alves
@ 2015-02-04 21:06 ` Paul_Koning
1 sibling, 0 replies; 10+ messages in thread
From: Paul_Koning @ 2015-02-04 21:06 UTC (permalink / raw)
To: jan.kratochvil; +Cc: palves, gdb-patches
> On Feb 4, 2015, at 2:20 PM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
>
> On Wed, 04 Feb 2015 19:58:18 +0100, Pedro Alves wrote:
>> I agree with Paul here. I think checking Python version is sufficient.
>
> I should point out that version checking is error prone to distro backports.
That’s true in many cases. But here we’re dealing with a documented API difference, and the definition explicitly states that the change is associated with a particular Python version (core version, not distro).
paul
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-02-04 21:06 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-04 17:43 [patch] Fix Python 3 build error on 32-bit hosts Jan Kratochvil
2015-02-04 18:39 ` Paul_Koning
2015-02-04 18:58 ` Pedro Alves
2015-02-04 19:08 ` Pedro Alves
2015-02-04 19:20 ` [patchv2] " Jan Kratochvil
2015-02-04 19:27 ` Pedro Alves
2015-02-04 19:34 ` [commit] " Jan Kratochvil
2015-02-04 19:42 ` Pedro Alves
2015-02-04 19:51 ` Phil Muldoon
2015-02-04 21:06 ` Paul_Koning
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).