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

Message ID 20180916184502.19617-3-tom@tromey.com
State New
Headers show
Series
  • two small configure patches
Related show

Commit Message

Tom Tromey Sept. 16, 2018, 6:45 p.m.
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(-)

-- 
2.17.1

Comments

Joel Brobecker Sept. 17, 2018, 6:19 p.m. | #1
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
Tom Tromey Sept. 17, 2018, 6:22 p.m. | #2
>>>>> "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
Tom Tromey Sept. 17, 2018, 7:52 p.m. | #3
>>>>> "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
Joel Brobecker Sept. 17, 2018, 9:21 p.m. | #4
> >>>>> "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
Tom Tromey Sept. 17, 2018, 9:22 p.m. | #5
>>>>> "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

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