[1/4] gdb/selftest.m4: ensure $development is set

Message ID 20200305193011.25939-1-simon.marchi@efficios.com
State New
Headers show
Series
  • [1/4] gdb/selftest.m4: ensure $development is set
Related show

Commit Message

Simon Marchi March 5, 2020, 7:30 p.m.
The GDB build in non-development mode (turn development to false in
bfd/development.sh if you want to try) is currently broken:

      CXXLD  gdb
    /home/smarchi/src/binutils-gdb/gdb/disasm-selftests.c:218: error: undefined reference to 'selftests::register_test_foreach_arch(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void (*)(gdbarch*))'
    /home/smarchi/src/binutils-gdb/gdb/disasm-selftests.c:220: error: undefined reference to 'selftests::register_test_foreach_arch(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void (*)(gdbarch*))'
    /home/smarchi/src/binutils-gdb/gdb/dwarf2/frame.c:2310: error: undefined reference to 'selftests::register_test_foreach_arch(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void (*)(gdbarch*))'
    /home/smarchi/src/binutils-gdb/gdb/gdbarch-selftests.c:168: error: undefined reference to 'selftests::register_test_foreach_arch(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void (*)(gdbarch*))'
    /home/smarchi/src/binutils-gdb/gdbsupport/selftest.cc:96: error: undefined reference to 'selftests::reset()'

This is because the gdbsupport configure script doesn't source
bfd/development.sh to set the development variable.  When $development
is unset, GDB_AC_SELFTEST defaults to enabling selftests.  I don't think
the macro was written with this intention in mind, it just happens to be
that way.

So gdbsupport thinks selftests are enabled, while gdb thinks they are
disabled.  gdbsupport compiles in code that calls selftests:: functions,
which are normally provided by gdb, but gdb doesn't provide them, hence
the undefined references.

Since the macro relies on the `development` variable, I propose to
modify it such that it errors out if $development does not have an
expected value of "true" or "false".  This could prevent a future
similar problem from happening while refactoring the configure scripts.
This catches the current problem in the gdbsupport configure script,
which is fixed by sourcing development.sh, as it's done in
gdb/configure.ac and gdbserver/configure.ac.

gdb/ChangeLog:

	* selftest.m4 (GDB_AC_SELFTEST): Error out if $development is
	not "true" or "false".
	* configure: Re-generate.

gdbserver/ChangeLog:

	* configure: Re-generate.

gdbsupport/ChangeLog:

	* configure.ac: Source bfd/development.sh.
	* configure: Re-generate.
---
 gdb/configure           | 5 +++++
 gdb/selftest.m4         | 4 ++++
 gdbserver/configure     | 5 +++++
 gdbsupport/configure    | 8 ++++++++
 gdbsupport/configure.ac | 3 +++
 5 files changed, 25 insertions(+)

-- 
2.25.1

Comments

Simon Marchi March 5, 2020, 7:42 p.m. | #1
On 2020-03-05 2:30 p.m., Simon Marchi wrote:
> The GDB build in non-development mode (turn development to false in

> bfd/development.sh if you want to try) is currently broken:

> 

>       CXXLD  gdb

>     /home/smarchi/src/binutils-gdb/gdb/disasm-selftests.c:218: error: undefined reference to 'selftests::register_test_foreach_arch(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void (*)(gdbarch*))'

>     /home/smarchi/src/binutils-gdb/gdb/disasm-selftests.c:220: error: undefined reference to 'selftests::register_test_foreach_arch(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void (*)(gdbarch*))'

>     /home/smarchi/src/binutils-gdb/gdb/dwarf2/frame.c:2310: error: undefined reference to 'selftests::register_test_foreach_arch(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void (*)(gdbarch*))'

>     /home/smarchi/src/binutils-gdb/gdb/gdbarch-selftests.c:168: error: undefined reference to 'selftests::register_test_foreach_arch(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void (*)(gdbarch*))'

>     /home/smarchi/src/binutils-gdb/gdbsupport/selftest.cc:96: error: undefined reference to 'selftests::reset()'

> 

> This is because the gdbsupport configure script doesn't source

> bfd/development.sh to set the development variable.  When $development

> is unset, GDB_AC_SELFTEST defaults to enabling selftests.  I don't think

> the macro was written with this intention in mind, it just happens to be

> that way.

> 

> So gdbsupport thinks selftests are enabled, while gdb thinks they are

> disabled.  gdbsupport compiles in code that calls selftests:: functions,

> which are normally provided by gdb, but gdb doesn't provide them, hence

> the undefined references.

> 

> Since the macro relies on the `development` variable, I propose to

> modify it such that it errors out if $development does not have an

> expected value of "true" or "false".  This could prevent a future

> similar problem from happening while refactoring the configure scripts.

> This catches the current problem in the gdbsupport configure script,

> which is fixed by sourcing development.sh, as it's done in

> gdb/configure.ac and gdbserver/configure.ac.

> 

> gdb/ChangeLog:

> 

> 	* selftest.m4 (GDB_AC_SELFTEST): Error out if $development is

> 	not "true" or "false".

> 	* configure: Re-generate.

> 

> gdbserver/ChangeLog:

> 

> 	* configure: Re-generate.

> 

> gdbsupport/ChangeLog:

> 

> 	* configure.ac: Source bfd/development.sh.

> 	* configure: Re-generate.


Since I've pushed this other patch:

  https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=3d1e5a43cbe1780ea66df0fe091998ee61177899

I've rebased this series and modified the commit message of patch 1/4, this is
the updated version.


From e9d5bf80c7d9fd27ea38ac82067e286d4b313184 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>

Date: Thu, 5 Mar 2020 11:23:52 -0500
Subject: [PATCH] gdb/selftest.m4: ensure $development is set

Before commit 3d1e5a43cbe ("gdbsupport/configure.ac: source
development.sh"), the GDB build in non-development mode (turn
development to false in bfd/development.sh if you want to try) was
broken because the gdbsupport configure script didn't source
bfd/development.sh to set the development variable.

Since the GDB_AC_SELFTEST macro relies on the `development` variable, I
propose to modify it such that it errors out if $development does not
have an expected value of "true" or "false".  This could prevent a
future similar problem from happening while refactoring the configure
scripts.  It would have caught the problem fixed by the patch mentioned
earlier.

gdb/ChangeLog:

	* selftest.m4 (GDB_AC_SELFTEST): Error out if $development is
	not "true" or "false".
	* configure: Re-generate.

gdbserver/ChangeLog:

	* configure: Re-generate.

gdbsupport/ChangeLog:

	* configure: Re-generate.
---
 gdb/configure        | 5 +++++
 gdb/selftest.m4      | 4 ++++
 gdbserver/configure  | 5 +++++
 gdbsupport/configure | 5 +++++
 4 files changed, 19 insertions(+)

diff --git a/gdb/configure b/gdb/configure
index f99cbe40f11f..d885b94b8b52 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -19186,6 +19186,11 @@ $as_echo "#define GDB_DEFAULT_HOST_CHARSET \"UTF-8\"" >>confdefs.h
 # The default value of this option changes depending whether we're on
 # development mode (in which case it's "true") or not (in which case
 # it's "false").
+
+if test "x$development" != xtrue && test "x$development" != xfalse; then :
+  as_fn_error $? "Invalid value for \$development, got \"$development\", expecting \"true\" or \"false\"." "$LINENO" 5
+fi
+
 # Check whether --enable-unit-tests was given.
 if test "${enable_unit_tests+set}" = set; then :
   enableval=$enable_unit_tests; case "${enableval}" in
diff --git a/gdb/selftest.m4 b/gdb/selftest.m4
index 4969de1cada9..a88aa96171cb 100644
--- a/gdb/selftest.m4
+++ b/gdb/selftest.m4
@@ -27,6 +27,10 @@ AC_DEFUN([GDB_AC_SELFTEST],[
 # The default value of this option changes depending whether we're on
 # development mode (in which case it's "true") or not (in which case
 # it's "false").
+
+AS_IF([test "x$development" != xtrue && test "x$development" != xfalse],
+  [AC_MSG_ERROR([Invalid value for \$development, got "$development", expecting "true" or "false".])])
+
 AC_ARG_ENABLE(unit-tests,
 AS_HELP_STRING([--enable-unit-tests],
 [Enable the inclusion of unit tests when compiling GDB]),
diff --git a/gdbserver/configure b/gdbserver/configure
index be5719eb77aa..06b25ba2b6e0 100755
--- a/gdbserver/configure
+++ b/gdbserver/configure
@@ -6083,6 +6083,11 @@ fi
 # The default value of this option changes depending whether we're on
 # development mode (in which case it's "true") or not (in which case
 # it's "false").
+
+if test "x$development" != xtrue && test "x$development" != xfalse; then :
+  as_fn_error $? "Invalid value for \$development, got \"$development\", expecting \"true\" or \"false\"." "$LINENO" 5
+fi
+
 # Check whether --enable-unit-tests was given.
 if test "${enable_unit_tests+set}" = set; then :
   enableval=$enable_unit_tests; case "${enableval}" in
diff --git a/gdbsupport/configure b/gdbsupport/configure
index e7a99e3ddfba..10dbd7e6ee76 100755
--- a/gdbsupport/configure
+++ b/gdbsupport/configure
@@ -10606,6 +10606,11 @@ $as_echo "$bfd_cv_have_sys_procfs_type_elf_fpregset_t" >&6; }
 # The default value of this option changes depending whether we're on
 # development mode (in which case it's "true") or not (in which case
 # it's "false").
+
+if test "x$development" != xtrue && test "x$development" != xfalse; then :
+  as_fn_error $? "Invalid value for \$development, got \"$development\", expecting \"true\" or \"false\"." "$LINENO" 5
+fi
+
 # Check whether --enable-unit-tests was given.
 if test "${enable_unit_tests+set}" = set; then :
   enableval=$enable_unit_tests; case "${enableval}" in
-- 
2.25.1
Tom Tromey March 12, 2020, 6:07 p.m. | #2
>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:


Simon> The GDB build in non-development mode (turn development to false in
Simon> bfd/development.sh if you want to try) is currently broken:
[...]

FWIW I read through this series and it all looked ok to me.
Thank you for doing this.

Tom
Simon Marchi March 12, 2020, 6:19 p.m. | #3
On 2020-03-12 2:07 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

> 

> Simon> The GDB build in non-development mode (turn development to false in

> Simon> bfd/development.sh if you want to try) is currently broken:

> [...]

> 

> FWIW I read through this series and it all looked ok to me.

> Thank you for doing this.

> 

> Tom

> 


Thanks for looking, I'll push it in a moment.

Simon

Patch

diff --git a/gdb/configure b/gdb/configure
index f99cbe40f1..d885b94b8b 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -19186,6 +19186,11 @@  $as_echo "#define GDB_DEFAULT_HOST_CHARSET \"UTF-8\"" >>confdefs.h
 # The default value of this option changes depending whether we're on
 # development mode (in which case it's "true") or not (in which case
 # it's "false").
+
+if test "x$development" != xtrue && test "x$development" != xfalse; then :
+  as_fn_error $? "Invalid value for \$development, got \"$development\", expecting \"true\" or \"false\"." "$LINENO" 5
+fi
+
 # Check whether --enable-unit-tests was given.
 if test "${enable_unit_tests+set}" = set; then :
   enableval=$enable_unit_tests; case "${enableval}" in
diff --git a/gdb/selftest.m4 b/gdb/selftest.m4
index 4969de1cad..a88aa96171 100644
--- a/gdb/selftest.m4
+++ b/gdb/selftest.m4
@@ -27,6 +27,10 @@  AC_DEFUN([GDB_AC_SELFTEST],[
 # The default value of this option changes depending whether we're on
 # development mode (in which case it's "true") or not (in which case
 # it's "false").
+
+AS_IF([test "x$development" != xtrue && test "x$development" != xfalse],
+  [AC_MSG_ERROR([Invalid value for \$development, got "$development", expecting "true" or "false".])])
+
 AC_ARG_ENABLE(unit-tests,
 AS_HELP_STRING([--enable-unit-tests],
 [Enable the inclusion of unit tests when compiling GDB]),
diff --git a/gdbserver/configure b/gdbserver/configure
index be5719eb77..06b25ba2b6 100755
--- a/gdbserver/configure
+++ b/gdbserver/configure
@@ -6083,6 +6083,11 @@  fi
 # The default value of this option changes depending whether we're on
 # development mode (in which case it's "true") or not (in which case
 # it's "false").
+
+if test "x$development" != xtrue && test "x$development" != xfalse; then :
+  as_fn_error $? "Invalid value for \$development, got \"$development\", expecting \"true\" or \"false\"." "$LINENO" 5
+fi
+
 # Check whether --enable-unit-tests was given.
 if test "${enable_unit_tests+set}" = set; then :
   enableval=$enable_unit_tests; case "${enableval}" in
diff --git a/gdbsupport/configure b/gdbsupport/configure
index a4871f8d5b..8d25380ec8 100755
--- a/gdbsupport/configure
+++ b/gdbsupport/configure
@@ -3538,6 +3538,9 @@  fi
 AM_BACKSLASH='\'
 
 
+# Set the 'development' global.
+. $srcdir/../bfd/development.sh
+
 ac_ext=c
 ac_cpp='$CPP $CPPFLAGS'
 ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
@@ -10603,6 +10606,11 @@  $as_echo "$bfd_cv_have_sys_procfs_type_elf_fpregset_t" >&6; }
 # The default value of this option changes depending whether we're on
 # development mode (in which case it's "true") or not (in which case
 # it's "false").
+
+if test "x$development" != xtrue && test "x$development" != xfalse; then :
+  as_fn_error $? "Invalid value for \$development, got \"$development\", expecting \"true\" or \"false\"." "$LINENO" 5
+fi
+
 # Check whether --enable-unit-tests was given.
 if test "${enable_unit_tests+set}" = set; then :
   enableval=$enable_unit_tests; case "${enableval}" in
diff --git a/gdbsupport/configure.ac b/gdbsupport/configure.ac
index 401e16f821..0b2f2415d5 100644
--- a/gdbsupport/configure.ac
+++ b/gdbsupport/configure.ac
@@ -25,6 +25,9 @@  AC_CONFIG_AUX_DIR(..)
 AM_INIT_AUTOMAKE
 AM_SILENT_RULES([yes])
 
+# Set the 'development' global.
+. $srcdir/../bfd/development.sh
+
 AC_PROG_CC
 AC_PROG_CXX
 AC_PROG_RANLIB