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

[Simon Marchi <simark@simark.ca>]

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