From patchwork Mon Jan 27 13:41:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 37577 Received: (qmail 35712 invoked by alias); 27 Jan 2020 13:41:28 -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 35624 invoked by uid 89); 27 Jan 2020 13:41:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=2.14.5 X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 27 Jan 2020 13:41:24 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580132482; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1/SkcZfgdX3x9LPuLl9KC1g9dPiOxZ12JRuKX+M28lo=; b=UtYsx7pRBuuvG2sxAy3Yr9VKdcsVGhnt4n1j8kzfgn8e/M52odal63vRgmuxNHq8u+JCIQ qavDSaKz64TLe3QfjvE+xc1nZZrtWG4tPXOiqfDm8M0MRMs97ROVgJHUNH/Ai7l1I2zWHS +x2OL4bLn17MewUBlLenDHPzA1pTf4w= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-419-rj90yBFQNAau6dL6NBnu-w-1; Mon, 27 Jan 2020 08:41:09 -0500 Received: by mail-wr1-f69.google.com with SMTP id b13so6037582wrx.22 for ; Mon, 27 Jan 2020 05:41:09 -0800 (PST) Return-Path: Received: from ?IPv6:2001:8a0:f909:7b00:56ee:75ff:fe8d:232b? ([2001:8a0:f909:7b00:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id d12sm20652927wrp.62.2020.01.27.05.41.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 27 Jan 2020 05:41:07 -0800 (PST) Subject: Re: [PATCH] Move gdbserver to top level To: Tom Tromey References: <87d0bf45up.fsf@tromey.com> <7ceebbb7-b2f7-3d4a-1d8a-f31310badbe8@redhat.com> <874kwk8nz9.fsf@tromey.com> <171a3144-af37-1c29-a2a4-c4cd7eaa14c0@redhat.com> <87r1zm6x8s.fsf@tromey.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <01b4b5ca-a802-54b5-3135-428b7c9faa84@redhat.com> Date: Mon, 27 Jan 2020 13:41:06 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <87r1zm6x8s.fsf@tromey.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com On 1/26/20 3:41 PM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves writes: > > Pedro> I think this should have a NEWS entry, given that it affects > Pedro> people building standalone gdbserver today. Do you have > Pedro> documentation changes queued? > > Nope, I've added those now. > >>> +// Host modules specific to gdbserver. >>> +dependencies = { module=configure-gdbserver; on=all-intl; }; > > Pedro> This surprised me, as I don't think gdbserver actually depends on > Pedro> intl currently? Might still be a good idea to actually enable it, > Pedro> but I was surprised from an "as pure as possible" perspective. > > I don't recall (I wrote this patch originally in May of last year), but > maybe I just copied gdb's dependencies. Anyway, I've dropped them now. Thanks. > > Tom > > commit 4c5c9a7a0b64c98f60c2ab79ad705d14ea568d0f > Author: Tom Tromey > Date: Sun Dec 15 07:37:06 2019 -0700 > > Move gdbserver to top level > > This patch moves gdbserver to the top level. > > This patch is as close to a pure move as possible -- gdbserver still > builds its own variant of gnulib and gdbsupport. Changing this will > be done in a separate patch. > > [v2] Note that, per Simon's review comment, this patch changes the > tree so that gdbserver is not built for or1k or score. This makes > sense, because there is apparently not actually a gdbserver port here. > > [v3] This version of the patch also splits out some configury into a > new file, gdbserver/configure.host, so that the top-level configure > script can simply rely on it in order to decide whether gdbserver > should be built. > > [v4] This version adds documentation and removes some unnecessary > top-level dependencies. Thanks. > +# Only allow gdbserver on some systems. > +if test -d ${srcdir}/gdbserver; then > + if test x$enable_gdbserver = x; then > + AC_MSG_CHECKING([for gdbserver support]) > + . ${srcdir}/gdbserver/configure.host > + if test x$build_gdbserver = xyes; then > + AC_MSG_RESULT([yes]) > + else > + noconfigdirs="$noconfigdirs gdbserver" > + AC_MSG_RESULT([no]) > + fi > + fi > +fi Some tabs vs spaces mixup above. (But see below.) > --- a/gdb/gdbserver/README > +++ b/gdbserver/README > @@ -100,27 +100,24 @@ The supported targets as of November 2006 are: > spu*-*-* > x86_64-*-linux* > > -Configuring GDBserver you should specify the same machine for host and > -target (which are the machine that GDBserver is going to run on. This > -is not the same as the machine that GDB is going to run on; building > -GDBserver automatically as part of building a whole tree of tools does > -not currently work if cross-compilation is involved (we don't get the > -right CC in the Makefile, to start with)). > - > -Building GDBserver for your target is very straightforward. If you build > -GDB natively on a target which GDBserver supports, it will be built > +Building GDBserver for your host is very straightforward. If you build > +GDB natively on a host which GDBserver supports, it will be built > automatically when you build GDB. You can also build just GDBserver: > > % mkdir obj > % cd obj > - % path-to-gdbserver-sources/configure > + % path-to-toplevel-sources/configure --disable-gdb > % make > > +(If you have a combined binutils+gdb tree, you may want to also > +disable other directories when configuring, e.g., binutils, gas, gold, > +gprof, and ld.) > + On a git checkout, configuring with: configure \ --disable-binutils \ --disable-ld \ --disable-gas \ --disable-gprof \ --disable-gold \ --disable-gdb and compiling with '$ make', still builds a number of top level directories other than gdbserver. We end up with this in the top level Makefile.in: SUBDIRS = intl libiberty opcodes bfd readline zlib libdecnumber libctf sim gdbserver etc So to build just gdbserver, you'd need to add a --disable-foo for each of those above in addition to the ones already indicated, something like: configure \ --disable-binutils \ --disable-ld \ --disable-gas \ --disable-gprof \ --disable-gold \ --disable-gdb \ --disable-intl \ --disable-libiberty \ --disable-opcodes \ --disable-bfd \ --disable-readline \ --disable-zlib \ --disable-libdecnumber \ --disable-libctf \ --disable-sim \ --disable-etc I guess it's the intended design for top level to build readline, bfd, etc. by default even if no application is being built that depends on them. I don't know. People may trip on this if they try to build gdbserver for a port for which one of those top level dirs doesn't build, like bfd. powerpc-lynxos is I think one such case, but there are probably others in the non-glibc space, and also off tree. However, a configure command, followed by "make all-gdbserver" builds only gdbserver and its required dependencies (which are none at the moment). So I'm thinking that it might be better to document "make all-gdbserver" instead of the --disable approach. Or at least, mention it as alternative. WDYT? > If you prefer to cross-compile to your target, then you can also build > GDBserver that way. In a Bourne shell, for example: > > % export CC=your-cross-compiler > - % path-to-gdbserver-sources/configure your-target-name > + % path-to-topevel-sources/configure your-target-name --disable-gdb > % make > > Using GDBreplay: > index 00000000000..86d3a80148d > --- /dev/null > +++ b/gdbserver/configure.host I noticed something about this file that made me look a bit more and I have a couple questions, including a patch. I noticed that several directories already contain a file named configure.host, including gdb: $ find . -name configure.host ./gdb/configure.host ./ld/configure.host ./gdbserver/configure.host ./bfd/configure.host And in gdb's ld's and bfd's case, this file is sourced by the respective directory's configure script, not by the top level script. If we wanted to migrate gdb's top configure bits to source a new file from gdb/ to determine whether gdb is supported, like gdbserver, we wouldn't be able to call the new "do we support gdb on this host" configure snippet file gdb/configure.host, since that file already exists with a different purpose. Likewise for ld and bfd. So, with that in mind, should we name gdbserver's new configure.host file to something else? However, looking at how libatomic, liboffloadmic, etc. handle this at the top level, I see that the top level sources a file that is also sourced by their respective configures. I.e., the file sourced served a dual purpose. From the top level, the file is sourced in subshell, which avoids variable polluting the top level. This has the advantage that you only have to write support for a port once in one file, instead of in two places. The equivalent for gdbserver would be the patch below, which seems to work well. Was there a reason you didn't follow libatomic's (etc.) model? From 0a8e011249c5d887886b1e06d5b3b774bd6f9d10 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Mon, 27 Jan 2020 12:53:10 +0000 Subject: [PATCH] UNSUPPORTED --- configure | 18 ++++---- configure.ac | 16 ++++--- gdbserver/configure.host | 111 ----------------------------------------------- gdbserver/configure.srv | 12 +++-- 4 files changed, 27 insertions(+), 130 deletions(-) delete mode 100644 gdbserver/configure.host base-commit: 4c5c9a7a0b64c98f60c2ab79ad705d14ea568d0f diff --git a/configure b/configure index c31220e8e75..8a3e7026f0b 100755 --- a/configure +++ b/configure @@ -3541,17 +3541,19 @@ esac # Only allow gdbserver on some systems. if test -d ${srcdir}/gdbserver; then if test x$enable_gdbserver = x; then - { $as_echo "$as_me:${as_lineno-$LINENO}: checking for gdbserver support" >&5 + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for gdbserver support" >&5 $as_echo_n "checking for gdbserver support... " >&6; } - . ${srcdir}/gdbserver/configure.host - if test x$build_gdbserver = xyes; then - { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5 -$as_echo "yes" >&6; } - else - noconfigdirs="$noconfigdirs gdbserver" + if (srcdir=${srcdir}/gdbserver; \ + . ${srcdir}/configure.srv; \ + test -n "$UNSUPPORTED") + then { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 $as_echo "no" >&6; } - fi + noconfigdirs="$noconfigdirs gdbserver" + else + { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5 +$as_echo "yes" >&6; } + fi fi fi diff --git a/configure.ac b/configure.ac index 40669228e3c..35a9c1867d2 100644 --- a/configure.ac +++ b/configure.ac @@ -785,14 +785,16 @@ esac # Only allow gdbserver on some systems. if test -d ${srcdir}/gdbserver; then if test x$enable_gdbserver = x; then - AC_MSG_CHECKING([for gdbserver support]) - . ${srcdir}/gdbserver/configure.host - if test x$build_gdbserver = xyes; then - AC_MSG_RESULT([yes]) - else - noconfigdirs="$noconfigdirs gdbserver" + AC_MSG_CHECKING([for gdbserver support]) + if (srcdir=${srcdir}/gdbserver; \ + . ${srcdir}/configure.srv; \ + test -n "$UNSUPPORTED") + then AC_MSG_RESULT([no]) - fi + noconfigdirs="$noconfigdirs gdbserver" + else + AC_MSG_RESULT([yes]) + fi fi fi diff --git a/gdbserver/configure.host b/gdbserver/configure.host deleted file mode 100644 index 86d3a80148d..00000000000 --- a/gdbserver/configure.host +++ /dev/null @@ -1,111 +0,0 @@ -# Configure helper for gdbserver -# Copyright (C) 2000-2020 Free Software Foundation, Inc. -# -# This file is part of GDB. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -# Source this to determine whether gdbserver can be built for a given -# host. This will set the variable "build_gdbserver". The variable -# "host" must be set before sourcing. - -build_gdbserver= -case "$host" in -aarch64*-*-linux*) - # Target: AArch64 linux - build_gdbserver=yes - ;; - -arm*-wince-pe | arm*-*-mingw32ce*) - # Target: ARM based machine running Windows CE (win32) - build_gdbserver=yes - ;; -arm*-*-linux*) - # Target: ARM based machine running GNU/Linux - build_gdbserver=yes - ;; -bfin-*-*linux*) - # Target: Blackfin Linux - gdb_target_obs="bfin-tdep.o bfin-linux-tdep.o linux-tdep.o" - build_gdbserver=yes - ;; -i[34567]86-*-nto*) - # Target: Intel 386 running qnx6. - build_gdbserver=yes - ;; -i[34567]86-*-linux*) - # Target: Intel 386 running GNU/Linux - build_gdbserver=yes - ;; -i[34567]86-*-cygwin*) - # Target: Intel 386 running win32 - build_gdbserver=yes - ;; -i[34567]86-*-mingw32*) - # Target: Intel 386 running win32 - build_gdbserver=yes - ;; -ia64-*-linux*) - # Target: Intel IA-64 running GNU/Linux - build_gdbserver=yes - ;; -m32r*-*-linux*) - # Target: Renesas M32R running GNU/Linux - build_gdbserver=yes - ;; -m68*-*-linux*) - # Target: Motorola m68k with a.out and ELF - build_gdbserver=yes - ;; -mips*-*-linux*) - # Target: Linux/MIPS - build_gdbserver=yes - ;; -powerpc*-*-linux*) - # Target: PowerPC running Linux - build_gdbserver=yes - ;; -s390*-*-linux*) - # Target: S390 running Linux - build_gdbserver=yes - ;; -sh*-*-linux*) - # Target: GNU/Linux Super-H - build_gdbserver=yes - ;; -sparc-*-linux*) - # Target: GNU/Linux SPARC - build_gdbserver=yes - ;; -sparc64-*-linux*) - # Target: GNU/Linux UltraSPARC - build_gdbserver=yes - ;; -tilegx-*-linux*) - # Target: TILE-Gx - build_gdbserver=yes - ;; -x86_64-*-linux*) - # Target: GNU/Linux x86-64 - build_gdbserver=yes - ;; -x86_64-*-mingw* | x86_64-*-cygwin*) - # Target: MingW/amd64 - build_gdbserver=yes - ;; -xtensa*-*-*linux*) - # Target: GNU/Linux Xtensa - build_gdbserver=yes - ;; -esac diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv index 8437c1ace06..52264be8ecd 100644 --- a/gdbserver/configure.srv +++ b/gdbserver/configure.srv @@ -1,6 +1,8 @@ # Mappings from configuration triplets to gdbserver build options. # This is invoked from the autoconf-generated configure script, to # produce the appropriate Makefile substitutions. +# It is also sourced by the top level configure script, to determine +# whether gdbserver is supported on a given host. # This file sets the following shell variables: # srv_regobj The register protocol appropriate for this target. @@ -12,6 +14,7 @@ # gdbserver in this configuration. # ipa_obj Any other target-specific modules appropriate # for this target's in-process agent. +# UNSUPPORTED Set to 1 if the host is unsupported. # # In addition, on GNU/Linux the following shell variables will be set: # srv_linux_regsets Set to "yes" if ptrace(PTRACE_GETREGS) and friends @@ -30,9 +33,9 @@ ipa_ppc_linux_regobj="powerpc-32l-ipa.o powerpc-altivec32l-ipa.o powerpc-vsx32l- # these files over and over again. srv_linux_obj="linux-low.o nat/linux-osdata.o nat/linux-procfs.o nat/linux-ptrace.o nat/linux-waitpid.o nat/linux-personality.o nat/linux-namespaces.o fork-child.o nat/fork-inferior.o" -# Input is taken from the "${target}" variable. +# Input is taken from the "${host}" variable. -case "${target}" in +case "${host}" in aarch64*-*-linux*) srv_tgtobj="linux-aarch64-low.o" srv_tgtobj="$srv_tgtobj nat/aarch64-linux-hw-point.o" srv_tgtobj="$srv_tgtobj linux-aarch32-low.o" @@ -395,7 +398,8 @@ case "${target}" in srv_linux_regsets=yes srv_linux_thread_db=yes ;; - *) echo "Error: target not supported by gdbserver." - exit 1 + *) + # Who are you? + UNSUPPORTED=1 ;; esac