From patchwork Thu Mar 5 19:42:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 38440 Received: (qmail 102146 invoked by alias); 5 Mar 2020 19:42:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 100881 invoked by uid 89); 5 Mar 2020 19:42:40 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy=U*simon.marchi, sk:simon.m, sk:simonm X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 05 Mar 2020 19:42:37 +0000 Received: from [172.16.0.95] (192-222-181-218.qc.cable.ebox.net [192.222.181.218]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id B6E421E5F3; Thu, 5 Mar 2020 14:42:35 -0500 (EST) Subject: Re: [PATCH 1/4] gdb/selftest.m4: ensure $development is set To: Simon Marchi , gdb-patches@sourceware.org References: <20200305193011.25939-1-simon.marchi@efficios.com> From: Simon Marchi Message-ID: Date: Thu, 5 Mar 2020 14:42:35 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20200305193011.25939-1-simon.marchi@efficios.com> 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, std::allocator > 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, std::allocator > 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, std::allocator > 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, std::allocator > 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 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