From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 6VEPJutu/mb6tD8AWB0awg (envelope-from ) for ; Thu, 03 Oct 2024 06:16:11 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=EFKFiRrf; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 7BFE41E353; Thu, 3 Oct 2024 06:16:11 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-7.8 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_BLOCKED,RCVD_IN_VALIDITY_CERTIFIED,RCVD_IN_VALIDITY_RPBL, RCVD_IN_VALIDITY_SAFE,URIBL_BLOCKED,URIBL_DBL_BLOCKED_OPENDNS autolearn=ham autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 9DE611E08F for ; Thu, 3 Oct 2024 06:16:09 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EE5BD385DDDE for ; Thu, 3 Oct 2024 10:16:08 +0000 (GMT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 221F73858CDB for ; Thu, 3 Oct 2024 10:15:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 221F73858CDB Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 221F73858CDB Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727950548; cv=none; b=iD36Le5ByVQsdsFPB8X/SqnR3qDy4A/hJVGYl2XDutr2unV8KaaJvfZNsXClupaNSS57CoAcrsjhQCvgtQA+PJqoK5vKQwnFdikS5B7qpPIrx3uUtUjjg1A2/Fi1Dvwxx6bllVdfdbwiku1JGFZOz6SOa9/TewvfZZzYWjFKDTU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727950548; c=relaxed/simple; bh=WlToNILFklgUahAiWc8EhKud47kljWUm/sFzyUGRGqM=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=uzyz59pCUv55gEg9M1wFWiSn5ggEJeh/jxjZ8+uRKH4ryJCu69v4yXz0kLU4pBCCh/P0xlYHtE9yJlVl8ZoPCjht/LI4RzCZfriXJmuxlCapicd2kjhcOIakLYDtNEvlC3KqfPhDHUKU9nGpJOzqK72C+ZTxt6sqY7iezaFmlFE= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1727950543; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=DzVxh2XaCWkoqZdzlCB76cQFVqwZu+Tfi/Id2zWWyBo=; b=EFKFiRrfqOW4oz+qoawlHf/+bRQqIQWaWhWpNyQrP38EDUXCQcesaNueSfpwq2Hz+3etFK K86OKFvI8TurmYAfcFUsxu4CX9wWDB9X6CP39SHT49RVnZddzoY7GQNOz4g3CFYkgDNb1G /cGCaDUsDBQ1jntIBBSCmKhkarQAaOQ= Received: from mail-lf1-f69.google.com (mail-lf1-f69.google.com [209.85.167.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-381-HdxF-u-9MvSOY3ipM0yTag-1; Thu, 03 Oct 2024 06:15:42 -0400 X-MC-Unique: HdxF-u-9MvSOY3ipM0yTag-1 Received: by mail-lf1-f69.google.com with SMTP id 2adb3069b0e04-5389ef4c213so632283e87.3 for ; Thu, 03 Oct 2024 03:15:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727950541; x=1728555341; h=mime-version:message-id:date:references:in-reply-to:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=DzVxh2XaCWkoqZdzlCB76cQFVqwZu+Tfi/Id2zWWyBo=; b=V1MsIXQgJ68OVUxbv3IeGPkziVaOjEOwzhBqV3bplT0F6J/WZGnnNdnSdqU2jaedd5 /lBMfcfz83JHtq2GCVFqy88lwTFEDJz6X22z48F5L0+ELH5Sa+24XVyh/ylZAiHxjRUi Mmvr7WvTRmPCCf9e7+T6lnT/+EMtEQbtD+m81Kpzb8wiiiubQigl8LQa2zbnaVK1hSSR P/B1ma0Uf0Bg/X5zw9SLMhiMd4GqWJQEpizDI2yboZGhh7eRuAyksx08Y0WmuO9xTdhJ dJ7uN+QDZ0xX4pxxmHd1v1Ej4IrqiozFHx9k8nRWxfHV90VEDsHLLcWGuN5lSAxTIwtb 4gpQ== X-Forwarded-Encrypted: i=1; AJvYcCXUGjI9IhGxDtOcXyWHMKqNqhoOhWdM1O1mrmfX4JGMdkTE8pA7u1utuvIA/UWx/MotW9tdaYzARJpfQQ==@sourceware.org X-Gm-Message-State: AOJu0YxdAbkZsq86xTFB4g13QOQqIDTIz03tRQ9rtNQ6+rteCsztTLTK o+QhOJoddjg+jPQz8Gup5PtIBSzfHDZ4XjsgjtDMhSSbTrbcLlG012mirrBA1eORUtBDsddQe3s XZbfKjMbN+hvf1Xcs838/Xmi9n+n3Kleg7dBialxTeWThyHPzJEKNMaMsjSY= X-Received: by 2002:a05:6512:39c6:b0:52e:f9f1:c13a with SMTP id 2adb3069b0e04-539a06589b3mr3745960e87.12.1727950540460; Thu, 03 Oct 2024 03:15:40 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHbrkglV/GZjv16C0iIdHI9iVpQjE3TBFyrxU3oCmyD5kDuw5ykkQmzbi0+XzdBUnyWB/ULZA== X-Received: by 2002:a05:6512:39c6:b0:52e:f9f1:c13a with SMTP id 2adb3069b0e04-539a06589b3mr3745922e87.12.1727950539410; Thu, 03 Oct 2024 03:15:39 -0700 (PDT) Received: from localhost (243.223.159.143.dyn.plus.net. [143.159.223.243]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5c8ca4d6de3sm536609a12.76.2024.10.03.03.15.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Oct 2024 03:15:39 -0700 (PDT) From: Andrew Burgess To: Guinevere Larsen , gdb-patches@sourceware.org Subject: Re: [PATCH] gdb, configure: Add disable-formats option for configure In-Reply-To: <7cd41a93-3f67-4603-8178-16684401adb5@redhat.com> References: <20240925175340.1850969-1-guinevere@redhat.com> <87ploiy5ew.fsf@redhat.com> <7cd41a93-3f67-4603-8178-16684401adb5@redhat.com> Date: Thu, 03 Oct 2024 11:15:38 +0100 Message-ID: <87ldz5xzjp.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Guinevere Larsen writes: > On 10/2/24 10:56 AM, Andrew Burgess wrote: >> Guinevere Larsen writes: >> >>> GDB has support for many file formats, some which might be very unlikely >>> to be found in some situations (such as the COFF format in linux). This >>> commit introduces the option for a user to choose which formats GDB will >>> support at build configuration time. >>> >>> This is especially useful to avoid possible security concerns with >>> readers that aren't expected to be used at all, as they are one of >>> the simplest vectors for an attacker to try and hit GDB with. This >>> change also can reduce the size of the final binary, if that is a >>> concern. >>> >>> This commit adds a switch to the configure script allowing a user to >>> only enable selected file formats. The default behavior when the switch >>> is omitted is to compile all file formats, keeping the original behavior >>> of the script. >>> >>> When enabling selected readers, the configure script will also look at >>> the selected targets and may choose to add some readers that are the >>> default - or only - format available for the target. If that happens, >>> the script will emit the following warning: >>> >>> FOO is required to support one or more targets requested. Adding it >>> >>> Users aren't able to force the disabling of those formats, since tdep >>> files may directly call functions from the required readers. >>> >>> Ideally we'd like to be able to disable even those formats, in case a >>> user wants to build GDB only to examine remote files for example, but >>> the current infrastructure for the file format readers doesn't allow >>> us to do it. >>> --- >>> gdb/Makefile.in | 22 +++++++----- >>> gdb/NEWS | 11 ++++++ >>> gdb/README | 5 +++ >>> gdb/configure | 82 +++++++++++++++++++++++++++++++++++++++++--- >>> gdb/configure.ac | 68 ++++++++++++++++++++++++++++++++++-- >>> gdb/configure.format | 41 ++++++++++++++++++++++ >>> gdb/configure.tgt | 6 ++-- >>> gdb/doc/gdb.texinfo | 7 ++++ >>> 8 files changed, 225 insertions(+), 17 deletions(-) >>> create mode 100644 gdb/configure.format >>> >>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in >>> index bcf1ee45a70..009d68d6de2 100644 >>> --- a/gdb/Makefile.in >>> +++ b/gdb/Makefile.in >>> @@ -901,13 +901,24 @@ ALL_TARGET_OBS = \ >>> vax-tdep.o \ >>> windows-tdep.o \ >>> x86-tdep.o \ >>> - xcoffread.o \ >>> xstormy16-tdep.o \ >>> xtensa-config.o \ >>> xtensa-linux-tdep.o \ >>> xtensa-tdep.o \ >>> z80-tdep.o >>> >>> +# Object files for reading specific types of debug information. >>> +FORMAT_OBS = @FORMAT_OBS@ >>> + >>> +# All files that relate to GDB's ability to read debug information. >>> +# Used with --enable-formats=all. >>> +ALL_FORMAT_OBS = \ >>> + coff-pe-read.o \ >>> + coffread.o \ >>> + dbxread.o \ >>> + mipsread.o \ >>> + xcoffread.o >>> + >>> # The following native-target dependent variables are defined on >>> # configure.nat. >>> NAT_FILE = @NAT_FILE@ >>> @@ -1064,8 +1075,6 @@ COMMON_SFILES = \ >>> c-varobj.c \ >>> charset.c \ >>> cli-out.c \ >>> - coff-pe-read.c \ >>> - coffread.c \ >>> complaints.c \ >>> completer.c \ >>> copying.c \ >>> @@ -1079,7 +1088,6 @@ COMMON_SFILES = \ >>> d-lang.c \ >>> d-namespace.c \ >>> d-valprint.c \ >>> - dbxread.c \ >>> dcache.c \ >>> debug.c \ >>> debuginfod-support.c \ >>> @@ -1171,7 +1179,6 @@ COMMON_SFILES = \ >>> memtag.c \ >>> minidebug.c \ >>> minsyms.c \ >>> - mipsread.c \ >>> namespace.c \ >>> objc-lang.c \ >>> objfiles.c \ >>> @@ -1264,7 +1271,6 @@ SFILES = \ >>> d-exp.y \ >>> dtrace-probe.c \ >>> elf-none-tdep.c \ >>> - elfread.c \ >>> f-exp.y \ >>> gcore-elf.c \ >>> gdb.c \ >>> @@ -1886,7 +1892,6 @@ ALLDEPFILES = \ >>> windows-tdep.c \ >>> x86-nat.c \ >>> x86-tdep.c \ >>> - xcoffread.c \ >>> xstormy16-tdep.c \ >>> xtensa-config.c \ >>> xtensa-linux-nat.c \ >>> @@ -1908,7 +1913,8 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \ >>> $(SUBDIR_CLI_OBS) \ >>> $(SUBDIR_MI_OBS) \ >>> $(SUBDIR_TARGET_OBS) \ >>> - $(SUBDIR_GCC_COMPILE_OBS) >>> + $(SUBDIR_GCC_COMPILE_OBS) \ >>> + $(FORMAT_OBS) >>> >>> SUBDIRS = doc @subdirs@ data-directory >>> CLEANDIRS = $(SUBDIRS) >>> diff --git a/gdb/NEWS b/gdb/NEWS >>> index cfc9cb05f77..8d127558a1d 100644 >>> --- a/gdb/NEWS >>> +++ b/gdb/NEWS >>> @@ -92,6 +92,17 @@ vFile:stat >>> vFile:fstat but takes a filename rather than an open file >>> descriptor. >>> >>> +* Configure changes >>> + >>> +enable-formats=[FORMAT,]... >>> +enable-formats=all >>> + A user can now decide to only compile support for certain file formats. >>> + The available formats at this point are: dbx, coff, xcoff, elf, mach-o >>> + and mips. Some targets require specific file formats to be available, >>> + and in such cases, the configure script will warn the user and add >>> + support anyway. By default, all formats will be compiled in, to >>> + continue the behavior from before adding the switch. >>> + >>> *** Changes in GDB 15 >>> >>> * The MPX commands "show/set mpx bound" have been deprecated, as Intel >>> diff --git a/gdb/README b/gdb/README >>> index d85c37d5d17..342b2d07eb7 100644 >>> --- a/gdb/README >>> +++ b/gdb/README >>> @@ -403,6 +403,11 @@ more obscure GDB `configure' options are not listed here. >>> specified list of targets. The special value `all' configures >>> GDB for debugging programs running on any target it supports. >>> >>> +`--enable-formats=FORMAT,FORMAT,...' >>> +`--enable-formats=all` >>> + Configure GDB to be unable to read some binary file formats, such as >>> + coff, dbx or elf. >>> + >>> `--with-gdb-datadir=PATH' >>> Set the GDB-specific data directory. GDB will look here for >>> certain supporting files or scripts. This defaults to the `gdb' >>> diff --git a/gdb/configure b/gdb/configure >>> index 53eaad4f0e2..792e5cefefe 100755 >>> --- a/gdb/configure >>> +++ b/gdb/configure >>> @@ -706,6 +706,7 @@ LIBGUI >>> LTLIBLZMA >>> LIBLZMA >>> HAVE_LIBLZMA >>> +FORMAT_OBS >>> SER_HARDWIRE >>> WERROR_CFLAGS >>> WARN_CFLAGS >>> @@ -933,6 +934,7 @@ with_relocated_sources >>> with_auto_load_dir >>> with_auto_load_safe_path >>> enable_targets >>> +enable_formats >>> enable_64_bit_bfd >>> with_amd_dbgapi >>> enable_tui >>> @@ -1644,6 +1646,9 @@ Optional Features: >>> --disable-nls do not use Native Language Support >>> --enable-targets=TARGETS >>> alternative target configurations >>> + --enable-formats=FILE_FORMATS >>> + enable support for selected file formats(default >>> + 'all') >>> --enable-64-bit-bfd 64-bit support (on hosts with narrower word sizes) >>> --enable-tui enable full-screen terminal user interface (TUI) >>> --enable-gdbtk enable gdbtk graphical user interface (GUI) >>> @@ -11499,7 +11504,7 @@ else >>> lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 >>> lt_status=$lt_dlunknown >>> cat > conftest.$ac_ext <<_LT_EOF >>> -#line 11502 "configure" >>> +#line 11507 "configure" >>> #include "confdefs.h" >>> >>> #if HAVE_DLFCN_H >>> @@ -11605,7 +11610,7 @@ else >>> lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 >>> lt_status=$lt_dlunknown >>> cat > conftest.$ac_ext <<_LT_EOF >>> -#line 11608 "configure" >>> +#line 11613 "configure" >>> #include "confdefs.h" >>> >>> #if HAVE_DLFCN_H >>> @@ -24833,6 +24838,20 @@ esac >>> fi >>> >>> >>> +all_formats= >>> +# Check whether --enable-formats was given. >>> +if test "${enable_formats+set}" = set; then : >>> + enableval=$enable_formats; case "${enableval}" in >>> + yes | "") as_fn_error $? "enable-formats option must specify file formats or 'all'" "$LINENO" 5 >>> + ;; >>> + no) enable_formats= ;; >>> + *) enable_formats=$enableval ;; >>> +esac >>> +else >>> + all_formats=true >>> +fi >> I think it would be simpler to drop `all_formats` here, and make the >> default case just set `enable_formats=all`. Then later on, when you >> process `enable_formats` (where you have to check for 'all' anyway), >> you'll spot the 'all' case there and do whatever needs doing. I have >> another note about that logic below. > This makes sense, I'm not sure what I was thinking originally tbh. >> >>> + >>> + >>> # Check whether --enable-64-bit-bfd was given. >>> if test "${enable_64_bit_bfd+set}" = set; then : >>> enableval=$enable_64_bit_bfd; case $enableval in #( >>> @@ -24915,11 +24934,20 @@ fi >>> TARGET_OBS= >>> all_targets= >>> HAVE_NATIVE_GCORE_TARGET= >>> +# File formats that will ne enabled based on the selected >> type: ne ? > fixed >>> +# target(s). These are chosen because most, if not all, executables for >>> +# the target will follow this file format so it makes no sense to support >>> +# the target but not the debug information. >>> +target_formats= >>> +# If all targets were requested, this is all formats that should accompany >>> +# them. >>> +all_target_formats="elf xcoff mips coff" >>> >>> for targ_alias in `echo $target_alias $enable_targets | sed 's/,/ /g'` >>> do >>> if test "$targ_alias" = "all"; then >>> all_targets=true >>> + target_formats=$all_target_formats >>> else >>> # Canonicalize the secondary target names. >>> result=`$ac_config_sub $targ_alias 2>/dev/null` >>> @@ -24941,6 +24969,19 @@ fi >>> *" ${i} "*) ;; >>> *) >>> TARGET_OBS="$TARGET_OBS ${i}" >>> + # Decide which file formats are absolutely required based on >>> + # the requested targets. Warn later that they are added, in >>> + # case the user manually requested them, or requested all. >>> + # It's fine to add xcoff multiple times since the loop that >>> + # adds it to FORMAT_OBS will ensure that it is only added once. >>> + echo $i >> Is this required? Or is this left over debug output? > debug output, oops. fixed >> >>> + case "${i}" in >>> + *"windows-tdep.o" ) target_formats="${target_formats} coff";; >>> + "linux-tdep.o" ) target_formats="${target_formats} elf";; >>> + "rs6000-aix-tdep.o" ) target_formats="${target_formats} xcoff";; >>> + "rs6000-lynx178-tdep.o" ) target_formats="${target_formats} xcoff";; >>> + "mips-tdep.o" ) target_formats="${target_formats} mips";; >>> + esac >> I think this logic should be moved into configure.tgt. We already have >> a block at the end of that file which makes some choices based on which >> files are being added to the object list, which is exactly what you're >> doing here. This change would make the formats required by a target be >> an output from configure.tgt, which makes sense to me. > This makes sense. I'll do it. This also helps with the issue of needing > to run this check after determining the result of --target! >> >>> ;; >>> esac >>> done >>> @@ -24964,6 +25005,7 @@ if test x${all_targets} = xtrue; then >>> else >>> TARGET_OBS='$(ALL_TARGET_OBS)' >>> fi >>> + target_readers=$all_target_readers >>> fi >>> >>> # AMD debugger API support. >>> @@ -31462,6 +31504,7 @@ fi >>> # Note that WIN32APILIBS is set by GDB_AC_COMMON. >>> WIN32LIBS="$WIN32LIBS $WIN32APILIBS" >>> >>> +support_elf=no >>> # Add ELF support to GDB, but only if BFD includes ELF support. >>> >>> OLD_CFLAGS=$CFLAGS >>> @@ -31514,7 +31557,7 @@ $as_echo "$gdb_cv_var_elf" >&6; } >>> LDFLAGS=$OLD_LDFLAGS >>> LIBS=$OLD_LIBS >>> if test "$gdb_cv_var_elf" = yes; then >>> - CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o dtrace-probe.o \ >>> + CONFIG_OBS="$CONFIG_OBS stap-probe.o dtrace-probe.o \ >>> gcore-elf.o elf-none-tdep.o" >>> >>> $as_echo "#define HAVE_ELF 1" >>confdefs.h >>> @@ -31578,9 +31621,11 @@ if test "$ac_res" != no; then : >>> fi >>> >>> fi >>> + support_elf=yes >>> fi >>> >>> # Add macho support to GDB, but only if BFD includes it. >>> +support_macho=no >>> >>> OLD_CFLAGS=$CFLAGS >>> OLD_LDFLAGS=$LDFLAGS >>> @@ -31632,9 +31677,38 @@ $as_echo "$gdb_cv_var_macho" >&6; } >>> LDFLAGS=$OLD_LDFLAGS >>> LIBS=$OLD_LIBS >>> if test "$gdb_cv_var_macho" = yes; then >>> - CONFIG_OBS="$CONFIG_OBS machoread.o" >>> + support_macho=yes >>> fi >>> >>> +FORMAT_OBS= >>> +enable_formats=$(echo $enable_formats | sed 's/,/ /g') >>> + >>> +if test "$all_formats" = "true"; then >>> + FORMAT_OBS='$(ALL_FORMAT_OBS)' >> As Tom pointed out, right now we effectively have the list of format >> reading .c files twice. >> >> If instead of using ALL_FORMAT_OBS here we instead expanded ' all ' >> inside enable_formats into the list of all known formats >> (e.g. $all_target_formats), then, when we run through the loop below, >> FORMAT_OBS would always be set to the complete list, right? Then we'd >> not need the file list in the Makefile at all, instead we just need >> all_target_formats defined in configure.ac and then handling for each >> value in configure.format. > > $all_target_formats doesn't contain dbx, which is why things look > duplicated but aren't really. > > I guess I could have something like non_target_formats to list all the > file formats that aren't required to compile some target, and then use > both to set FORMAT_OBS. This way in the future if we have other formats > that aren't required by a target, we can place them there. What do you > think? Whatever works I guess. I'm sure there must be some way we can figure out the list of require .o files from the configure script and then pass that to the Makefile. It might mean a little more complexity in the configure script, but I think the benefit of having the configure script hold all the logic will be worth it. Thanks, Andrew > >> >>> +else >>> + # formats that are required by some requested target(s). >>> + # Warn users that they are added, so no one is surprised. >>> + for req in $target_formats; do >>> + if ! echo "$enable_formats" | grep -wq "$req"; then >>> + echo "$req is required to support one or more targets requested. Adding it" >> Instead of 'echo', maybe try AC_MSG_WARN maybe? Or AC_MSG_NOTICE? >> Check out: >> >> https://www.gnu.org/software/autoconf/manual/autoconf-2.60/html_node/Printing-Messages.html >> >> for options. > I'll go with AC_MSG_WARN, so it's unlikely the user will not see it, > thanks for the suggestion! >> >>> + enable_formats="${enable_formats} $req" >>> + fi >>> + done >>> + >>> + for format in $enable_formats >>> + do >>> + if test "$format" = "all"; then >>> + all_formats=true >>> + fi >>> + >>> + . ${srcdir}/configure.format >>> + done >>> +fi >>> + >>> +echo $FORMAT_OBS >> Is this left over debug? > yes, removed. >> >>> + >>> + >>> + >>> # Add any host-specific objects to GDB. >>> CONFIG_OBS="${CONFIG_OBS} ${gdb_host_obs}" >>> >>> diff --git a/gdb/configure.ac b/gdb/configure.ac >>> index 8368fea0423..5f5187ecd0f 100644 >>> --- a/gdb/configure.ac >>> +++ b/gdb/configure.ac >>> @@ -187,6 +187,16 @@ AS_HELP_STRING([--enable-targets=TARGETS], [alternative target configurations]), >>> *) enable_targets=$enableval ;; >>> esac]) >>> >>> +all_formats= >>> +AC_ARG_ENABLE(formats, >>> + AS_HELP_STRING([--enable-formats=FILE_FORMATS], [enable support for selected file formats(default 'all')]), >>> +[case "${enableval}" in >>> + yes | "") AC_MSG_ERROR(enable-formats option must specify file formats or 'all') >>> + ;; >>> + no) enable_formats= ;; >>> + *) enable_formats=$enableval ;; >>> +esac], [all_formats=true]) >>> + >>> BFD_64_BIT >>> >>> # Provide defaults for some variables set by the per-host and per-target >>> @@ -206,11 +216,20 @@ fi >>> TARGET_OBS= >>> all_targets= >>> HAVE_NATIVE_GCORE_TARGET= >>> +# File formats that will be enabled based on the selected >>> +# target(s). These are chosen because most, if not all, executables for >>> +# the target will follow this file format so it makes no sense to support >>> +# the target but not the debug information. >>> +target_formats= >>> +# If all targets were requested, this is all formats that should accompany >>> +# them. >>> +all_target_formats="elf xcoff mips coff" >>> >>> for targ_alias in `echo $target_alias $enable_targets | sed 's/,/ /g'` >>> do >>> if test "$targ_alias" = "all"; then >>> all_targets=true >>> + target_formats=$all_target_formats >>> else >>> # Canonicalize the secondary target names. >>> result=`$ac_config_sub $targ_alias 2>/dev/null` >>> @@ -231,6 +250,19 @@ do >>> *" ${i} "*) ;; >>> *) >>> TARGET_OBS="$TARGET_OBS ${i}" >>> + # Decide which file formats are absolutely required based on >>> + # the requested targets. Warn later that they are added, in >>> + # case the user manually requested them, or requested all. >>> + # It's fine to add xcoff multiple times since the loop that >>> + # adds it to FORMAT_OBS will ensure that it is only added once. >>> + echo $i >>> + case "${i}" in >>> + *"windows-tdep.o" ) target_formats="${target_formats} coff";; >>> + "linux-tdep.o" ) target_formats="${target_formats} elf";; >>> + "rs6000-aix-tdep.o" ) target_formats="${target_formats} xcoff";; >>> + "rs6000-lynx178-tdep.o" ) target_formats="${target_formats} xcoff";; >>> + "mips-tdep.o" ) target_formats="${target_formats} mips";; >>> + esac >>> ;; >>> esac >>> done >>> @@ -1850,11 +1882,12 @@ fi >>> # Note that WIN32APILIBS is set by GDB_AC_COMMON. >>> WIN32LIBS="$WIN32LIBS $WIN32APILIBS" >>> >>> +support_elf=no >>> # Add ELF support to GDB, but only if BFD includes ELF support. >>> GDB_AC_CHECK_BFD([for ELF support in BFD], gdb_cv_var_elf, >>> [bfd_get_elf_phdr_upper_bound (NULL)], elf-bfd.h) >>> if test "$gdb_cv_var_elf" = yes; then >>> - CONFIG_OBS="$CONFIG_OBS elfread.o stap-probe.o dtrace-probe.o \ >>> + CONFIG_OBS="$CONFIG_OBS stap-probe.o dtrace-probe.o \ >>> gcore-elf.o elf-none-tdep.o" >> Maybe this is something we can unpick later, but if we only add all >> these .o files if we have ELF support. And if we chose to configure GDB >> without ELF support .... should we actually be dropping all these .o >> files? > > I could also invert the logic of my change here. Instead of setting this > part of "elf support" and adding the .o file later, if I moved that > format part to be higher up, closer to the target stuff (where I had it > originally), I can have that section request the elf target, and this > part be fully skipped if elf wasn't requested (and error out if we can't > support but the user asked for it). > > Same would go for Mach-O. > > What do you think? > >> >>> AC_DEFINE(HAVE_ELF, 1, >>> [Define if ELF support should be included.]) >>> @@ -1862,15 +1895,46 @@ if test "$gdb_cv_var_elf" = yes; then >>> if test "$plugins" = "yes"; then >>> AC_SEARCH_LIBS(dlopen, dl) >>> fi >>> + support_elf=yes >>> fi >>> >>> # Add macho support to GDB, but only if BFD includes it. >>> +support_macho=no >>> GDB_AC_CHECK_BFD([for Mach-O support in BFD], gdb_cv_var_macho, >>> [bfd_mach_o_lookup_command (NULL, 0, NULL)], mach-o.h) >>> if test "$gdb_cv_var_macho" = yes; then >>> - CONFIG_OBS="$CONFIG_OBS machoread.o" >>> + support_macho=yes >>> fi >>> >>> +FORMAT_OBS= >>> +enable_formats=$(echo $enable_formats | sed 's/,/ /g') >>> + >>> +if test "$all_formats" = "true"; then >>> + FORMAT_OBS='$(ALL_FORMAT_OBS)' >>> +else >>> + # Formats that are required by some requested target(s). >>> + # Warn users that they are added, so no one is surprised. >>> + for req in $target_formats; do >>> + if ! echo "$enable_formats" | grep -wq "$req"; then >>> + echo "$req is required to support one or more targets requested. Adding it" >>> + enable_formats="${enable_formats} $req" >>> + fi >>> + done >>> + >>> + for format in $enable_formats >>> + do >>> + if test "$format" = "all"; then >>> + all_formats=true >>> + fi >> Maybe I'm missing something, but isn't setting 'all_formats' here too >> late? That is, we are already inside the 'else' block which checked the >> 'all_formats' variable, and after this point it's not (as far as I can >> tell) checked again. > You're right! I think this was a leftover from moving things around. > However, I think I like your idea of dealing with "all" inside > configure.format anyway, I just didn't do it at first because I was > copying what the target stuff does. >>> + >>> + . ${srcdir}/configure.format >>> + done >>> +fi >>> + >>> +echo $FORMAT_OBS >>> + >>> +AC_SUBST(FORMAT_OBS) >>> + >>> # Add any host-specific objects to GDB. >>> CONFIG_OBS="${CONFIG_OBS} ${gdb_host_obs}" >>> >>> diff --git a/gdb/configure.format b/gdb/configure.format >>> new file mode 100644 >>> index 00000000000..12dd2d25717 >>> --- /dev/null >>> +++ b/gdb/configure.format >>> @@ -0,0 +1,41 @@ >>> +# Copyright (C) 2024 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 . >>> + >>> +# This file is used to decide which files need to be compiled to support >>> +# the requested file formats >>> + >>> +case $format in >>> + xcoff) FORMAT_OBS="$FORMAT_OBS xcoffread.o" ;; >> In all the other configure.* files the format used is like this: >> >> xcoff) >> FORMAT_OBS="$FORMAT_OBS xcoffread.o" >> ;; >> >> I think we should adopt this for consistency (I also prefer it, but lets >> go with the consistency argument). > Alright, will fix for the next version. > > -- > Cheers, > Guinevere Larsen > She/Her/Hers > >> >> Thanks, >> Andrew >> >>> + >>> + # Despite the naming convention implying coff-pe to be a separate >>> + # reader, it is in fact just a helper for coffread; >>> + coff) FORMAT_OBS="$FORMAT_OBS coffread.o coff-pe-read.o" ;; >>> + >>> + dbx) FORMAT_OBS="$FORMAT_OBS dbxread.o" ;; >>> + >>> + elf) if "$support_elf"="yes"; then >>> + FORMAT_OBS="$FORMAT_OBS elfread.o" >>> + fi >>> + ;; >>> + >>> + macho) if "$support_macho"="yes"; then >>> + FORMAT_OBS="$FORMAT_OBS machoread.o" >>> + fi >>> + ;; >>> + >>> + mips) FORMAT_OBS="$FORMAT_OBS mipsread.o" ;; >>> +esac >>> diff --git a/gdb/configure.tgt b/gdb/configure.tgt >>> index 8d85a597ec8..793793601c1 100644 >>> --- a/gdb/configure.tgt >>> +++ b/gdb/configure.tgt >>> @@ -507,7 +507,7 @@ powerpc-*-openbsd*) >>> ;; >>> powerpc-*-aix* | rs6000-*-* | powerpc64-*-aix*) >>> # Target: PowerPC running AIX >>> - gdb_target_obs="rs6000-tdep.o rs6000-aix-tdep.o xcoffread.o \ >>> + gdb_target_obs="rs6000-tdep.o rs6000-aix-tdep.o \ >>> ppc-sysv-tdep.o solib-aix.o \ >>> ravenscar-thread.o ppc-ravenscar-thread.o" >>> ;; >>> @@ -523,8 +523,8 @@ powerpc*-*-linux*) >>> powerpc-*-lynx*178) >>> # Target: PowerPC running Lynx178. >>> gdb_target_obs="rs6000-tdep.o rs6000-lynx178-tdep.o \ >>> - xcoffread.o ppc-sysv-tdep.o \ >>> - ravenscar-thread.o ppc-ravenscar-thread.o" >>> + ppc-sysv-tdep.o ravenscar-thread.o \ >>> + ppc-ravenscar-thread.o" >>> ;; >>> powerpc*-*-*) >>> # Target: PowerPC running eabi >>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo >>> index 77a4021b36a..c569e68060e 100644 >>> --- a/gdb/doc/gdb.texinfo >>> +++ b/gdb/doc/gdb.texinfo >>> @@ -41177,6 +41177,13 @@ Configure @value{GDBN} for cross-debugging programs running on the >>> specified list of targets. The special value @samp{all} configures >>> @value{GDBN} for debugging programs running on any target it supports. >>> >>> +@item --enable-formats=@r{[}@var{format}@r{]}@dots{} >>> +@itemx --enable-formats=all >>> +Configure @value{GDBN} to support certain binary file formats. If a >>> +format is the main (or only) file format for a given target, the >>> +configure script may add support to it anyway, and warn the user. >>> +If not given, all file formats that @value{GDBN} supports are compiled. >>> + >>> @item --with-gdb-datadir=@var{path} >>> Set the @value{GDBN}-specific data directory. @value{GDBN} will look >>> here for certain supporting files or scripts. This defaults to the >>> -- >>> 2.46.1