From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id XnzBLCrM9WYxWDgAWB0awg (envelope-from ) for ; Thu, 26 Sep 2024 17:03:38 -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=K4QJCsv7; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 95E5A1E353; Thu, 26 Sep 2024 17:03:38 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-12.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_HI,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 9E8BE1E05C for ; Thu, 26 Sep 2024 17:03:34 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8759F3858C50 for ; Thu, 26 Sep 2024 21:03:33 +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 D233E3858D28 for ; Thu, 26 Sep 2024 21:03:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D233E3858D28 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 D233E3858D28 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=1727384592; cv=none; b=JQTdRusaSvXTQ7BRMzU3yOoU5EcLgxwJyHOJArWU8zr+uWjVmyUy++2XUJRLHXBlkBQsBG4jhQMPu3Pkz22Aw6Reuf0YW0NeTALlVisFDG5UkfZIJxNf0Ln2ih8NaKsjytx2rvCE1yZvNyrY4uQr2hz14BXFk1odfhshgHqipWY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727384592; c=relaxed/simple; bh=S9tASFTF3RJxUaFZE++gfkz4pHcI9e1pQFW1jfTtgio=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=TScFYfTeuV76Nqp1yTs3laonaS38EVBATjC4+mc+JnVv3WpCua6APznc9wob7sD0u/tBLEI77nnN/UvTC0sb4ElptzINqUPrCv2lqydNVkzDmyBAUilvn+gSYSklqd8JddNgP3nKWMiOkHVCRMeEfPgXqpvAlNomt/+tgbRRvFs= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1727384590; 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=OdCtgvM4NLl0hZ9Ax2FwP4PSb55GO+dcieZNy4xm8jw=; b=K4QJCsv72fi0tSVcc755+GXNBXhBd6HAGRcmBPZlQ4gbgY30QRfQOz8z8mFEAU3HkBi8E1 heBota8mImn634BjVMh10+5+iW4wjMRhCcH5EpW2pJhiu0yGJViJJ0dg5h+twwHZ39Ry1M 4DchrG3y5lSklwC+U/E/otbN46ENvq8= Received: from mail-pf1-f197.google.com (mail-pf1-f197.google.com [209.85.210.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-197-ASFD8leRPfGj7JSKQS_0bQ-1; Thu, 26 Sep 2024 17:03:09 -0400 X-MC-Unique: ASFD8leRPfGj7JSKQS_0bQ-1 Received: by mail-pf1-f197.google.com with SMTP id d2e1a72fcca58-71985781cbdso1718435b3a.3 for ; Thu, 26 Sep 2024 14:03:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727384588; x=1727989388; h=content-transfer-encoding:in-reply-to:from:content-language :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=OdCtgvM4NLl0hZ9Ax2FwP4PSb55GO+dcieZNy4xm8jw=; b=AKPMW6J8YI8gEsOPko74o1zweR8qYoBDbyVQd2nBi+ic9J8F/ny0jWkiENUPQKD5Du 3wacJMH9PorkZDeikQnxEzMsgNTJ72OfrjRN7n8oCvFWOmRUyyK17zjTvIcn1OtkgFLF 0vQfbP87qPWHYWzAAvhd8jiD2uqS4zg+6laf7s0aungZjBZuk5+wmE0vGzKoPEBOH5YK PlNfN1VtzJYwNYeO5IkjIIo7IeT9B4BsgHrXtF5POxlI0urr52EHCXq+TcDnGo0BIieP nVgcXzBWQ7fmX3geXTw3EfnfLasB8nePPgymmALFstbg4oBfb03gWjCXmC69DgVIaEs6 +W6A== X-Gm-Message-State: AOJu0Yx6xjJ0LFIQsRrwRPRvhieKkk4pUIPkzG9T9SAYQxRMRbFO+OMP mfTtTkc+p/OoSqwXLnfbLkdTykvhO/Ali3iVNS5SEas7zDSMbl81oeR3cQ7/xOwN8n8d6ZggQiH COJIGX0Ezp8i8+Y9BuAt6sdvtImxgd6EJ9p1EjlqQKd/+p1smzdEc0BryxRE= X-Received: by 2002:a05:6a21:139b:b0:1cf:47b3:cbbe with SMTP id adf61e73a8af0-1d4fa7ae130mr1094187637.45.1727384588122; Thu, 26 Sep 2024 14:03:08 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFNTNh1BeDtLNi/p2+LRPRU9LKoIJ5MpzD+2j5kxHNCd+UhuHmrX54Y2AC+ZylAuMRKQiuv9g== X-Received: by 2002:a05:6a21:139b:b0:1cf:47b3:cbbe with SMTP id adf61e73a8af0-1d4fa7ae130mr1094156637.45.1727384587714; Thu, 26 Sep 2024 14:03:07 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:92c5::1000? ([2804:14d:8084:92c5::1000]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-71b2653665csm325070b3a.185.2024.09.26.14.03.06 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 26 Sep 2024 14:03:07 -0700 (PDT) Message-ID: Date: Thu, 26 Sep 2024 18:03:05 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] gdb, configure: Add disable-formats option for configure To: Eli Zaretskii Cc: gdb-patches@sourceware.org References: <20240925175340.1850969-1-guinevere@redhat.com> <86msjvars2.fsf@gnu.org> <865xqi9sak.fsf@gnu.org> From: Guinevere Larsen In-Reply-To: <865xqi9sak.fsf@gnu.org> X-Mimecast-Spam-Score: 0 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 9/26/24 3:35 PM, Eli Zaretskii wrote: >> 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`. Oh sorry, I missed that, I'll fix the formatting. And I guess we have to fix --enable-tagets=all at some point. > >>>> + *"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. I did describe it from the point of view of functionality, but the technical reason for it was compilation problems. I will change the explanation on the next version to be more straight forward. > What > good is a GDB if it cannot access the binary file format used by the > target? The GDB may have been built primarily for remote debugging a different target... And even locally, like I said, you can still debug as an unstructured collection of bytes, setting breaks on addresses, pausing and continuing, so it's not like it is useless. Plus, this will only affect users that actively decide to not compile support, if they ignore the flag everything will work as it used to. When chatting on IRC, Simon mentioned that ideally we'd be able to not compile any formats, and give the user full control. If they wish to shoot their own foot off, who are we to know better. Most users will probably just accept whatever their distribution gives them, and I would guess most people building their own wouldn't mess with this flag, I think this is pretty low impact. > >>>> + # 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? That would be my suggestion, yes. If you have a good suggestion for a way to have coff-pe be a switch, that is obviously dependent on coff and still obviously an objfile format, I'm all ears! Also, worthy of note, this is still an improvement for a security conscious DJGPP user, since currently we compile coff-pe in along with every other file format reader, so they'll still get a bit of unnecessary cruft, but not nearly as much as before this change. > >>> '--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. I hope I clarified enough in this email to allow you to judge my changes on their own, and a later unrelated patch can fill in the missing information, since the changes really aren't related to enable-targets and it was my mistake to parse them there instead of just seeing which tdep files are compiled in and need a file format. > >>> 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. > -- Cheers, Guinevere Larsen She/Her/Hers