[patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Jan Kratochvil]

On Thu, 30 Aug 2012 18:00:46 +0200, Khoo Yit Phang wrote:
> 2012-08-30  Khoo Yit Phang  <khooyp@cs.umd.edu>

This patch does not seem to require copyright assignment but it would be safer
to complete one for example from Sergio Durigan Junior <sergiodj@redhat.com>.
Also for any possible future contributions.


> 	Disable -lmcheck when Python is enabled with threading support.
> 	* configure.ac: Move --enable-lmcheck after --with-python, and
> 	disable if python_has_threads=yes.

Describe it more technically, new variable python_has_threads, check
${python_prog} --has-threads etc.


[...]
> +# Provide a --enable-libmcheck/--disable-libmcheck set of options
> +# allowing a user to enable this option even when building releases,
> +# or to disable it when building a snapshot.
> +AC_ARG_ENABLE(libmcheck,
> +  AS_HELP_STRING([--enable-libmcheck],
> +                 [Try building GDB with -lmcheck if available]),
> +  [case "${enableval}" in
> +     yes | y) ENABLE_LIBMCHECK="yes" ;;
> +     no | n)  ENABLE_LIBMCHECK="no" ;;
> +     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
> +   esac])
> +
> +# Enable -lmcheck by default (it provides cheap-enough memory mangling),
> +# but turn it off for releases.
> +if test -z "${ENABLE_LIBMCHECK}" && $development; then
> +  ENABLE_LIBMCHECK=yes
> +fi
> +
> +if test "$ENABLE_LIBMCHECK" = "yes" ; then
> +  if test "${have_libpython}" != "no" -a "${python_has_threads}" = "yes" ; then
> +    AC_MSG_WARN(libmcheck disabled since python has threads)
> +  else
> +    AC_CHECK_LIB(mcheck, main)
> +  fi
> +fi

I am fine with this goal of disabling -lmcheck by default (when
--enable-libmcheck was not specified) when threaded libpython is linked in.

But when user explicitly specifies --enable-libmcheck it should give the
warning but still enable libmcheck.  In most of the uses of GDB libmcheck is
safe with GDB.


> --- a/gdb/python/python-config.py
> +++ b/gdb/python/python-config.py
> @@ -1,5 +1,5 @@
>  # Program to fetch python compilation parameters.
> -# Copied from python-config of the 2.7 release.
> +# Copied from python-config of the 2.7 release, modified to test for threads.

Please submit it also for Python.  The commit in GDB sources is OK as long as
it gets imported by upstream Python.  They may also suggest some other
solution if they do not agree with this one.


Thanks for catching it.


Jan

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 861 bytes --]

Hi,

On Aug 30, 2012, at 12:43 PM, Jan Kratochvil wrote:

> On Thu, 30 Aug 2012 18:00:46 +0200, Khoo Yit Phang wrote:
>> 2012-08-30  Khoo Yit Phang  <khooyp@cs.umd.edu>
> 
> This patch does not seem to require copyright assignment but it would be safer
> to complete one for example from Sergio Durigan Junior <sergiodj@redhat.com>.
> Also for any possible future contributions.

I have copyright assignment set up, and commit access.

I've attached a new patch with the following changes: 1) I reverted python-config.py and used a different method to test for threads ("python -m threading"); 2) -lmcheck is not enabled by default when Python is available with reads; and 3) running configure with --enable-lmcheck when Python is available with threads now leads to a warning, rather than disabling -lmcheck.

Thanks,

Yit
August 30, 2012


[-- Attachment #2: mcheck-python.txt --]
[-- Type: text/plain, Size: 3590 bytes --]

# HG changeset patch
# Parent 38ae59cbbba369e1b4bd78b4b2f378e26548b79d
Do not enable -lmcheck by default when Python is enabled with threading support.

gdb/ChangeLog:

2012-08-30  Khoo Yit Phang  <khooyp@cs.umd.edu>

	Do not enable -lmcheck by default when Python is enabled with
	threading support.
	* configure.ac: (python_has_threads) New variable, by testing
	"python -m threading".
	Move --enable-lmcheck after --with-python.
	Do not enable -lmcheck by default if python_has_threads=yes.
	Warn if --enable-lmcheck=yes and python_has_threads=yes.
	* configure: Regenerate.

diff --git a/gdb/configure.ac b/gdb/configure.ac
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -645,28 +645,6 @@
 AC_SUBST(READLINE_CFLAGS)
 AC_SUBST(READLINE_TEXI_INCFLAG)
 
-# Provide a --enable-libmcheck/--disable-libmcheck set of options
-# allowing a user to enable this option even when building releases,
-# or to disable it when building a snapshot.
-AC_ARG_ENABLE(libmcheck,
-  AS_HELP_STRING([--enable-libmcheck],
-                 [Try building GDB with -lmcheck if available]),
-  [case "${enableval}" in
-     yes | y) ENABLE_LIBMCHECK="yes" ;;
-     no | n)  ENABLE_LIBMCHECK="no" ;;
-     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
-   esac])
-
-# Enable -lmcheck by default (it provides cheap-enough memory mangling),
-# but turn it off for releases.
-if test -z "${ENABLE_LIBMCHECK}" && $development; then
-    ENABLE_LIBMCHECK=yes
-fi
-
-if test "$ENABLE_LIBMCHECK" = "yes" ; then
-  AC_CHECK_LIB(mcheck, main)
-fi
-
 # Generate jit-reader.h
 
 # This is typedeffed to GDB_CORE_ADDR in jit-reader.h
@@ -882,6 +860,11 @@
         AC_ERROR(failure running python-config --exec-prefix)
       fi
     fi
+    if ${python_prog} -m threading >/dev/null 2>/dev/null; then
+      python_has_threads=yes
+    else
+      python_has_threads=no
+    fi
   else
     # We do not have a python executable we can use to determine where
     # to find the Python headers and libs.  We cannot guess the include
@@ -890,6 +873,7 @@
     # do except assume that the compiler will be able to find those files.
     python_includes=
     python_libs=
+    python_has_threads=yes
     have_python_config=no
   fi
 
@@ -1028,6 +1012,34 @@
 AC_SUBST(PYTHON_CPPFLAGS)
 AC_SUBST(PYTHON_LIBS)
 
+# Provide a --enable-libmcheck/--disable-libmcheck set of options
+# allowing a user to enable this option even when building releases,
+# or to disable it when building a snapshot.
+AC_ARG_ENABLE(libmcheck,
+  AS_HELP_STRING([--enable-libmcheck],
+                 [Try building GDB with -lmcheck if available]),
+  [case "${enableval}" in
+     yes | y) ENABLE_LIBMCHECK="yes" ;;
+     no | n)  ENABLE_LIBMCHECK="no" ;;
+     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
+   esac])
+
+# Enable -lmcheck by default (it provides cheap-enough memory mangling),
+# but turn it off if Python is enabled with threads, and for releases.
+if test -z "${ENABLE_LIBMCHECK}" \
+        -a \( "${have_libpython}" = "no" \
+	     -o "${python_has_threads}" = "no" \) \
+    && $development; then
+  ENABLE_LIBMCHECK=yes
+fi
+
+if test "$ENABLE_LIBMCHECK" = "yes" ; then
+  if test "${have_libpython}" != "no" -a "${python_has_threads}" = "yes" ; then
+    AC_MSG_WARN(--enable-libmcheck may lead to spurious crashes if threads are used in python)
+  fi
+  AC_CHECK_LIB(mcheck, main)
+fi
+
 # ------------------------- #
 # Checks for header files.  #
 # ------------------------- #

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Jan Kratochvil]

On Thu, 30 Aug 2012 20:05:01 +0200, Khoo Yit Phang wrote:
> 1) I reverted python-config.py and used a different method to test for
> threads ("python -m threading");

Unfortunately it does not work for me with python-2.7, only with python-3.3,
tested on Fedora 18 x86_64:

$ python3 -m threading;echo $?
0
$ python3 -m threadin;echo $?
/usr/bin/python3: No module named threadin
1

but:

$ python2 -m threading;echo $?
Traceback (most recent call last):
  File "/usr/lib64/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/usr/lib64/python2.7/threading.py", line 985, in <module>
    _test()
  File "/usr/lib64/python2.7/threading.py", line 969, in _test
    Q = BoundedQueue(QL)
  File "/usr/lib64/python2.7/threading.py", line 912, in __init__
    self.queue = deque()
NameError: global name 'deque' is not defined
255

python3-3.3.0-0.3.b2.fc18.x86_64
python-2.7.3-13.fc18.x86_64

GDB moveover currently is not compatible with python3.x at all.

I do not know much Python but I would guess thread-less Pythons are rare,
aren't they?  There would be also a possibility to assume Python is threaded,
if one already links with Python.


Otherwise I am fine with the patch.


Thanks,
Jan

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]

Hi,

On Aug 30, 2012, at 2:21 PM, Jan Kratochvil wrote:

> On Thu, 30 Aug 2012 20:05:01 +0200, Khoo Yit Phang wrote:
>> 1) I reverted python-config.py and used a different method to test for
>> threads ("python -m threading");
> 
> Unfortunately it does not work for me with python-2.7, only with python-3.3,
> tested on Fedora 18 x86_64:
> 
> $ python3 -m threading;echo $?
> 0
> $ python3 -m threadin;echo $?
> /usr/bin/python3: No module named threadin
> 1
> 
> but:
> 
> $ python2 -m threading;echo $?
> Traceback (most recent call last):
>  File "/usr/lib64/python2.7/runpy.py", line 162, in _run_module_as_main
>    "__main__", fname, loader, pkg_name)
>  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
>    exec code in run_globals
>  File "/usr/lib64/python2.7/threading.py", line 985, in <module>
>    _test()
>  File "/usr/lib64/python2.7/threading.py", line 969, in _test
>    Q = BoundedQueue(QL)
>  File "/usr/lib64/python2.7/threading.py", line 912, in __init__
>    self.queue = deque()
> NameError: global name 'deque' is not defined
> 255
> 
> python3-3.3.0-0.3.b2.fc18.x86_64
> python-2.7.3-13.fc18.x86_64
> 
> GDB moveover currently is not compatible with python3.x at all.
> 
> I do not know much Python but I would guess thread-less Pythons are rare,
> aren't they?  There would be also a possibility to assume Python is threaded,
> if one already links with Python.

That's strange, it works for me under Ubuntu 11.04 with Python 2.7.1 and RHEL5 with Python 2.6.6. I suppose it would be easier to just assume Python is threaded, this new patch does that. I'll check it in if it looks alright.

Thanks,

Yit
August 30, 2012


[-- Attachment #2: mcheck-python.txt --]
[-- Type: text/plain, Size: 2749 bytes --]

# HG changeset patch
# Parent 38ae59cbbba369e1b4bd78b4b2f378e26548b79d
Do not enable -lmcheck by default when Python is enabled.

gdb/ChangeLog:

2012-08-30  Khoo Yit Phang  <khooyp@cs.umd.edu>

	Do not enable -lmcheck by default when Python is enabled.
	* configure.ac: Move --enable-lmcheck after --with-python.
	Do not enable -lmcheck by default if have_libpython=yes.
	Warn if --enable-lmcheck=yes and have_libpython=yes.
	* configure: Regenerate.

diff --git a/gdb/configure.ac b/gdb/configure.ac
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -645,28 +645,6 @@
 AC_SUBST(READLINE_CFLAGS)
 AC_SUBST(READLINE_TEXI_INCFLAG)
 
-# Provide a --enable-libmcheck/--disable-libmcheck set of options
-# allowing a user to enable this option even when building releases,
-# or to disable it when building a snapshot.
-AC_ARG_ENABLE(libmcheck,
-  AS_HELP_STRING([--enable-libmcheck],
-                 [Try building GDB with -lmcheck if available]),
-  [case "${enableval}" in
-     yes | y) ENABLE_LIBMCHECK="yes" ;;
-     no | n)  ENABLE_LIBMCHECK="no" ;;
-     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
-   esac])
-
-# Enable -lmcheck by default (it provides cheap-enough memory mangling),
-# but turn it off for releases.
-if test -z "${ENABLE_LIBMCHECK}" && $development; then
-    ENABLE_LIBMCHECK=yes
-fi
-
-if test "$ENABLE_LIBMCHECK" = "yes" ; then
-  AC_CHECK_LIB(mcheck, main)
-fi
-
 # Generate jit-reader.h
 
 # This is typedeffed to GDB_CORE_ADDR in jit-reader.h
@@ -1028,6 +1006,32 @@
 AC_SUBST(PYTHON_CPPFLAGS)
 AC_SUBST(PYTHON_LIBS)
 
+# Provide a --enable-libmcheck/--disable-libmcheck set of options
+# allowing a user to enable this option even when building releases,
+# or to disable it when building a snapshot.
+AC_ARG_ENABLE(libmcheck,
+  AS_HELP_STRING([--enable-libmcheck],
+                 [Try building GDB with -lmcheck if available]),
+  [case "${enableval}" in
+     yes | y) ENABLE_LIBMCHECK="yes" ;;
+     no | n)  ENABLE_LIBMCHECK="no" ;;
+     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
+   esac])
+
+# Enable -lmcheck by default (it provides cheap-enough memory mangling),
+# but turn it off if Python is enabled with threads, and for releases.
+if test -z "${ENABLE_LIBMCHECK}" -a "${have_libpython}" = "no" \
+    && $development ; then
+  ENABLE_LIBMCHECK=yes
+fi
+
+if test "$ENABLE_LIBMCHECK" = "yes" ; then
+  if test "${have_libpython}" != "no" ; then
+    AC_MSG_WARN(--enable-libmcheck may lead to spurious crashes if threads are used in python)
+  fi
+  AC_CHECK_LIB(mcheck, main)
+fi
+
 # ------------------------- #
 # Checks for header files.  #
 # ------------------------- #

[-- Attachment #3: Type: text/plain, Size: 3 bytes --]

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Jan Kratochvil]

Hi David,

On Thu, 30 Aug 2012 20:58:21 +0200, Khoo Yit Phang wrote:
> On Aug 30, 2012, at 2:21 PM, Jan Kratochvil wrote:
> > On Thu, 30 Aug 2012 20:05:01 +0200, Khoo Yit Phang wrote:
> >> 1) I reverted python-config.py and used a different method to test for
> >> threads ("python -m threading");
> > 
> > Unfortunately it does not work for me with python-2.7, only with python-3.3,
> > tested on Fedora 18 x86_64:
> > 
> > $ python3 -m threading;echo $?
> > 0
> > $ python3 -m threadin;echo $?
> > /usr/bin/python3: No module named threadin
> > 1
> > 
> > but:
> > 
> > $ python2 -m threading;echo $?
> > Traceback (most recent call last):
> >  File "/usr/lib64/python2.7/runpy.py", line 162, in _run_module_as_main
> >    "__main__", fname, loader, pkg_name)
> >  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
> >    exec code in run_globals
> >  File "/usr/lib64/python2.7/threading.py", line 985, in <module>
> >    _test()
> >  File "/usr/lib64/python2.7/threading.py", line 969, in _test
> >    Q = BoundedQueue(QL)
> >  File "/usr/lib64/python2.7/threading.py", line 912, in __init__
> >    self.queue = deque()
> > NameError: global name 'deque' is not defined
> > 255
> > 
> > python3-3.3.0-0.3.b2.fc18.x86_64
> > python-2.7.3-13.fc18.x86_64
> > 
> > GDB moveover currently is not compatible with python3.x at all.
> > 
> > I do not know much Python but I would guess thread-less Pythons are rare,
> > aren't they?  There would be also a possibility to assume Python is threaded,
> > if one already links with Python.
> 
> That's strange, it works for me under Ubuntu 11.04 with Python 2.7.1 and
> RHEL5 with Python 2.6.6. I suppose it would be easier to just assume Python
> is threaded, this new patch does that.

On RHEL-5.8 I have python-2.4.3-46.el5 (not Python 2.6.6) although I can
confirm it works there:

$ python -m threading; echo $?
Producer-1.1
[...]
Producer-3.5
0


> I'll check it in if it looks alright.

Please wait till Python experts will comment it.


Thanks,
Jan

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Khoo Yit Phang]

Hi,

On Aug 30, 2012, at 3:05 PM, Jan Kratochvil wrote:

> Hi David,
> 
> On Thu, 30 Aug 2012 20:58:21 +0200, Khoo Yit Phang wrote:
>> On Aug 30, 2012, at 2:21 PM, Jan Kratochvil wrote:
>>> On Thu, 30 Aug 2012 20:05:01 +0200, Khoo Yit Phang wrote:
>>>> 1) I reverted python-config.py and used a different method to test for
>>>> threads ("python -m threading");
>>> 
>>> Unfortunately it does not work for me with python-2.7, only with python-3.3,
>>> tested on Fedora 18 x86_64:
>>> 
>>> $ python3 -m threading;echo $?
>>> 0
>>> $ python3 -m threadin;echo $?
>>> /usr/bin/python3: No module named threadin
>>> 1
>>> 
>>> but:
>>> 
>>> $ python2 -m threading;echo $?
>>> Traceback (most recent call last):
>>> File "/usr/lib64/python2.7/runpy.py", line 162, in _run_module_as_main
>>>   "__main__", fname, loader, pkg_name)
>>> File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
>>>   exec code in run_globals
>>> File "/usr/lib64/python2.7/threading.py", line 985, in <module>
>>>   _test()
>>> File "/usr/lib64/python2.7/threading.py", line 969, in _test
>>>   Q = BoundedQueue(QL)
>>> File "/usr/lib64/python2.7/threading.py", line 912, in __init__
>>>   self.queue = deque()
>>> NameError: global name 'deque' is not defined
>>> 255
>>> 
>>> python3-3.3.0-0.3.b2.fc18.x86_64
>>> python-2.7.3-13.fc18.x86_64
>>> 
>>> GDB moveover currently is not compatible with python3.x at all.
>>> 
>>> I do not know much Python but I would guess thread-less Pythons are rare,
>>> aren't they?  There would be also a possibility to assume Python is threaded,
>>> if one already links with Python.
>> 
>> That's strange, it works for me under Ubuntu 11.04 with Python 2.7.1 and
>> RHEL5 with Python 2.6.6. I suppose it would be easier to just assume Python
>> is threaded, this new patch does that.
> 
> On RHEL-5.8 I have python-2.4.3-46.el5 (not Python 2.6.6) although I can
> confirm it works there:
> 
> $ python -m threading; echo $?
> Producer-1.1
> [...]
> Producer-3.5
> 0

I see what the problem is now: there is bug in Python 2.7.3 in the threading._test function, which is what gets run under "python -m threading". The bug has been fixed, see http://bugs.python.org/issue15567.

Yit
August 30, 2012

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 243 bytes --]

Hi,

Here's another update to my patch: it checks whether Python has threads using the same technique as in gdb/Python/python-internal.h, that is by testing if WITH_THREAD is defined in Python.h. Is this better?

Yit
August 30, 2012



[-- Attachment #2: mcheck-python.txt --]
[-- Type: text/plain, Size: 3513 bytes --]

# HG changeset patch
# Parent 70f7f4713a0f0828f9b8c3a7163ee881f01de122
Do not enable -lmcheck by default when Python is enabled with threading support.

gdb/ChangeLog:

2012-08-30  Khoo Yit Phang  <khooyp@cs.umd.edu>

	Do not enable -lmcheck by default when Python is enabled with
	threading support.
	* configure.ac: (python_has_threads) New variable, by testing
	if WITH_THREAD is defined in Python.h.
	Move --enable-lmcheck after --with-python.
	Do not enable -lmcheck by default if python_has_threads=yes.
	Warn if --enable-lmcheck=yes and python_has_threads=yes.
	* configure: Regenerate.

diff --git a/gdb/configure.ac b/gdb/configure.ac
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -645,28 +645,6 @@
 AC_SUBST(READLINE_CFLAGS)
 AC_SUBST(READLINE_TEXI_INCFLAG)
 
-# Provide a --enable-libmcheck/--disable-libmcheck set of options
-# allowing a user to enable this option even when building releases,
-# or to disable it when building a snapshot.
-AC_ARG_ENABLE(libmcheck,
-  AS_HELP_STRING([--enable-libmcheck],
-                 [Try building GDB with -lmcheck if available]),
-  [case "${enableval}" in
-     yes | y) ENABLE_LIBMCHECK="yes" ;;
-     no | n)  ENABLE_LIBMCHECK="no" ;;
-     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
-   esac])
-
-# Enable -lmcheck by default (it provides cheap-enough memory mangling),
-# but turn it off for releases.
-if test -z "${ENABLE_LIBMCHECK}" && $development; then
-    ENABLE_LIBMCHECK=yes
-fi
-
-if test "$ENABLE_LIBMCHECK" = "yes" ; then
-  AC_CHECK_LIB(mcheck, main)
-fi
-
 # Generate jit-reader.h
 
 # This is typedeffed to GDB_CORE_ADDR in jit-reader.h
@@ -1017,6 +995,18 @@
            fi
            ;;
   esac
+
+  AC_MSG_CHECKING(whether python supports threads)
+  saved_CPPFLAGS="${CPPFLAGS}"
+  CPPFLAGS="${PYTHON_CPPFLAGS}"
+  AC_PREPROC_IFELSE(AC_LANG_SOURCE([[
+#include <Python.h>
+#ifndef WITH_THREAD
+#error
+#endif
+  ]]), [python_has_threads=yes], [python_has_threads=no])
+  AC_MSG_RESULT(${python_has_threads})
+  CPPFLAGS="${saved_CPPFLAGS}"
 else
   # Even if Python support is not compiled in, we need to have these files
   # included.
@@ -1028,6 +1018,34 @@
 AC_SUBST(PYTHON_CPPFLAGS)
 AC_SUBST(PYTHON_LIBS)
 
+# Provide a --enable-libmcheck/--disable-libmcheck set of options
+# allowing a user to enable this option even when building releases,
+# or to disable it when building a snapshot.
+AC_ARG_ENABLE(libmcheck,
+  AS_HELP_STRING([--enable-libmcheck],
+                 [Try building GDB with -lmcheck if available]),
+  [case "${enableval}" in
+     yes | y) ENABLE_LIBMCHECK="yes" ;;
+     no | n)  ENABLE_LIBMCHECK="no" ;;
+     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
+   esac])
+
+# Enable -lmcheck by default (it provides cheap-enough memory mangling),
+# but turn it off if Python is enabled with threads, and for releases.
+if test -z "${ENABLE_LIBMCHECK}" \
+        -a \( "${have_libpython}" = "no" \
+	     -o "${python_has_threads}" = "no" \) \
+    && $development; then
+  ENABLE_LIBMCHECK=yes
+fi
+
+if test "$ENABLE_LIBMCHECK" = "yes" ; then
+  if test "${have_libpython}" != "no" -a "${python_has_threads}" = "yes" ; then
+    AC_MSG_WARN(--enable-libmcheck may lead to spurious crashes if threads are used in python)
+  fi
+  AC_CHECK_LIB(mcheck, main)
+fi
+
 # ------------------------- #
 # Checks for header files.  #
 # ------------------------- #

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Jan Kratochvil]

On Fri, 31 Aug 2012 03:32:06 +0200, Khoo Yit Phang wrote:
> 	Do not enable -lmcheck by default when Python is enabled with
> 	threading support.
> 	* configure.ac: (python_has_threads) New variable, by testing
> 	if WITH_THREAD is defined in Python.h.
> 	Move --enable-lmcheck after --with-python.
> 	Do not enable -lmcheck by default if python_has_threads=yes.
> 	Warn if --enable-lmcheck=yes and python_has_threads=yes.
> 	* configure: Regenerate.

OK with the changes below.

Be sure to use autoconf-2.64 so that configure gets regenerated without
unrelated version changes.


> diff --git a/gdb/configure.ac b/gdb/configure.ac
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -645,28 +645,6 @@
>  AC_SUBST(READLINE_CFLAGS)
>  AC_SUBST(READLINE_TEXI_INCFLAG)
>  
> -# Provide a --enable-libmcheck/--disable-libmcheck set of options
> -# allowing a user to enable this option even when building releases,
> -# or to disable it when building a snapshot.
> -AC_ARG_ENABLE(libmcheck,
> -  AS_HELP_STRING([--enable-libmcheck],
> -                 [Try building GDB with -lmcheck if available]),
> -  [case "${enableval}" in
> -     yes | y) ENABLE_LIBMCHECK="yes" ;;
> -     no | n)  ENABLE_LIBMCHECK="no" ;;
> -     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
> -   esac])
> -
> -# Enable -lmcheck by default (it provides cheap-enough memory mangling),
> -# but turn it off for releases.
> -if test -z "${ENABLE_LIBMCHECK}" && $development; then
> -    ENABLE_LIBMCHECK=yes
> -fi
> -
> -if test "$ENABLE_LIBMCHECK" = "yes" ; then
> -  AC_CHECK_LIB(mcheck, main)
> -fi
> -
>  # Generate jit-reader.h
>  
>  # This is typedeffed to GDB_CORE_ADDR in jit-reader.h
> @@ -1017,6 +995,18 @@
>             fi
>             ;;
>    esac
> +

Put here a comment with reference to the Python issue why '-m threading' is
not used.


> +  AC_MSG_CHECKING(whether python supports threads)
> +  saved_CPPFLAGS="${CPPFLAGS}"
> +  CPPFLAGS="${PYTHON_CPPFLAGS}"
> +  AC_PREPROC_IFELSE(AC_LANG_SOURCE([[
> +#include <Python.h>
> +#ifndef WITH_THREAD
> +#error
> +#endif

A nitpick but the indentation should be:
#ifndef WITH_THREAD
# error
#endif


> +  ]]), [python_has_threads=yes], [python_has_threads=no])

Revert the condition.  If for any reason the compilation fails then it is
safer to default to 'python_has_threads=yes' than to 'no'.


> +  AC_MSG_RESULT(${python_has_threads})
> +  CPPFLAGS="${saved_CPPFLAGS}"
>  else
>    # Even if Python support is not compiled in, we need to have these files
>    # included.
> @@ -1028,6 +1018,34 @@
>  AC_SUBST(PYTHON_CPPFLAGS)
>  AC_SUBST(PYTHON_LIBS)
>  
> +# Provide a --enable-libmcheck/--disable-libmcheck set of options
> +# allowing a user to enable this option even when building releases,
> +# or to disable it when building a snapshot.
> +AC_ARG_ENABLE(libmcheck,
> +  AS_HELP_STRING([--enable-libmcheck],
> +                 [Try building GDB with -lmcheck if available]),
> +  [case "${enableval}" in
> +     yes | y) ENABLE_LIBMCHECK="yes" ;;
> +     no | n)  ENABLE_LIBMCHECK="no" ;;
> +     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
> +   esac])
> +
> +# Enable -lmcheck by default (it provides cheap-enough memory mangling),
> +# but turn it off if Python is enabled with threads, and for releases.

Here should be also comment with the PR libc/9939 to explain why to "turn it
off if Python is enabled with threads".


> +if test -z "${ENABLE_LIBMCHECK}" \
> +        -a \( "${have_libpython}" = "no" \
> +	     -o "${python_has_threads}" = "no" \) \
> +    && $development; then
> +  ENABLE_LIBMCHECK=yes
> +fi
> +
> +if test "$ENABLE_LIBMCHECK" = "yes" ; then
> +  if test "${have_libpython}" != "no" -a "${python_has_threads}" = "yes" ; then
> +    AC_MSG_WARN(--enable-libmcheck may lead to spurious crashes if threads are used in python)
> +  fi
> +  AC_CHECK_LIB(mcheck, main)
> +fi
> +
>  # ------------------------- #
>  # Checks for header files.  #
>  # ------------------------- #


Thanks,
Jan

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 142 bytes --]

Hi,

Thanks for the review! Here's the final patch, which I'll check in a little later today barring any issues.

Yit
August 31, 2012


[-- Attachment #2: mcheck-python.txt --]
[-- Type: text/plain, Size: 3866 bytes --]

# HG changeset patch
# Parent fcbd8836311e169b4142f484ee50e352363cdccd
Do not enable -lmcheck by default when Python is enabled with threading support.

gdb/ChangeLog:

2012-08-31  Khoo Yit Phang  <khooyp@cs.umd.edu>

	Do not enable -lmcheck by default when Python is enabled with
	threading support.
	* configure.ac: (python_has_threads) New variable, by testing
	if WITH_THREAD is defined in Python.h.
	Move --enable-lmcheck after --with-python.
	Do not enable -lmcheck by default if python_has_threads=yes.
	Warn if --enable-lmcheck and python_has_threads=yes.
	* configure: Regenerate.

diff --git a/gdb/configure.ac b/gdb/configure.ac
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -645,28 +645,6 @@
 AC_SUBST(READLINE_CFLAGS)
 AC_SUBST(READLINE_TEXI_INCFLAG)
 
-# Provide a --enable-libmcheck/--disable-libmcheck set of options
-# allowing a user to enable this option even when building releases,
-# or to disable it when building a snapshot.
-AC_ARG_ENABLE(libmcheck,
-  AS_HELP_STRING([--enable-libmcheck],
-                 [Try building GDB with -lmcheck if available]),
-  [case "${enableval}" in
-     yes | y) ENABLE_LIBMCHECK="yes" ;;
-     no | n)  ENABLE_LIBMCHECK="no" ;;
-     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
-   esac])
-
-# Enable -lmcheck by default (it provides cheap-enough memory mangling),
-# but turn it off for releases.
-if test -z "${ENABLE_LIBMCHECK}" && $development; then
-    ENABLE_LIBMCHECK=yes
-fi
-
-if test "$ENABLE_LIBMCHECK" = "yes" ; then
-  AC_CHECK_LIB(mcheck, main)
-fi
-
 # Generate jit-reader.h
 
 # This is typedeffed to GDB_CORE_ADDR in jit-reader.h
@@ -1017,6 +995,23 @@
            fi
            ;;
   esac
+
+  # Note that "python -m threading" cannot be used to check for
+  # threading support due to a bug in Python 2.7.3
+  # (http://bugs.python.org/issue15567).
+  AC_MSG_CHECKING(whether python supports threads)
+  saved_CPPFLAGS="${CPPFLAGS}"
+  CPPFLAGS="${PYTHON_CPPFLAGS}"
+  # Note that the test is reversed so that python_has_threads=yes on
+  # unexpected failures.
+  AC_PREPROC_IFELSE(AC_LANG_SOURCE([[
+#include <Python.h>
+#ifdef WITH_THREAD
+# error
+#endif
+  ]]), [python_has_threads=no], [python_has_threads=yes])
+  AC_MSG_RESULT(${python_has_threads})
+  CPPFLAGS="${saved_CPPFLAGS}"
 else
   # Even if Python support is not compiled in, we need to have these files
   # included.
@@ -1028,6 +1023,36 @@
 AC_SUBST(PYTHON_CPPFLAGS)
 AC_SUBST(PYTHON_LIBS)
 
+# Provide a --enable-libmcheck/--disable-libmcheck set of options
+# allowing a user to enable this option even when building releases,
+# or to disable it when building a snapshot.
+AC_ARG_ENABLE(libmcheck,
+  AS_HELP_STRING([--enable-libmcheck],
+                 [Try building GDB with -lmcheck if available]),
+  [case "${enableval}" in
+     yes | y) ENABLE_LIBMCHECK="yes" ;;
+     no | n)  ENABLE_LIBMCHECK="no" ;;
+     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
+   esac])
+
+# Enable -lmcheck by default (it provides cheap-enough memory mangling),
+# but turn it off if Python is enabled with threads, since -lmcheck is
+# not thread safe (http://sourceware.org/bugzilla/show_bug.cgi?id=9939),
+# and for releases.
+if test -z "${ENABLE_LIBMCHECK}" \
+        -a \( "${have_libpython}" = "no" \
+	     -o "${python_has_threads}" = "no" \) \
+    && $development; then
+  ENABLE_LIBMCHECK=yes
+fi
+
+if test "$ENABLE_LIBMCHECK" = "yes" ; then
+  if test "${have_libpython}" != "no" -a "${python_has_threads}" = "yes" ; then
+    AC_MSG_WARN(--enable-libmcheck may lead to spurious crashes if threads are used in python)
+  fi
+  AC_CHECK_LIB(mcheck, main)
+fi
+
 # ------------------------- #
 # Checks for header files.  #
 # ------------------------- #

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Andreas Schwab]

Khoo Yit Phang <khooyp@cs.umd.edu> writes:

> +if test -z "${ENABLE_LIBMCHECK}" \
> +        -a \( "${have_libpython}" = "no" \
> +	     -o "${python_has_threads}" = "no" \) \

Such complicated test expressions are not portable, you should use
separate test invocations chained with || and &&.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Khoo Yit Phang]

Hi,

On Aug 31, 2012, at 11:40 AM, Andreas Schwab wrote:

> Khoo Yit Phang <khooyp@cs.umd.edu> writes:
> 
>> +if test -z "${ENABLE_LIBMCHECK}" \
>> +        -a \( "${have_libpython}" = "no" \
>> +	     -o "${python_has_threads}" = "no" \) \
> 
> Such complicated test expressions are not portable, you should use
> separate test invocations chained with || and &&.

Can you clarify the rule-of-thumb for this? Both -a and -o are used in tests elsewhere in configure.ac. No more than one -a or -o?

Thanks,

Yit
August 31, 2012

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Andreas Schwab]

Khoo Yit Phang <khooyp@cs.umd.edu> writes:

> Can you clarify the rule-of-thumb for this?

*Note (autoconf) Limitations of Builtins::

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Khoo Yit Phang]

[-- Attachment #1: Type: text/plain, Size: 287 bytes --]

Hi,

Here's an update patch that does not use -a/-o, as per autoconf recommendations (http://www.gnu.org/software/autoconf/manual/autoconf-2.64/html_node/Limitations-of-Builtins.html#index-g_t_0040command_007btest_007d-1583). Does it look good?

Thanks,

Yit
August 31, 2012



[-- Attachment #2: mcheck-python.txt --]
[-- Type: text/plain, Size: 3881 bytes --]

# HG changeset patch
# Parent 8db089d9c7da1d50e6cb2732cef1f7ed226a1889
Do not enable -lmcheck by default when Python is enabled with threading support.

gdb/ChangeLog:

2012-08-31  Khoo Yit Phang  <khooyp@cs.umd.edu>

	Do not enable -lmcheck by default when Python is enabled with
	threading support.
	* configure.ac: (python_has_threads) New variable, by testing
	if WITH_THREAD is defined in Python.h.
	Move --enable-lmcheck after --with-python.
	Do not enable -lmcheck by default if python_has_threads=yes.
	Warn if --enable-lmcheck and python_has_threads=yes.
	* configure: Regenerate.

diff --git a/gdb/configure.ac b/gdb/configure.ac
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -645,28 +645,6 @@
 AC_SUBST(READLINE_CFLAGS)
 AC_SUBST(READLINE_TEXI_INCFLAG)
 
-# Provide a --enable-libmcheck/--disable-libmcheck set of options
-# allowing a user to enable this option even when building releases,
-# or to disable it when building a snapshot.
-AC_ARG_ENABLE(libmcheck,
-  AS_HELP_STRING([--enable-libmcheck],
-                 [Try building GDB with -lmcheck if available]),
-  [case "${enableval}" in
-     yes | y) ENABLE_LIBMCHECK="yes" ;;
-     no | n)  ENABLE_LIBMCHECK="no" ;;
-     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
-   esac])
-
-# Enable -lmcheck by default (it provides cheap-enough memory mangling),
-# but turn it off for releases.
-if test -z "${ENABLE_LIBMCHECK}" && $development; then
-    ENABLE_LIBMCHECK=yes
-fi
-
-if test "$ENABLE_LIBMCHECK" = "yes" ; then
-  AC_CHECK_LIB(mcheck, main)
-fi
-
 # Generate jit-reader.h
 
 # This is typedeffed to GDB_CORE_ADDR in jit-reader.h
@@ -1017,6 +995,23 @@
            fi
            ;;
   esac
+
+  # Note that "python -m threading" cannot be used to check for
+  # threading support due to a bug in Python 2.7.3
+  # (http://bugs.python.org/issue15567).
+  AC_MSG_CHECKING(whether python supports threads)
+  saved_CPPFLAGS="${CPPFLAGS}"
+  CPPFLAGS="${PYTHON_CPPFLAGS}"
+  # Note that the test is reversed so that python_has_threads=yes on
+  # unexpected failures.
+  AC_PREPROC_IFELSE(AC_LANG_SOURCE([[
+#include <Python.h>
+#ifdef WITH_THREAD
+# error
+#endif
+  ]]), [python_has_threads=no], [python_has_threads=yes])
+  AC_MSG_RESULT(${python_has_threads})
+  CPPFLAGS="${saved_CPPFLAGS}"
 else
   # Even if Python support is not compiled in, we need to have these files
   # included.
@@ -1028,6 +1023,37 @@
 AC_SUBST(PYTHON_CPPFLAGS)
 AC_SUBST(PYTHON_LIBS)
 
+# Provide a --enable-libmcheck/--disable-libmcheck set of options
+# allowing a user to enable this option even when building releases,
+# or to disable it when building a snapshot.
+AC_ARG_ENABLE(libmcheck,
+  AS_HELP_STRING([--enable-libmcheck],
+                 [Try building GDB with -lmcheck if available]),
+  [case "${enableval}" in
+     yes | y) ENABLE_LIBMCHECK="yes" ;;
+     no | n)  ENABLE_LIBMCHECK="no" ;;
+     *) AC_MSG_ERROR(bad value ${enableval} for --enable-libmcheck) ;;
+   esac])
+
+# Enable -lmcheck by default (it provides cheap-enough memory mangling),
+# but turn it off if Python is enabled with threads, since -lmcheck is
+# not thread safe (http://sourceware.org/bugzilla/show_bug.cgi?id=9939),
+# and for releases.
+if test -z "${ENABLE_LIBMCHECK}" \
+    && { test "${have_libpython}" = "no" \
+	|| test "${python_has_threads}" = "no"; } \
+    && $development; then
+  ENABLE_LIBMCHECK=yes
+fi
+
+if test "$ENABLE_LIBMCHECK" = "yes" ; then
+  if test "${have_libpython}" != "no" \
+      && test "${python_has_threads}" = "yes" ; then
+    AC_MSG_WARN(--enable-libmcheck may lead to spurious crashes if threads are used in python)
+  fi
+  AC_CHECK_LIB(mcheck, main)
+fi
+
 # ------------------------- #
 # Checks for header files.  #
 # ------------------------- #

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Jan Kratochvil]

On Fri, 31 Aug 2012 18:07:43 +0200, Khoo Yit Phang wrote:
> On Aug 31, 2012, at 11:40 AM, Andreas Schwab wrote:
> > Such complicated test expressions are not portable, you should use
> > separate test invocations chained with || and &&.
> 
> Can you clarify the rule-of-thumb for this? Both -a and -o are used in tests
> elsewhere in configure.ac. No more than one -a or -o?

Autoconf manual really says:
	The `-a', `-o', `(', and `)' operands are not portable and should be
	avoided.

But '-a' and '-o' are already used in configure.ac.  '(' and ')' are not but
that should be the same category, also POSIX documents '(' and ')'.


Jan

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Khoo Yit Phang]

Hi,

On Aug 31, 2012, at 1:56 PM, Jan Kratochvil wrote:

> On Fri, 31 Aug 2012 18:07:43 +0200, Khoo Yit Phang wrote:
>> On Aug 31, 2012, at 11:40 AM, Andreas Schwab wrote:
>>> Such complicated test expressions are not portable, you should use
>>> separate test invocations chained with || and &&.
>> 
>> Can you clarify the rule-of-thumb for this? Both -a and -o are used in tests
>> elsewhere in configure.ac. No more than one -a or -o?
> 
> Autoconf manual really says:
> 	The `-a', `-o', `(', and `)' operands are not portable and should be
> 	avoided.
> 
> But '-a' and '-o' are already used in configure.ac.  '(' and ')' are not but
> that should be the same category, also POSIX documents '(' and ')'.

I've posted patches for both styles, I'll check in whichever is preferable.

Yit
August 31, 2012

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Jan Kratochvil]

On Fri, 31 Aug 2012 20:40:49 +0200, Khoo Yit Phang wrote:
> On Aug 31, 2012, at 1:56 PM, Jan Kratochvil wrote:
> > But '-a' and '-o' are already used in configure.ac.  '(' and ')' are not but
> > that should be the same category, also POSIX documents '(' and ')'.
> 
> I've posted patches for both styles, I'll check in whichever is preferable.

While Andreas is formally right I do not think it matters with current
gdb/configure.ac, please check in the more readable -a/-o/(/) variant.

If one finds an incompatible build platform one needs to fix all the other
cases already anyway.


Thanks,
Jan

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Khoo Yit Phang]

Hi,

On Sep 2, 2012, at 11:36 AM, Jan Kratochvil wrote:

> On Fri, 31 Aug 2012 20:40:49 +0200, Khoo Yit Phang wrote:
>> On Aug 31, 2012, at 1:56 PM, Jan Kratochvil wrote:
>>> But '-a' and '-o' are already used in configure.ac.  '(' and ')' are not but
>>> that should be the same category, also POSIX documents '(' and ')'.
>> 
>> I've posted patches for both styles, I'll check in whichever is preferable.
> 
> While Andreas is formally right I do not think it matters with current
> gdb/configure.ac, please check in the more readable -a/-o/(/) variant.
> 
> If one finds an incompatible build platform one needs to fix all the other
> cases already anyway.

I've checked in the -a/-o variant of this patch.

Thanks!

Yit
September 6, 2012

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Sergio Durigan Junior]

On Thursday, August 30 2012, Khoo Yit Phang wrote:

> On Aug 30, 2012, at 12:43 PM, Jan Kratochvil wrote:
>
>> On Thu, 30 Aug 2012 18:00:46 +0200, Khoo Yit Phang wrote:
>>> 2012-08-30  Khoo Yit Phang  <khooyp@cs.umd.edu>
>> 
>> This patch does not seem to require copyright assignment but it would be safer
>> to complete one for example from Sergio Durigan Junior <sergiodj@redhat.com>.
>> Also for any possible future contributions.
>
> I have copyright assignment set up, and commit access.

I can't find your name on gdb/MAINTAINERS.  You should put it there.

-- 
Sergio

Re: [patch] Re: Disable -lmcheck when Python has threads (Re: [BUG] gdb crash when "python import gtk")

[Khoo Yit Phang]

Hi,

On Aug 30, 2012, at 2:22 PM, Sergio Durigan Junior wrote:

> On Thursday, August 30 2012, Khoo Yit Phang wrote:
> 
>> On Aug 30, 2012, at 12:43 PM, Jan Kratochvil wrote:
>> 
>>> On Thu, 30 Aug 2012 18:00:46 +0200, Khoo Yit Phang wrote:
>>>> 2012-08-30  Khoo Yit Phang  <khooyp@cs.umd.edu>
>>> 
>>> This patch does not seem to require copyright assignment but it would be safer
>>> to complete one for example from Sergio Durigan Junior <sergiodj@redhat.com>.
>>> Also for any possible future contributions.
>> 
>> I have copyright assignment set up, and commit access.
> 
> I can't find your name on gdb/MAINTAINERS.  You should put it there.

Thanks, I'll do just that!

Yit
August 30, 2012