[RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[Joel Brobecker]

This is a bit ugly...

A recent change introduced the include of "python-internal.h" from
inside breakpoint.h.  This in turn affected procfs.c:

| In file included from /usr/include/sys/procfs.h:29:0,
|                  from ../../src/gdb/core-regset.c:42:
| /usr/include/sys/old_procfs.h:39:2: error: #error "Cannot use procfs in the large file compilation environment"

What happens is that _FILE_OFFSET_BITS gets unconditionally re-defined
by pyconfig.h to a value that procfs.c does not support.  So far, we were
able to get by with the following #undef:

    /* On sparc-solaris, /usr/include/sys/feature_tests.h defines
       _FILE_OFFSET_BITS, which pyconfig.h also defines.  Same work
       around technique as above.  */
    #undef _FILE_OFFSET_BITS

...because procfs.c never included Python, even indirectly.  But this
is no longer sufficient, because the indirect dependency causes procfs.c
to get a value of _FILE_OFFSET_BITS that it does not support.

As I was looking at this, I looked at the configure script in Python 2.7,
and it appears that disabling large-file support is not possible.
Regardless of that, I think we want to be able to build GDB against
pre-built versions of Python if we can...

So the fix I applied was to make sure we restore the initial value
of _FILE_OFFSET_BITS after having included Python.h.

gdb/ChangeLog:

        * python/python-internal.h (_FILE_OFFSET_BITS): Restore initial value
        after having included "Python.h".

It fixes the problem on sparc-solaris. Tested on x86_64-linux.
OK to commit?

---
 gdb/python/python-internal.h |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 30d7533..026a05a 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -37,9 +37,16 @@
 #undef _XOPEN_SOURCE
 
 /* On sparc-solaris, /usr/include/sys/feature_tests.h defines
-   _FILE_OFFSET_BITS, which pyconfig.h also defines.  Same work
-   around technique as above.  */
+   _FILE_OFFSET_BITS, which pyconfig.h also defines.  We cannot apply
+   the same technique as above, because the actual value is important.
+   If we let Python define _FILE_OFFSET_BITS to a value that is not
+   compatible with procfs, we will get a compilation error while trying
+   to include sys/procfs.h.  To avoid that, we save the current value,
+   and restore it after Python.h has been included.  */
+#ifdef _FILE_OFFSET_BITS
+#define OLD_FILE_OFFSET_BITS _FILE_OFFSET_BITS
 #undef _FILE_OFFSET_BITS
+#endif
 
 #if HAVE_LIBPYTHON2_4
 #include "python2.4/Python.h"
@@ -62,6 +69,13 @@ typedef int Py_ssize_t;
 #error "Unable to find usable Python.h"
 #endif
 
+/* Restore the original _FILE_OFFSET_BITS value if applicable.  */
+#ifdef OLD_FILE_OFFSET_BITS
+#undef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS OLD_FILE_OFFSET_BITS
+#undef OLD_FILE_OFFSET_BITS
+#endif
+
 /* If Python.h does not define WITH_THREAD, then the various
    GIL-related functions will not be defined.  However,
    PyGILState_STATE will be.  */
-- 
1.7.1

Re: [RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[Jan Kratochvil]

On Tue, 23 Nov 2010 02:13:54 +0100, Joel Brobecker wrote:
> +#ifdef _FILE_OFFSET_BITS
> +#define OLD_FILE_OFFSET_BITS _FILE_OFFSET_BITS
>  #undef _FILE_OFFSET_BITS
> +#endif
>  
>  #if HAVE_LIBPYTHON2_4
>  #include "python2.4/Python.h"
> @@ -62,6 +69,13 @@ typedef int Py_ssize_t;
>  #error "Unable to find usable Python.h"
>  #endif
>  
> +/* Restore the original _FILE_OFFSET_BITS value if applicable.  */
> +#ifdef OLD_FILE_OFFSET_BITS
> +#undef _FILE_OFFSET_BITS
> +#define _FILE_OFFSET_BITS OLD_FILE_OFFSET_BITS
> +#undef OLD_FILE_OFFSET_BITS
> +#endif
> +

Without any comments on the Solaris specifics such saving of macro content
cannot work.  cpp macros expand only during their final use, not as right hand
side of a #define line.

_FILE_OFFSET_BITS will have content `OLD_FILE_OFFSET_BITS' at the end, if it
had any content in the beginning.


Regards,
Jan

Re: [RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[Pedro Alves]

On Tuesday 23 November 2010 01:13:54, Joel Brobecker wrote:
> This is a bit ugly...
> 
> A recent change introduced the include of "python-internal.h" from
> inside breakpoint.h.  This in turn affected procfs.c:

How hard would it be to get rid of the need to include
python-internal.h in any header?  It's included by breakpoint.h
for PyObject,

    /* With a Python scripting enabled GDB, store a reference to the
       Python object that has been associated with this breakpoint.
       This is always NULL for a GDB that is not script enabled.  It
       can sometimes be NULL for enabled GDBs as not all breakpoint
       types are tracked by the Python scripting API.  */
    PyObject *py_bp_object;

Given that we only need to hold a pointer to this, thus the PyObject
need not be complete at this point, can we forward declare PyObject
instead somehow?  If not, can we perhaps come up with a wrapper type
that itself can be forward declared?  Or fallback to void* if nothing
else cleaner is possible?  The advantage would be that python-internal.h
would then only be included in the specific modules that require it,
in other words, it would be moved to breakpoint.c.

WDYT?

-- 
Pedro Alves

Re: [RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro>     /* With a Python scripting enabled GDB, store a reference to the
Pedro>        Python object that has been associated with this breakpoint.
Pedro>        This is always NULL for a GDB that is not script enabled.  It
Pedro>        can sometimes be NULL for enabled GDBs as not all breakpoint
Pedro>        types are tracked by the Python scripting API.  */
Pedro>     PyObject *py_bp_object;

Pedro> Given that we only need to hold a pointer to this, thus the PyObject
Pedro> need not be complete at this point, can we forward declare PyObject
Pedro> instead somehow?

Python defines it as a typedef for "struct _object *".  Yuck -- I'd
rather not copy that into our code.

Pedro> If not, can we perhaps come up with a wrapper type that itself
Pedro> can be forward declared?  Or fallback to void* if nothing else
Pedro> cleaner is possible?

Either of these works for me.

Pedro> The advantage would be that python-internal.h would then only be
Pedro> included in the specific modules that require it, in other words,
Pedro> it would be moved to breakpoint.c.

I think this would be good.  Python has turned out to be unclean enough
that it is better to work around its problems systematically.

Tom

Re: [RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[Pedro Alves]

On Tuesday 23 November 2010 15:21:41, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

> Pedro> Given that we only need to hold a pointer to this, thus the PyObject
> Pedro> need not be complete at this point, can we forward declare PyObject
> Pedro> instead somehow?
> 
> Python defines it as a typedef for "struct _object *".  Yuck -- I'd
> rather not copy that into our code.

I see that PyObject is an abstract base class, and that this particular
concrete class is struct breakpoint_object ...

> Pedro> If not, can we perhaps come up with a wrapper type that itself
> Pedro> can be forward declared?  Or fallback to void* if nothing else
> Pedro> cleaner is possible?
> 
> Either of these works for me.

... so we already have a wrapper type we can forward declare.

Here's a patch.  We can move the PyObject fallback typedef from defs.h
to varobj.c again.  I haven't looked to see if the PyObject pointers
in struct varobj have some other concrete type we could forward
declare instead.  Or why don't we #ifdef out those fields if building
without python.

Tested by building gdb with and without --with-python=no.

-- 
Pedro Alves

2010-11-23  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* breakpoint.h: No longer include python.h or python-internal.h.
	(struct breakpoint_object): Forward declare.
	* defs.h (PyObject) [!HAVE_PYTHON]: Don't define.
	* varobj.c (PyObject) [!HAVE_PYTHON]: Define.
	* python/py-breakpoint.c (build_bp_list): Cast py_bp_object to
	PyObject pointer.
	(gdbpy_breakpoint_created): Remove casts around py_bp_object
	accesses.

---
 gdb/breakpoint.h           |    8 ++------
 gdb/defs.h                 |    4 ----
 gdb/python/py-breakpoint.c |    6 +++---
 gdb/varobj.c               |    2 ++
 4 files changed, 7 insertions(+), 13 deletions(-)

Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h	2010-11-11 14:46:24.000000000 +0000
+++ src/gdb/breakpoint.h	2010-11-23 16:07:20.000000000 +0000
@@ -24,13 +24,9 @@
 #include "value.h"
 #include "vec.h"
 
-#if HAVE_PYTHON
-#include "python/python.h"
-#include "python/python-internal.h"
-#endif
-
 struct value;
 struct block;
+struct breakpoint_object;
 
 /* This is the maximum number of bytes a breakpoint instruction can take.
    Feel free to increase it.  It's just used in a few places to size
@@ -568,7 +564,7 @@ struct breakpoint
        This is always NULL for a GDB that is not script enabled.  It
        can sometimes be NULL for enabled GDBs as not all breakpoint
        types are tracked by the Python scripting API.  */
-    PyObject *py_bp_object;
+    struct breakpoint_object *py_bp_object;
 };
 
 typedef struct breakpoint *breakpoint_p;
Index: src/gdb/defs.h
===================================================================
--- src.orig/gdb/defs.h	2010-11-11 14:46:24.000000000 +0000
+++ src/gdb/defs.h	2010-11-23 16:05:28.000000000 +0000
@@ -1240,8 +1240,4 @@ void dummy_obstack_deallocate (void *obj
 extern void initialize_progspace (void);
 extern void initialize_inferiors (void);
 
-#ifndef HAVE_PYTHON
-typedef int PyObject;
-#endif
-
 #endif /* #ifndef DEFS_H */
Index: src/gdb/python/py-breakpoint.c
===================================================================
--- src.orig/gdb/python/py-breakpoint.c	2010-11-11 14:46:26.000000000 +0000
+++ src/gdb/python/py-breakpoint.c	2010-11-23 16:11:24.000000000 +0000
@@ -636,7 +636,7 @@ static int
 build_bp_list (struct breakpoint *b, void *arg)
 {
   PyObject *list = arg;
-  PyObject *bp = b->py_bp_object;
+  PyObject *bp = (PyObject *) b->py_bp_object;
   int iserr = 0;
 
   /* Not all breakpoints will have a companion Python object.
@@ -718,7 +718,7 @@ gdbpy_breakpoint_created (int num)
     {
       newbp->number = num;
       newbp->bp = bp;
-      newbp->bp->py_bp_object = (PyObject *) newbp;
+      newbp->bp->py_bp_object = newbp;
       Py_INCREF (newbp);
       ++bppy_live;
     }
@@ -746,7 +746,7 @@ gdbpy_breakpoint_deleted (int num)
   if (! bp)
     return;
 
-  bp_obj = ((breakpoint_object *) bp->py_bp_object);
+  bp_obj = bp->py_bp_object;
   if (bp_obj)
     {
       bp_obj->bp = NULL;
Index: src/gdb/varobj.c
===================================================================
--- src.orig/gdb/varobj.c	2010-11-19 16:49:24.000000000 +0000
+++ src/gdb/varobj.c	2010-11-23 16:23:17.000000000 +0000
@@ -39,6 +39,8 @@
 #if HAVE_PYTHON
 #include "python/python.h"
 #include "python/python-internal.h"
+#else
+typedef int PyObject;
 #endif
 
 /* Non-zero if we want to see trace of varobj level stuff.  */

Re: [RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[Phil Muldoon]

Pedro Alves <pedro@codesourcery.com> writes:

> On Tuesday 23 November 2010 15:21:41, Tom Tromey wrote:
>> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro,


> Here's a patch.  We can move the PyObject fallback typedef from defs.h
> to varobj.c again.  I haven't looked to see if the PyObject pointers
> in struct varobj have some other concrete type we could forward
> declare instead.  Or why don't we #ifdef out those fields if building
> without python.
>
> Tested by building gdb with and without --with-python=no.

I've no objection really, but wouldn't the PyObject typedef be better
suited to defs.h?

Cheers,

Phil

Re: [RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[Pedro Alves]

On Tuesday 23 November 2010 17:22:48, Phil Muldoon wrote:
> Pedro Alves <pedro@codesourcery.com> writes:

> > Here's a patch.  We can move the PyObject fallback typedef from defs.h
> > to varobj.c again.  I haven't looked to see if the PyObject pointers
> > in struct varobj have some other concrete type we could forward
> > declare instead.  Or why don't we #ifdef out those fields if building
> > without python.
> >
> > Tested by building gdb with and without --with-python=no.
> 
> I've no objection really, but wouldn't the PyObject typedef be better
> suited to defs.h?

It's only necessary in one file, and this is how it was before.
IMO, keeping it contained hints that we should avoid putting
uses of the type in core interfaces.

-- 
Pedro Alves

Re: [RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[Joel Brobecker]

> 2010-11-23  Pedro Alves  <pedro@codesourcery.com>
> 
> 	gdb/
> 	* breakpoint.h: No longer include python.h or python-internal.h.
> 	(struct breakpoint_object): Forward declare.
> 	* defs.h (PyObject) [!HAVE_PYTHON]: Don't define.
> 	* varobj.c (PyObject) [!HAVE_PYTHON]: Define.
> 	* python/py-breakpoint.c (build_bp_list): Cast py_bp_object to
> 	PyObject pointer.
> 	(gdbpy_breakpoint_created): Remove casts around py_bp_object
> 	accesses.

Thanks, Pedro! I confirm that this fixes the problem. 
Can we check it in? (review needed?)

-- 
Joel

Re: [RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[Pedro Alves]

On Wednesday 24 November 2010 17:20:02, Joel Brobecker wrote:
> > 2010-11-23  Pedro Alves  <pedro@codesourcery.com>
> > 
> > 	gdb/
> > 	* breakpoint.h: No longer include python.h or python-internal.h.
> > 	(struct breakpoint_object): Forward declare.
> > 	* defs.h (PyObject) [!HAVE_PYTHON]: Don't define.
> > 	* varobj.c (PyObject) [!HAVE_PYTHON]: Define.
> > 	* python/py-breakpoint.c (build_bp_list): Cast py_bp_object to
> > 	PyObject pointer.
> > 	(gdbpy_breakpoint_created): Remove casts around py_bp_object
> > 	accesses.
> 
> Thanks, Pedro! I confirm that this fixes the problem. 
> Can we check it in? (review needed?)

Thanks for confirming.  I've checked it in now.

-- 
Pedro Alves

Re: [RFA/Python] Fix procfs.c build failure on 32bit solaris (_FILE_OFFSET_BITS)

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> Here's a patch.  We can move the PyObject fallback typedef from defs.h
Pedro> to varobj.c again.  I haven't looked to see if the PyObject pointers
Pedro> in struct varobj have some other concrete type we could forward
Pedro> declare instead.  Or why don't we #ifdef out those fields if building
Pedro> without python.

Pedro> Tested by building gdb with and without --with-python=no.

Looks good to me.

Tom