From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id /AvkGgReOmqntxQAWB0awg (envelope-from ) for ; Tue, 23 Jun 2026 06:20:52 -0400 Received: by simark.ca (Postfix, from userid 112) id 507AB1E024; Tue, 23 Jun 2026 06:20:52 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.3 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [IPv6:2620:52:6:3111::32]) (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 499731E024 for ; Tue, 23 Jun 2026 06:20:47 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 662154BA2E1A for ; Tue, 23 Jun 2026 10:20:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 662154BA2E1A Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) by sourceware.org (Postfix) with ESMTPS id F3A7D4BA5435 for ; Tue, 23 Jun 2026 10:20:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F3A7D4BA5435 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org F3A7D4BA5435 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=209.85.221.52 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1782210021; cv=none; b=QHoCqWJH1T9F7R47bbxNeUyWln8d3S3GdmsiVD5mtfGNnaZtZxVT71R2ErziwsN1vVMFqYOO+Rh5djW1C/Xjx9ikHPipP7B5C4SIYzJE4Z1xomUwtjY6vewNxmO5fG2zJfGYWWhHATuYRVKAOQ1SvWEgLUn6RBlxf1Sz9HTp/kY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1782210021; c=relaxed/simple; bh=xLN3C6Meh8iPnl8wpOSuCKMVYUrOySYc100bIoXn7aU=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=LXkSDhhy4cjY1H244B8nsGPSSwaUubO5Ep0f8vYqNMe99IX+zr1FYbuYGbaYuoSMeeV8vNCCmn1BmKNJKbFexpYCAzsGUuaFEpK6wNFSeY9xjMs4BYJLN1LtvzMth/oIcqGJLTndmi1sT3KMwKGmaXDi+J+Bocv/cD5D2T1VvmU= ARC-Authentication-Results: i=1; sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org F3A7D4BA5435 Received: by mail-wr1-f52.google.com with SMTP id ffacd0b85a97d-461edb387ddso1239535f8f.3 for ; Tue, 23 Jun 2026 03:20:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782210020; x=1782814820; h=content-transfer-encoding:in-reply-to:content-language:from :references:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=a+Qoh2fQvBw63P9tO43c0vQhNhPaZ5FCxhvVBh6BS7A=; b=nVGig79P1YfAct3HKL68C2TKOigyiyHZjoT3EB8jdad1kWoUryv2C8cEAVJq1/0ovm r6FyZK6FWm9XiK7JR9CmY0gSm5HLjgiNvYQFwgdSiFnn+TVpgDQrpQx6h3IzcshhcOgL MkR/UL8IMmC0nrJfTCdR6WHOTjvCjI4Fxm7kNIADHUsSF5qAjhTNxWln5MNkavdFIV+N sA0qDemOQ0+ubind12f+z6z1XUnXGBhR9UHcfOof2uOdB8Itgk1dZYKbmqq55zvHsioT uicY1m5wr0AyzCVI4pSukDQZpqa387jgMS9QWe4l/lNDptdteCgkigjGVTCKOqj4P6tx ipog== X-Forwarded-Encrypted: i=1; AFNElJ844y/+bUKrmI8q1Ei9VMRK7SlGIgsSqAFDC6xfGQheJXD5slmG+PJHCOLAoF0qRaSwJH74uJzLC06dEQ==@sourceware.org X-Gm-Message-State: AOJu0Yz4MqZRAqRyCOEKE8pk/zwngKEGBK1KofIJ/mwFgywAd9B2MP1h n/+gjpYhgVLZj/Zcm1Rm1TUrA+F69f8PmBosCR04YN9reklkKs2ZC7p8iZ8i/g== X-Gm-Gg: AfdE7cl9o4/8GXeLCRkiIbIVvmq8natwLt24PrCDIgSNQU/5lB7uIJws2oOLzCEfALd ZAzL8aM21hXvQkLAl0/smld9O+VSvcKphFJkJJdVMHHL00Ij39MjzBAkenunkGwy/A0YZq+xYd3 5gQBD6/RNaSKuAJVh9QWFBQMIPy92aGSgqnSsvaqt5npzDI2fGXqyZh2qH0ZgjRvM869HYLM6gU dVxVAIeAD/Od8L6Ja4dRIzvPD0ehu8BdwV/QcoYHC4YYer4Vv9gPyRC7MMzeN/ld9iXPhRcoEAs VgpjDISQkVoSQuag3WjB3O9KppmCDOzr0nuYJKDOdtzet0v0c0+1n/COP60G6F1BTesJ9eiSk2Z L9A0dL+xP7MRfQDTeur3+fk/huIqoKddtGhGJekTf1YNIsr/dF5HRgxDBM+UJ7DWxjGe27K8ALv HnlV+rn/FHphcgOrPY8Pe9xr9KrAefHytFiPsvmyzquwGWNt/755a9n666cfsc4EDBVA== X-Received: by 2002:a05:600d:4448:20b0:490:da12:f1fa with SMTP id 5b1f17b1804b1-49240ea7f99mr247693715e9.31.1782210019441; Tue, 23 Jun 2026 03:20:19 -0700 (PDT) Received: from ?IPV6:2001:8a0:fae3:2600:c29d:3566:808b:5f79? ([2001:8a0:fae3:2600:c29d:3566:808b:5f79]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-46666c579d4sm35660327f8f.28.2026.06.23.03.20.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 23 Jun 2026 03:20:19 -0700 (PDT) Message-ID: <34d263fd-be8b-463d-b297-8d73ef0cf0f0@palves.net> Date: Tue, 23 Jun 2026 11:20:09 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCHv2 2/3] gdb: introduce program_space::get_entry_point_info function To: Andrew Burgess , gdb-patches@sourceware.org References: <41fe591d58ba010fa771e80ca674b61e30feef2f.1780942441.git.aburgess@redhat.com> <864cefb52d208dd8aac6b1b5f452cadad7546af8.1781214731.git.aburgess@redhat.com> <87fr2op04p.fsf@redhat.com> <87zf0loahg.fsf@redhat.com> From: Pedro Alves Content-Language: en-US In-Reply-To: <87zf0loahg.fsf@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 2026-06-23 10:46, Andrew Burgess wrote: > Pedro Alves writes: > >> Hi! >> >> On 2026-06-15 11:29, Andrew Burgess wrote: >>> Pedro Alves writes: >>> >>>> On 2026-06-11 22:59, Andrew Burgess wrote: >>>>> + # Only svr4 targets currently support querying the inferior entry >>>>> + # address. >>>>> + if {[istarget *-linux*]} { >>>> >>>> There are more svr4 targets than linux. I think this style of allow-list has >>>> a good chance of never getting updated. Deny-lists are better, IMHO. If the >>>> test fails on some port, that might trigger someone to add the feature >>>> there. >>> >>> Hey Pedro, >>> >>> I'm still looking at your other feedback, but before I start making this >>> specific change, I just wanted to clarify how you see this as being >>> different from what I have right now. >>> >>> If I write this as a deny list, e.g. >>> >>> if { ![istarget ....] && ![istarget ....] } { >>> # Run tests. >>> } >>> >>> Is there not the same problem? We rely on someone realising that the >>> reason the test isn't run on their target is some missing GDB >>> functionality, and them adding the functionality and removing the >>> `istarget` block for their target. >> >> >> Speaking from principles, and not this particular case: >> >> The difference is that there was a failure that made someone see that >> some functionality is missing. With the allow-list, nobody ever notices it. >> >> With a target-based allow-list, even if someone adds some functionality >> to a port, it's typical to not comb through the testsuite and >> relax the relevant allow-lists to also allow their targets. >> >> And even if they want to, there's no easy marker to grep for. E.g, say I >> implement feature X on Windows, then how do I know that I need to >> grep for "istarget Y" to find all the tests that I need to adjust? >> And which Y? And then which ones that hit my grep should I look at? >> >> I'll give you one example, in gdb.threads/watchpoint-fork.exp: >> >> # Only GNU/Linux is known to support `set follow-fork-mode child'. >> if {[istarget "*-*-linux*"]} { >> test child FOLLOW_CHILD >> } else { >> untested "${testfile}: child" >> } >> >> I think at least FreeBSD has supported that for over a decade. > > Sure, but how would rewriting this as you're suggesting have helped in > any way? From what you suggest below I'm imagining your rewrite of this > would have looked like this: > > if { [supports_fork_follow_child] } { > test child FOLLOW_CHILD > } else { > untested "${testfile}: child" > } > > with: > > proc supports_fork_follow_child {} { > if { [istarget *-freebsd*] } { > # Not supported here. I assume at the point the test was first > # added this feature wasn't supported on FreeBSD. > return 0 > } > > # Assume true by default. > return 1 > } > > Now, I agree that this is better as fixing `supports_fork_follow_child` > means we only need to update one proc and all tests that used the proc > would then start testing fork follow child behaviour. But, as you say, > it's unlikely that when this feature was fixed on FreeBSD the support > proc would actually be updated, so I fail to see how this is > significantly different. Well, with the proc, you effectively turned: if {[istarget "*-*-linux*"]} { test child FOLLOW_CHILD } else { untested "${testfile}: child" } into: if {![istarget "*-*-freebsd*"]} { test child FOLLOW_CHILD } else { untested "${testfile}: child" } ... meaning, any new target that comes along, after the testcase is written, automatically gets that test exercised. While the "before" is stuck in testing only on linux and nobody ever remembers to update it. That's the main point about allow vs deny lists. Also, the wrapper proc is a lot more discoverable, especially if its in gdb.exp. You only have to discover it once, and from there you can easily find all the testcases that use it. With explicit "istarget", you have the issue I mentioned earlier: no clue what to look for. Even assuming you ran into that one in gdb.threads/watchpoint-fork.exp one, which other testcases have a similar issue? Maybe there are others using "istarget linux"? Or some other slightly different if condition? > > And the example you give is fundamentally different than my code. What > I wrote is this: > > if { [is_svr4_target] } { > set expected_result "PATTERN WHEN SUPPORTED" > } else { > set expected_result "PATTERN WHEN NOT SUPPORTED" > } > > gdb_test "some command" $expected_result OK, I missed that. That's even better, and takes care of my main concern -- discoverability. > > The difference here is that if a non-svr4 target is fixed then it will > stop emitting "PATTERN WHEN NOT SUPPORTED" and will start emitting > "PATTERN WHEN SUPPORTED". If the developer actually runs the complete > testsuite then they will see a PASS -> FAIL for this test, which will > force them to update the `if` condition (but see below). Agreed, and that's the ideal. Thanks. Pedro Alves > I do agree with you that having a 'supports_....' proc will be better > than having to update the `if` condition within the test itself. If the > supports proc ends up being used multiple times then we only need to > update the one place and all the tests will be fixed. > > I'll follow the structure you propose here as I'd like to progress this > patch. I'll post a v4 with the update soon.