From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 6DO0NApikmhIqwQAWB0awg (envelope-from ) for ; Tue, 05 Aug 2025 15:56:58 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1754423818; bh=P8cHXfviu5ooCcty12BYnmuWvr8JaO3u7krYXGewSm0=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=GFiEzFBS6VJexWwQJplsbbBMRn8xtjkiRTR1+q6SWXe8sXl8R+E9XomdbE5S+PXe+ TJOjfhjMAdhhFk4Jr3w5wxWP2qa1ke7f9WX4+eyi4b7gj6u2HwubuDjVbpZsWK4NUd As0n0TqVfeN7oCBTiQavCoBHiM0YIu4/jMRDoeJA= Received: by simark.ca (Postfix, from userid 112) id D12751E100; Tue, 5 Aug 2025 15:56:58 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-9.1 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED,RCVD_IN_VALIDITY_RPBL, RCVD_IN_VALIDITY_SAFE autolearn=unavailable autolearn_force=no version=4.0.1 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=WBXG8cPf; dkim=pass (1024-bit key) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=HHp98+yb; dkim-atps=neutral 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 B98F91E091 for ; Tue, 5 Aug 2025 15:56:57 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 46C6F3858408 for ; Tue, 5 Aug 2025 19:56:57 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 46C6F3858408 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=WBXG8cPf; dkim=pass (1024-bit key) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=HHp98+yb Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 28279385B516 for ; Tue, 5 Aug 2025 16:03:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 28279385B516 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 28279385B516 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1754409813; cv=none; b=FgaeGjHJ8BpXyDEiMBtL0u+WtCXd4/exlzLCvGisw57tFNynyhN06kp+/dLGOcdn8cMhbnRo7C2uWdW0rmpfGWg/wAoPj26RdYncfFkYP7gjw2vj3fg8wqrB13CGW1ihHD0mZA7yS72/fQr8kQsrlRU8d3ck+hW3P/13N6lG+qs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1754409813; c=relaxed/simple; bh=P8cHXfviu5ooCcty12BYnmuWvr8JaO3u7krYXGewSm0=; h=DKIM-Signature:DKIM-Signature:Message-ID:Date:MIME-Version: Subject:To:From; b=khrNesgsvIrdvMUJ6YkgaCyYaENGixcOFVq+lXwo/QhigLdsKLAZNQ6fbiFzEYVsXSiKYvTzJScEviXHkWff/9RBMYtJ1JpPE2IjJFCmrbzOc480bFm2QKtMEp1HLKcC+ZinWbVT4D9b6P9XHGWfahkxAxPBsw4JNxrodZlwHlA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 28279385B516 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1754409812; bh=P8cHXfviu5ooCcty12BYnmuWvr8JaO3u7krYXGewSm0=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=WBXG8cPfjiUdTpr0av2r5De8ZpMPU61yBOVBe3sCXW0Ry4DLGkzju0yTR94yLx0PK 1IW4sfnVfywTUah6nYHcKb0Lr0E/iruM8wC+CErMX+aO+xeJ+nqA+leM1E9C0Ewq01 n9HiNIIJYvJLvlZlQMMdYhpw1qjN7Y71NYqsrvzo= Received: by simark.ca (Postfix, from userid 112) id B6C681E11F; Tue, 5 Aug 2025 12:03:32 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1754409809; bh=P8cHXfviu5ooCcty12BYnmuWvr8JaO3u7krYXGewSm0=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=HHp98+yb/gBiAA5ZUodW+ma4lKQx541jxBAuyjiPF3k1HCUtb3XMZ3a7PqKZwWIXb rzSXThVMVAI4ncm2j9+U4hRTzwxrPnl+MCzEjoWkOsgiSpCXGdrH2P9P3UWAE1cS3P ep6yGMHi077M4lyuPhXsJ9RM6AvP7zOROzuE6JCI= Received: from [172.24.0.6] (192-222-132-26.qc.cable.ebox.net [192.222.132.26]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 75FA11E091; Tue, 5 Aug 2025 12:03:29 -0400 (EDT) Message-ID: <551458c7-bcb7-4643-91e4-9e01e0792425@simark.ca> Date: Tue, 5 Aug 2025 12:03:29 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6] gdb, configure: Add disable-formats option for configure To: Guinevere Larsen , gdb-patches@sourceware.org Cc: Eli Zaretskii References: <20250526171111.2633770-1-guinevere@redhat.com> <20250707201657.972546-2-guinevere@redhat.com> Content-Language: fr From: Simon Marchi In-Reply-To: <20250707201657.972546-2-guinevere@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 On 7/7/25 4:16 PM, Guinevere Larsen wrote: > GDB has support for many binary file formats, some which might be very > unlikely to be found in some situations (such as the XCOFF format in > an x86 system). 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 can also 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, called --enable-binary-file-formats. > The default behavior when the switch is omitted is to compile all file > formats, keeping the original behavior of the script. At the time of > this commit, the valid options for this option are: dbx, coff (which > includes coff-pe), xcoff, mips, elf, macho and all. All is treated > especially, activating all supported readers. > > A few targets may require specific binary file format support, as they > directly call functions defined by the file reader. Specifically, > windows targets require coff support, and rs6000 aix and lynx178 targets > require xcoff support. Considering that those formats are the main - or > only - one available in those targets, I believe it makes sense to > re-enable those readers. If that happens, the script will emit the > following warning: > > FOO is required to support one or more requested targets. Adding it > > Users aren't able to force the disabling of those formats, since GDB > will not compile without those 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. > > Mach-O and elf support are also dependent on BFD support being compiled > in. In case one of those was requested and BFD does not support them, > the following error is emitted: > > FOO was requested, but BFD does not support it. > > Finally, this configure switch is also printed by the "show > configuration" command in GDB. > > Reviewed-By: Eli Zaretskii > --- > > This is rebased on current master, there was a bit of a rebase conflict, > but nothing complicated. I think the git commit subject needs to be updated to reflect the new option name. > diff --git a/gdb/README b/gdb/README > index eca4181c751..d45e6d37c4a 100644 > --- a/gdb/README > +++ b/gdb/README > @@ -417,6 +417,30 @@ more obscure GDB `configure' options are not listed here. > There is no convenient way to generate a list of all available > targets. > > +`--enable-binary-file-formats=FORMAT,FORMAT,...' > +`--enable-binary-file-formats=all' > + Configure GDB to only be be able to read selected file formats. > + The special value "all" causes all file formats to be compiled > + in, and is the the default behavior of the option. This option > + is meant for advanced users who are sure of what they expect, > + if you are unsure which options you will need on your debugging > + sessions, we recommend that you not use this feature. The > + accepted options are: In the list below, a few instances of "two spaces after period". > + * coff: Main format on windows systems. This is required to > + compile with windows target support; windows -> Windows > + * dbx (also known as a.out): is a legacy file format, this > + is recommended if you know you will be dealing with this > + file format; Missing "This" at the beginning, to be like the other entries. > + * elf: Main format of linux systems. This is heavily > + recommended when compiling with linux support; linux -> Linux For consistency: in all entries except this one, the "This is heavily" is just a phrase following a comma. I would change this one to match. In general if you can make the form consistent across items, it would be nice. > + * macho: Main format on MacOS systems, this is heavily > + recommended when compiling for those targets; MacOS -> macOS > @@ -191,6 +191,15 @@ AS_HELP_STRING([--enable-targets=TARGETS], [alternative target configurations]), > ;; > esac]) > > +AC_ARG_ENABLE(binary_file_formats, > + AS_HELP_STRING([--enable-binary-file-formats=FILE_FORMATS], Question mostly for Eli, should the metavar use an underscore or dash ("FILE-FORMATS")? > + [enable support for selected file formats (default 'all')]), Since the list of valide values is finite and not too long, it might be nice to list the possible values right there in the help message. > +[case "${enableval}" in > + yes | "") AC_MSG_ERROR(enable-formats option must specify file formats or 'all') enable-formats -> enable-binary-file-formats I think you are also missing a space before "yes". > @@ -239,11 +248,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 they are required to > +# compile one or more of the selected targets. > +target_formats= > +# If all targets were requested, this is all formats that should > +# accompany them. These are just the ones required for compilation > +# to succeed, not the formats suggested based on targets. > +all_target_formats="coff xcoff" Please add empty lines before each comment above. > @@ -1952,11 +1970,17 @@ fi > # Note that WIN32APILIBS is set by GDB_AC_COMMON. > WIN32LIBS="$WIN32LIBS $WIN32APILIBS" > > +# Object files to be used when building with support for all file formats. > +# This should not have elf or macho, as support for those formats depends > +# on BFD enabling them as well. > +all_binary_file_srcs="\$(dbx_SRCS) \$(mips_SRCS) \$(coff_SRCS) \$(xcoff_SRCS)" > + > +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" Does it make sense (or is it useful) to compile these other files (other than elfread.o) if ELF support is not enabled? Could we just list them all in elf_SRCS so that they are not built if the user doesn't want ELF support? If you don't have an ELF reader. If I understand correctly, "support_elf" means that BFD supports ELF, not that GDB will support ELF, right? If so, could you rename it to "bfd_supports_elf"? I also wouldn't mind if you renamed the conf var gdb_cv_var_elf to something like gdb_cv_var_bfd_elf. > AC_DEFINE(HAVE_ELF, 1, > [Define if ELF support should be included.]) > @@ -1964,15 +1988,67 @@ if test "$gdb_cv_var_elf" = yes; then > if test "$plugins" = "yes"; then > AC_SEARCH_LIBS(dlopen, dl) > fi > + support_elf=yes > + all_binary_file_srcs="$all_binary_file_srcs \$(elf_SRCS)" > fi > > # Add macho support to GDB, but only if BFD includes it. > +support_macho=no Likewise for support_macho. > 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 > + all_binary_file_srcs="$all_binary_file_srcs \$(macho_SRCS)" > +fi > + > +FORMAT_SRCS= > + > +if test "$enable_binary_file_formats" != "all"; then > + # 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 > + case ,$enable_binary_file_formats, in > + *,$req,*) > + # Do nothing. > + ;; Does this work if $req is first or last in the list? > + *) > + AC_MSG_WARN("$req is required to support one or more requested targets. Adding it") > + enable_binary_file_formats="${enable_binary_file_formats},$req" > + ;; > + esac > + done > + > + AC_DEFINE_UNQUOTED(SUPPORTED_FORMATS, "$enable_binary_file_formats", > + Which file formats were requested at configure time. ) I think that all variables related to this should be renamed to include "binary file". For instance, SUPPORTED_FORMATS -> SUPPORTED_BINARY_FILE_FORMATS. There are a few others in the configure script or Makefile. Also, "Which file formats" -> "Which binary file formats". > fi > > +enable_binary_file_formats=$(echo $enable_binary_file_formats | sed 's/,/ /g') > + > +for format in $enable_binary_file_formats > +do Can you write a comment above that loop to indicate what it does? > + if test "$format" = "elf"; then > + if test "$support_elf" != "yes"; then > + AC_MSG_ERROR("elf support was requested, but BFD does not support it."); > + fi > + elif test "$format" = "macho"; then > + if test "$support_macho" != "yes"; then > + AC_MSG_ERROR("Mach-O support was requested, but BFD does not support it."); > + fi > + fi Can you write the above as if test "$format" = "elf" && "$support_elf" != "yes"; then AC_MSG_ERROR("elf support was requested, but BFD does not support it."); fi if test "$format" = "macho" && "$support_macho" != "yes"; then AC_MSG_ERROR("Mach-O support was requested, but BFD does not support it."); fi ? I find it a bit easier to read (less boilerplate). Simon