From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id +QveMqGp9WaLOTgAWB0awg (envelope-from ) for ; Thu, 26 Sep 2024 14:36:17 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=gnu.org header.i=@gnu.org header.a=rsa-sha256 header.s=fencepost-gnu-org header.b=CHpDquTx; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id AF87A1E353; Thu, 26 Sep 2024 14:36:17 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-10.1 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_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 5BF721E05C for ; Thu, 26 Sep 2024 14:36:16 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C76C43858C33 for ; Thu, 26 Sep 2024 18:36:15 +0000 (GMT) Received: from eggs.gnu.org (eggs.gnu.org [IPv6:2001:470:142:3::10]) by sourceware.org (Postfix) with ESMTPS id 1202A3858D28 for ; Thu, 26 Sep 2024 18:35:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1202A3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gnu.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gnu.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 1202A3858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:470:142:3::10 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727375754; cv=none; b=pxOTJLnIixeKqKqa1JPfYs7yxYeQAC1jiXl4DgViXH/ltlTj870gSJX6z9bX236vL+rBl5vm3+XxyYAg9evMA1as4cgazahYNm+mD896yValkZecG/Ir+NJVryI1fybotXRyk+55n4kpaAFI0zdztz+lZaCDSd1bUg6gcQ/PR+8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727375754; c=relaxed/simple; bh=iDMjl5zJ/GbCYzvB/6vfXISMpUG8laPhpRIX1+A8h3U=; h=DKIM-Signature:Date:Message-Id:From:To:Subject:MIME-version; b=czRHZA79Jm+Bpr6Xw7BaZyVflockEL69WTO5sNKfrrDU1wzkDPgPCe0mQ+r5ntncXWacJn9Fo0loCQtOIjNlYOSinBllaRFsOd6SrDkvxukAif+RQazJ6jrhgzQvGf0cbe4cs1styjw2c89yHAXHBnFPqyWycCOGRlwqkDTkdMw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sttLC-0006xF-1p; Thu, 26 Sep 2024 14:35:50 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=kGORSH6yJ9g0BC+8OfChe+rTUNytdKDoUrd4rIfrOIQ=; b=CHpDquTxtlbBR9Mmjq6t ulriIdNdnv2mu4ahV+yBdvia9iE4LPro051aff5YKAYv79aKpQJV5P76lPXiAGQGVPj/u0QCQbEyh Vg4pedSx0gTK/nqjcYPbUhp2Uurc8uCa/ZDIZQQiJwr1WHKwHQk35xnk8CDziuw1Gi2E8+kGNPWYs IUNlV6dd3AM7jq3SrEVr8LXQEaIuez24UBpk9msPyRrBuZqlbJRwkY22dQ4f0wxK+ys+5wfgMUcTY wOf4Psc4TfBEmJFM/IaMypZGVl/nZNI8CRxCZ9NEAm/6WiBDcW2P2D/ZDtiK5SwjMg9MXwjMzRGT/ PtrjItaEckK5hw==; Date: Thu, 26 Sep 2024 21:35:47 +0300 Message-Id: <865xqi9sak.fsf@gnu.org> From: Eli Zaretskii To: Guinevere Larsen Cc: gdb-patches@sourceware.org In-Reply-To: (message from Guinevere Larsen on Thu, 26 Sep 2024 15:16:34 -0300) Subject: Re: [PATCH] gdb, configure: Add disable-formats option for configure References: <20240925175340.1850969-1-guinevere@redhat.com> <86msjvars2.fsf@gnu.org> MIME-version: 1.0 Content-type: text/plain; charset=utf-8 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 > Date: Thu, 26 Sep 2024 15:16:34 -0300 > Cc: gdb-patches@sourceware.org > From: Guinevere Larsen > > >> +`--enable-formats=FORMAT,FORMAT,...' > >> +`--enable-formats=all` > > ^^^^^^^^^^^^^^^^^^^^^^ > > Please don't quote `like this`: it's a markdown-style quoting we don't > > use in our documentation. > > All other options in the README file are formatted like this. No, they use quoting `like this', not `like this`. > >> + *"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 > > This list seems to imply that only non-ELF targets should be in it > > (but then why linux-tdep.o is in the list?), because otherwise the > > list is way too short; there are a lot more *-tdep.o files that are > > built for various platforms, just look what "ls *-tdep.c" produces. > > If indeed this mentions only non-ELF targets, that should be mentioned > > in the comment. Also, this misses at least i386-go32-tdep.o (which > > needs coff). > > The list is meant to be "if these tdep files are included, compilation > will fail". I just tested locally and i386-go32-tdep.o compiles just > fine without coffread.c. I guess I should describe the list (and reasons > to add file format) more in terms of compilation failing than in terms > of what formats are available for a target. I thought this is about having in the built GDB support for the necessary binary file format, not about compilation failures. What good is a GDB if it cannot access the binary file format used by the target? > >> + # 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" ;; > > But the DJGPP (a.k.a. "go32") native target needs only coff, it > > doesn't need coff-pe. > Right, but I don't think it makes sense to have a separate option for > coff-pe when it requires coff to function. I think it would end up > making stuff pretty confusing to have only one format that has a > dependency, when all formats are not listed, and so there's no way to > annotate that one in specific. So what's the solution? forcing coff-pe to be compiled by the DJGPP port? > > '--enable-targets=[TARGET]...' > > '--enable-targets=all' > > Configure GDB for cross-debugging programs running on the specified > > list of targets. The special value 'all' configures GDB for > > debugging programs running on any target it supports. > > > > First, this doesn't say what is the default if --enable-targets is not > > specified; I think we should add that. More importantly, it says > > "cross-debugging", not "remote debugging", and my reading of > > configure.ac matches that: this option affects the value of > > TARGET_OBS, which is the list of target-specific files needed for > > debugging by GDB itself, without the help of gdbserver. So using the > > value of --enable-targets= for the purpose of deciding which readers > > will be compiled into GDB seems to be wrong, because when a target is > > debugged remotely via gdbserver, it is my understanding that the > > reader of the target's binary file format is needed in GDB itself, no? > > Yes, the client needs to understand the file format, but GDB already > does this partial format support only considering which targets are > requested. xcoff is only compiled in if rs6000-aix-tdep.o is compiled > in. Also, we only compile Mach-O and ELF support if BFD is has support > for those. My patch just makes it more controllable by the user, and > loudly warns of new files being added. > > And as I said at the beginning, GDB already can't properly work with > gdbservers if the target where the server is running is not compiled in, > I don't think this is making things worse in any significant way. My point was that the manual doesn't clarify these issues. It left me confused. > > Maybe I'm confused, but if I am, it means we lack some important > > information in the manual which would clarify this. > Maybe more clarity would be helpful, but I don't think these issues have > to do with my patch itself, so I think this should be further > improvement for documentation rather than having to fix it in this patch. You may be right, but without clarifying this whole issue I cannot decide whether your additions about this are okay or not. So I think we have no choice but to clarify those other aspects together with this review, even if just in principle. > > Reviewed-By: Eli Zaretskii > > > The  review tag is meant to be used if you are OK with the patch, maybe > with a few tweaks (like the obvious fixes you pointed out to me of > missing spaces and stuff). Which is what happened here. My main reservations are not about the documentation, but about the larger picture, where I don't have a decisive word anyway.