[PATCH 0/2] two small configure patches

[PATCH 0/2] two small configure patches

[Tom Tromey]

Here are two small configure patches.
They are each pretty self-explanatory.

Tom

[PATCH 2/2] Do not pass -DNDEBUG to Python compilations in development mode

[Tom Tromey]

The Python CFLAGS include -DNDEBUG.  This was apparently done
intentionally -- setting the flags is done manually because, according
to a comment, python-config passes too many things to the compiler
(which is true).

Per PR python/20445, this patch changes configure so that -DNDEBUG is
only used by release builds.  This probably doesn't have very much
effect in practice, but I did see that some Python headers use assert,
so perhaps it will give some safety.

Tested by rebuilding and re-running gdb.python/*.exp on x86-64 Fedora 28.

ChangeLog
2018-09-16  Tom Tromey  <tom@tromey.com>

	PR python/20445:
	* configure: Rebuild.
	* configure.ac: Conditionally use -DNDEBUG for Python.
---
 gdb/ChangeLog    | 6 ++++++
 gdb/configure    | 6 +++++-
 gdb/configure.ac | 6 +++++-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/gdb/configure b/gdb/configure
index e011b77414b..2fb69a8eda6 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -10762,7 +10762,11 @@ $as_echo "#define HAVE_PYTHON 1" >>confdefs.h
   # would make the python-related objects be compiled differently from the
   # rest of GDB (e.g., -O2 and -fPIC).
   if test "${GCC}" = yes; then
-    tentative_python_cflags="-fno-strict-aliasing -DNDEBUG -fwrapv"
+    tentative_python_cflags="-fno-strict-aliasing -fwrapv"
+    # Python headers recommend -DNDEBUG, but it's unclear if that just
+    # refers to building Python itself.  In release mode, though, it
+    # doesn't hurt for the Python code in gdb to follow.
+    $development || tentative_python_cflags="tentative_python_cflags -DNDEBUG"
   fi
 
   if test "x${tentative_python_cflags}" != x; then
diff --git a/gdb/configure.ac b/gdb/configure.ac
index f658da84e32..0709b1ff720 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -965,7 +965,11 @@ if test "${have_libpython}" != no; then
   # would make the python-related objects be compiled differently from the
   # rest of GDB (e.g., -O2 and -fPIC).
   if test "${GCC}" = yes; then
-    tentative_python_cflags="-fno-strict-aliasing -DNDEBUG -fwrapv"
+    tentative_python_cflags="-fno-strict-aliasing -fwrapv"
+    # Python headers recommend -DNDEBUG, but it's unclear if that just
+    # refers to building Python itself.  In release mode, though, it
+    # doesn't hurt for the Python code in gdb to follow.
+    $development || tentative_python_cflags="tentative_python_cflags -DNDEBUG"
   fi
 
   if test "x${tentative_python_cflags}" != x; then
-- 
2.17.1

Re: [PATCH 2/2] Do not pass -DNDEBUG to Python compilations in development mode

[Joel Brobecker]

Hi Tom,

On Sun, Sep 16, 2018 at 12:45:02PM -0600, Tom Tromey wrote:
> The Python CFLAGS include -DNDEBUG.  This was apparently done
> intentionally -- setting the flags is done manually because, according
> to a comment, python-config passes too many things to the compiler
> (which is true).
> 
> Per PR python/20445, this patch changes configure so that -DNDEBUG is
> only used by release builds.  This probably doesn't have very much
> effect in practice, but I did see that some Python headers use assert,
> so perhaps it will give some safety.
> 
> Tested by rebuilding and re-running gdb.python/*.exp on x86-64 Fedora 28.
> 
> ChangeLog
> 2018-09-16  Tom Tromey  <tom@tromey.com>
> 
> 	PR python/20445:
> 	* configure: Rebuild.
> 	* configure.ac: Conditionally use -DNDEBUG for Python.
> ---
>  gdb/ChangeLog    | 6 ++++++
>  gdb/configure    | 6 +++++-
>  gdb/configure.ac | 6 +++++-
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/configure b/gdb/configure
> index e011b77414b..2fb69a8eda6 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -10762,7 +10762,11 @@ $as_echo "#define HAVE_PYTHON 1" >>confdefs.h
>    # would make the python-related objects be compiled differently from the
>    # rest of GDB (e.g., -O2 and -fPIC).
>    if test "${GCC}" = yes; then
> -    tentative_python_cflags="-fno-strict-aliasing -DNDEBUG -fwrapv"
> +    tentative_python_cflags="-fno-strict-aliasing -fwrapv"
> +    # Python headers recommend -DNDEBUG, but it's unclear if that just
> +    # refers to building Python itself.  In release mode, though, it
> +    # doesn't hurt for the Python code in gdb to follow.
> +    $development || tentative_python_cflags="tentative_python_cflags -DNDEBUG"

Are we missing a '$' before tentative_python_cflags in the string,
by any chance?

>    fi
>  
>    if test "x${tentative_python_cflags}" != x; then
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index f658da84e32..0709b1ff720 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -965,7 +965,11 @@ if test "${have_libpython}" != no; then
>    # would make the python-related objects be compiled differently from the
>    # rest of GDB (e.g., -O2 and -fPIC).
>    if test "${GCC}" = yes; then
> -    tentative_python_cflags="-fno-strict-aliasing -DNDEBUG -fwrapv"
> +    tentative_python_cflags="-fno-strict-aliasing -fwrapv"
> +    # Python headers recommend -DNDEBUG, but it's unclear if that just
> +    # refers to building Python itself.  In release mode, though, it
> +    # doesn't hurt for the Python code in gdb to follow.
> +    $development || tentative_python_cflags="tentative_python_cflags -DNDEBUG"
>    fi
>  
>    if test "x${tentative_python_cflags}" != x; then
> -- 
> 2.17.1

-- 
Joel

Re: [PATCH 2/2] Do not pass -DNDEBUG to Python compilations in development mode

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Are we missing a '$' before tentative_python_cflags in the string,
Joel> by any chance?

Yes, oops.  I wonder how that slipped through.

Tom

Re: [PATCH 2/2] Do not pass -DNDEBUG to Python compilations in development mode

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
Joel> Are we missing a '$' before tentative_python_cflags in the string,
Joel> by any chance?

Tom> Yes, oops.  I wonder how that slipped through.

Obviously I did not test in release mode.
I've fixed the bug and tried it both ways.

Tom

Re: [PATCH 2/2] Do not pass -DNDEBUG to Python compilations in development mode

[Joel Brobecker]

> >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
> Joel> Are we missing a '$' before tentative_python_cflags in the string,
> Joel> by any chance?
> 
> Tom> Yes, oops.  I wonder how that slipped through.
> 
> Obviously I did not test in release mode.
> I've fixed the bug and tried it both ways.

Thanks Tom. Other than this comment, I had no other comment so,
now that it is fixed and tested, I think you can push.

-- 
Joel

Re: [PATCH 2/2] Do not pass -DNDEBUG to Python compilations in development mode

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
Joel> Are we missing a '$' before tentative_python_cflags in the string,
Joel> by any chance?
>> 
Tom> Yes, oops.  I wonder how that slipped through.
>> 
>> Obviously I did not test in release mode.
>> I've fixed the bug and tried it both ways.

Joel> Thanks Tom. Other than this comment, I had no other comment so,
Joel> now that it is fixed and tested, I think you can push.

I'd already done it since it seemed pretty clear.

Tom

[PATCH 1/2] Check for gmp when checking for mpfr

[Tom Tromey]

There was a report on irc that the gdb check for mpfr failed when only
static libraries are available.  The issue is that mpfr depends on
gmp, but this is not handled explicitly by gdb.

Ideally upstream would switch to pkg-config.  Or even more ideally, we
would incorporate pkg-config into the compiler and not mess with any
of this.

Meanwhile, this changes gdb's configure to add gmp to the link line
when checking for mpfr.

ChangeLog
2018-09-16  Tom Tromey  <tom@tromey.com>

	* configure: Rebuild.
	* configure.ac: Use gmp as a library dependency when checking for
	mpfr.
---
 gdb/ChangeLog    | 6 ++++++
 gdb/configure    | 2 +-
 gdb/configure.ac | 2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/configure b/gdb/configure
index d92a256f1ff..e011b77414b 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -9837,7 +9837,7 @@ fi
   rpathdirs=
   ltrpathdirs=
   names_already_handled=
-  names_next_round='mpfr '
+  names_next_round='mpfr gmp'
   while test -n "$names_next_round"; do
     names_this_round="$names_next_round"
     names_next_round=
diff --git a/gdb/configure.ac b/gdb/configure.ac
index e38604cb656..f658da84e32 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -682,7 +682,7 @@ if test "${with_mpfr}" = no; then
   AC_MSG_WARN([MPFR support disabled; some features may be unavailable.])
   HAVE_LIBMPFR=no
 else
-  AC_LIB_HAVE_LINKFLAGS([mpfr], [], [#include <mpfr.h>],
+  AC_LIB_HAVE_LINKFLAGS([mpfr], [gmp], [#include <mpfr.h>],
 			[mpfr_exp_t exp; mpfr_t x;
 			 mpfr_frexp (&exp, x, x, MPFR_RNDN);])
   if test "$HAVE_LIBMPFR" != yes; then
-- 
2.17.1

Re: [PATCH 1/2] Check for gmp when checking for mpfr

[Joel Brobecker]

Hi Tom,

On Sun, Sep 16, 2018 at 12:45:01PM -0600, Tom Tromey wrote:
> There was a report on irc that the gdb check for mpfr failed when only
> static libraries are available.  The issue is that mpfr depends on
> gmp, but this is not handled explicitly by gdb.
> 
> Ideally upstream would switch to pkg-config.  Or even more ideally, we
> would incorporate pkg-config into the compiler and not mess with any
> of this.
> 
> Meanwhile, this changes gdb's configure to add gmp to the link line
> when checking for mpfr.
> 
> ChangeLog
> 2018-09-16  Tom Tromey  <tom@tromey.com>
> 
> 	* configure: Rebuild.
> 	* configure.ac: Use gmp as a library dependency when checking for
> 	mpfr.

Thanks for the patch. This looks good to me.

-- 
Joel