From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id AEqsBDxlkmjErgQAWB0awg (envelope-from ) for ; Tue, 05 Aug 2025 16:10:36 -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=IUBx4Kbo; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 0D0461E100; Tue, 5 Aug 2025 16:10:36 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.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_MED,RCVD_IN_SBL_CSS,RCVD_IN_VALIDITY_CERTIFIED, RCVD_IN_VALIDITY_RPBL,RCVD_IN_VALIDITY_SAFE autolearn=unavailable autolearn_force=no version=4.0.1 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 A09EA1E091 for ; Tue, 5 Aug 2025 16:10:34 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 586AE3858C42 for ; Tue, 5 Aug 2025 20:10:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 586AE3858C42 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=IUBx4Kbo 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 AEC1B385840C for ; Tue, 5 Aug 2025 20:07:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AEC1B385840C Authentication-Results: sourceware.org; dmarc=pass (p=quarantine 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 AEC1B385840C 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=1754424459; cv=none; b=AxujVq5kQPPFXLXgK8TbFSQ13ltEjE+4Z/Akqmtzm0gzpTJo4GePM/nhgrQ7tFEG3L3nGDmpZ3XuL3EONJXgUDrdyjanFBzdyK+3meZT3a62qptwA2E8gQ5FGPqpma7tNqPhDIfkXoGzuxeCnYnb4l2E7i6GrFx01/dTTXg3pXg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1754424459; c=relaxed/simple; bh=6Z4CrBDiYH4LoiEWBcS59lCapFdVlng+dHEX/DXIUcY=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=OTOS0kbHDS1EHpJTiaYAxRi1sgtO0YAoQtNEnVqap0pknsidZq5OZKo/705pifw+NwIfv157QsZOAtgN17lK4h0a6vcixp+iN3CbaFLuGosmBrOaAisO9uRpD+HaXWe3Hfclm1nw0X8Wz+QX5PEwt61/bUpjs0IMaLaQbuQlSsA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AEC1B385840C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1754424459; 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=SGPUgLeL4hh5RszGmrIWLhqapXQfLltvIqHC6VDRGkI=; b=IUBx4KboKx2F/3+qm/8DteM6dZHToMxLbR84VcK3gbJUHSaTKPqslzaDoEt8hqbmYZdrsm YlMuF96c6G7UC5ke9zHfHbrSrFHTztVmkzFus7uNv900+OujuU3ryoLgi0f/d2VwdwhPV8 4AHwRKGPhquwcLnPYhTdNpC+T8THI80= Received: from mail-pj1-f71.google.com (mail-pj1-f71.google.com [209.85.216.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-27-LkfkLntbNUmvBytLDLGU2Q-1; Tue, 05 Aug 2025 16:07:38 -0400 X-MC-Unique: LkfkLntbNUmvBytLDLGU2Q-1 X-Mimecast-MFC-AGG-ID: LkfkLntbNUmvBytLDLGU2Q_1754424457 Received: by mail-pj1-f71.google.com with SMTP id 98e67ed59e1d1-31332dc2b59so6240199a91.0 for ; Tue, 05 Aug 2025 13:07:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1754424457; x=1755029257; h=content-transfer-encoding:in-reply-to:content-language:from :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=SGPUgLeL4hh5RszGmrIWLhqapXQfLltvIqHC6VDRGkI=; b=aqRdd+ux3WbrjKQeSp5HiD8ZknH9VsoHSxq6AOmskPtTkJagucJFL/nZrRvRHwajng 4+18vm3wmnYKLsMPNMgkKUBgx47KvtepPZe9MTQd6md31h3YEQR146tnORmkMHGsOUrX deMzlarUrCF9u2qArEfX2c8EE25ZnuT6xUlzg0x679p40tE3orSY8MHEpiEoMP7kW/8A VL3ttaY5IeFZ8/ydPxAX69FegeoMPUOFInKaXR+85JMYjtrNDXuvh30UNbqtxBlxN0kZ k2IK3PoOrH7FgviIEjH+gz+RUmS5/h5XGU9VpprvdY6Xji9rCXoGLZl4tRT+tklp/C7e CORw== X-Forwarded-Encrypted: i=1; AJvYcCXd2tGI7to1QhodcSxNtYfiFgPR1yo/bRESYtQj0fB1dJ+X43LKBjAaU+BI6j5bPO2P9+geF1vS1qLtfQ==@sourceware.org X-Gm-Message-State: AOJu0Yx0pS0BCE0UxA/Mt7wbp1C1Y80hfb1EW1XidCxyzmXnAhpXu/+e fZyLHgaur1hY7RxmDqT7t2X+ExVFRH4ElIun1FAGrRCjirV1MX6NWVXEpU2iX0Ffe2MfO+Pyh7X 6R2nOvq9vJh9N82eBah1ZjVwuEiyELM0SJe+DPu4q6zDLfQitSXwxaibuewIc2kc= X-Gm-Gg: ASbGncvHO43Bo6ItGyGPv+mscK1hIF+bcXuCAzFhYfuT/2NPulQ8OxLlg2jRtm9vpsD IXp6W84XRAFiRVzwOtDk+pbDJCVyzB2HK3LTCAv/o2GL4xxZmhl3xZvXRVjgS6/r2CpkO6gapwx uyb1Dkox0EdycykiZJJA1y19BvR8gYbPC8TpKUUQQuVepsN+9UGMTbZGAvWJekz3daQx0mdzK2W dDhqiA/PItrFsLAD9VaTDz9joTuKXcx28c83rY7oIDFiSXP2uY1L57LCRfdp3lNevAM1VyQAqFO ZG8Zw5CGmFZlS+nPdVNhXEpEAzQKXJ3b+JSC6m7jlcvurr9cnSU= X-Received: by 2002:a17:90a:e7ce:b0:31f:42cd:6900 with SMTP id 98e67ed59e1d1-32166c2d499mr161976a91.12.1754424456983; Tue, 05 Aug 2025 13:07:36 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG5Bl8FZxDJY5yiVeA7yaSGtHkxtVIi0wh0o2KYY6OisrkSUWn4/VYusnH1qNwrQ1xMSzx6Cg== X-Received: by 2002:a17:90a:e7ce:b0:31f:42cd:6900 with SMTP id 98e67ed59e1d1-32166c2d499mr161937a91.12.1754424456426; Tue, 05 Aug 2025 13:07:36 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:9a69::1000? ([2804:14d:8084:9a69::1000]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-3216129d6b9sm932512a91.35.2025.08.05.13.07.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 05 Aug 2025 13:07:35 -0700 (PDT) Message-ID: <995b2448-6e30-4ba6-acb8-099b0723ee83@redhat.com> Date: Tue, 5 Aug 2025 17:07:32 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6] gdb, configure: Add disable-formats option for configure To: Simon Marchi , gdb-patches@sourceware.org Cc: Eli Zaretskii References: <20250526171111.2633770-1-guinevere@redhat.com> <20250707201657.972546-2-guinevere@redhat.com> <551458c7-bcb7-4643-91e4-9e01e0792425@simark.ca> From: Guinevere Larsen In-Reply-To: <551458c7-bcb7-4643-91e4-9e01e0792425@simark.ca> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: IyLGtEsDn1uxUYw-_2e5jpM0UfB5p0z7LFNupHFgIFs_1754424457 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 8/5/25 1:03 PM, Simon Marchi wrote: > 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. Fixed > >> 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". For consistency, I ended up changing all periods to commas, so this ended up being a non-issue > >> + * 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. For consistency, I actually changed this to just say "Legacy file format, this is (...)" Since the other options will all read "Main format on (...)". > >> + * 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")? I saw Eli's response, I'll change this to 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. done. This now reads as: enable support for selected file formats (default 'all') available formats: coff, dbx, elf, macho, mips, xcoff > >> +[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". done both > >> @@ -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. done > >> @@ -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. I don't know. I took the path of least changes to implementing this, made worse by me not having a convenient way to test a non-ELF target. I'll try to find a way to test if those are necessary and update the patch if they aren't > 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"? yes, you're correct. I'll update the name > I also wouldn't mind if you renamed the conf var gdb_cv_var_elf to > something like gdb_cv_var_bfd_elf. Sure, might as well. >> 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. done > >> 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? yes, because the case checks for ",$enable_binary_file_formats,", so even if there is only one option the commas will be there. > >> + *) >> + 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". Alright, fixed. > >> 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? sure, added a small comment like so:     Go through all requested and required binary file formats to compile     GDB, and double check that we can compile them. >> + 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). Sure, no problem. I don't really know or enjoy bash, so I go with whatever is easiest or I already know, so I wasn't sure this was viable. > > Simon > -- Cheers, Guinevere Larsen She/it