From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 06LKKsp1dWa3YQMAWB0awg (envelope-from ) for ; Fri, 21 Jun 2024 08:44:58 -0400 Received: by simark.ca (Postfix, from userid 112) id A39BB1E0C1; Fri, 21 Jun 2024 08:44:58 -0400 (EDT) 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 8E4081E030 for ; Fri, 21 Jun 2024 08:44:56 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EA7253898516 for ; Fri, 21 Jun 2024 12:44:55 +0000 (GMT) Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) by sourceware.org (Postfix) with ESMTPS id 579993898512 for ; Fri, 21 Jun 2024 12:44:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 579993898512 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 579993898512 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.221.45 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718973873; cv=none; b=jX+YA1WXyn5ZuT1jYdX/VHZd/sSUSHfdzokEMckITkt3lQNCLLoNb7W5Dx3n4OvjL59/537B0yjklKvQ5eueW8PZUIxksoycz8ZnIJeJGEYQYqZ2DbnBahhiFfSgn/I8IqeaaHwxVAcX5xK+NdY4VxDStAfxolR0EkC3Yhia0mQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718973873; c=relaxed/simple; bh=1FdCQ6TNmk4Dxv8vAGQq5RTT27FjwC/DiXKcXGIKmmc=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=tQxJNmKa9fYDkGMmCBRLHcMzY4ILkYuDEJn+Z+SiFDlz1hT7aPyUsT/M65RfcMr+FhvOZKAa106QBc6bZcJlUJIWF/qShvXa73i4Xn3K0hLcRlvJ4KkWA8bmCIXGYHqfH9Nz0iUFNRMgmeoAIUV8UfsY0fTbTosNUi9eV+Sksxc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wr1-f45.google.com with SMTP id ffacd0b85a97d-364b2f92388so1319642f8f.2 for ; Fri, 21 Jun 2024 05:44:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718973870; x=1719578670; h=content-transfer-encoding:in-reply-to:content-language:from :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=NMhhZytnMlHA4CQ/CZfxemBvYrTOeF2KlGQqZ7rmrwI=; b=u4OQhiW1WiLyAHXK/uR045prFQiNEhWTk1oYVABJ6797mswf3Ws9jqa9mVCt3GeVEK 8/qo6GzfovqYsNZCSQeYAFgW4aouhpr5mD0Fuk5JF+C+fMW1Nh6fEFBM+MKfx+/KMBuy LcaKzDJQW15Vzq68UK/u4wvV0SkjCgr9V+mT1cpwl/xIunFjRl87BLzY5qpDwfemBIWN wryDL9lSt6djeWrQiOKssnq/QXBJCdK59I7whgARoezXsNTtYWdRPuqjs9laHEtd/1W0 +osP0iFS0jjknM51G4Z7xFOsZg2gZcFMP4A29WJYgS7ddWjRMmQ+GaIxuwFLGeULQ+0C bGcg== X-Forwarded-Encrypted: i=1; AJvYcCVFndJtodrKV8Yv2mohh6Kvv0kPSNO14uN64aTiBFvE0frkVIosjdrNRbQvTpo6Am1rcYuZR6X0upLxqgqotAq1ZvMwqMlcV3xSZg== X-Gm-Message-State: AOJu0Ywpq7mc9lbCWFlIDrdc6+j1Tr7xxO3n6gJ8lIJ4+uKvj9JUNDLX y5aR6bNY6NmOd6fGxsagIET9kHW//VrFE5M0a0ytDezsG/aBoeBs X-Google-Smtp-Source: AGHT+IGyXUJtPf97iSuHB03qpSuSko2fsIugD7Hxoz/EgavJ4srVYxQxs1Ojhkh8+NwVwVNdLUUS6g== X-Received: by 2002:adf:e983:0:b0:35f:2cd8:cb31 with SMTP id ffacd0b85a97d-36317b82ec3mr5750798f8f.35.1718973869688; Fri, 21 Jun 2024 05:44:29 -0700 (PDT) Received: from ?IPV6:2001:8a0:f91f:4900:f671:19e2:418a:1f1a? ([2001:8a0:f91f:4900:f671:19e2:418a:1f1a]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-366383f65bcsm1652451f8f.13.2024.06.21.05.44.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 21 Jun 2024 05:44:29 -0700 (PDT) Message-ID: Date: Fri, 21 Jun 2024 13:44:25 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] [gdb/tdep] Fix gdb.base/watchpoint-running on {arm, ppc64le}-linux To: Tom de Vries , gdb-patches@sourceware.org References: <20240607063525.9887-1-tdevries@suse.de> <9c0d2050-3555-47c9-bec8-1f2548eba9c6@palves.net> <88e14e37-9fbd-4dfb-ac37-6bee23eff372@suse.de> From: Pedro Alves Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org 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 Hi Tom, On 2024-06-21 10:43, Tom de Vries wrote: > On 6/20/24 20:15, Pedro Alves wrote: >> On 2024-06-17 19:22, Tom de Vries wrote: >>> On 6/14/24 18:49, Pedro Alves wrote: >> >>>> And, from another angle, why isn't aarch64 doing the same, why two mechanisms? >>> >>> Well, the patch adds a fallback, that aarch64 doesn't need, but that powerpc and arm do need. >>> >> >> Aarch64 absolutely needs it, it's just that it already has the fix in place (by checking early in >> post_startup_inferior/post_attach).  We're adding code to the other backends to handle it too, but using a >> somewhat different solution.  If arm / ppc were being adjusted to use the same approach as aarch64 (like in a >> previous patch in bugzilla), then I wouldn't have asked that question.  I see no good reason for multiple ways >> of doing the same thing. >> > > To formulate it a bit differently: the fallback implementation enables only lazy implementations.  Aarch64 doesn't have a lazy implementation, so it doesn't need the fallback.  Both arm and ppc do have lazy implementations, which is why the fallback works for those. Yes, but with the fix, the arm and ppc backends aren't really lazy anymore, as in, the code makes it look like it, but they aren't, as we do the check always even with no watchpoints. The lazy-support support code could be removed in the line of Aarch64, as being unnecessary, which is what I was doing on ppc. > > If we'd drop the calls to aarch64_linux_get_debug_reg_capacity from both aarch64_linux_nat_target::post_startup_inferior and aarch64_linux_nat_target::post_attach, the fallback wouldn't help, we'd have to a add a call to aarch64_linux_get_debug_reg_capacity in probably an override of can_use_hw_breakpoint.  In which case I prefer the current solution for aarch64. > >>> There might be other targets that needs such a fallback, but that we don't know about. >> >> We have a testcase that will show us if so. >> > > To which my first thought is: And why not have a trivial fix that might work for some of those targets and make that test-case pass. > > I suppose we're weighing a trade-off between: > - reducing complexity by ensuring to do things in one way only, and > - catering for an unknown subset of target implementations in a common >   implementation. > > I don't see a good way or principle to decide between those, my preference is clearly on the latter but I do understand the importance of the former. I'm not against a fallback in principle. I am more against the fallback being to call a higher level function and assuming that it works because some other functions in the backend do things in a specific way (the lazy checking). It's crossing abstractions boundaries, calling into the target stack, assuming the right inferior is the current one, and overall more than what is needed. We have a hook at the low level to do low level things that only concern the architecture, and it seems to me way cleaner and future-design-proof to let it do the strict things that are specific to the architecture. That even allows getting rid of most of the now unnecessary lazy code as I was doing on ppc (ok, it needs some tweaks). Baking assumptions into the default kind of masks such things. > > Anyway, since this is a point of contention, I didn't include it in the tested patch, so ... moving on. Thank you. >>> So, this is what I have tested on x86_64-linux, aarch64-linux, arm-linux and ppc64le-linux. >> >> OK, let's go with this, then.  Thank for testing! > > I've submitted a v2 ( https://sourceware.org/pipermail/gdb-patches/2024-June/210138.html ), with comments a bit expanded, and commit log copied from v1 and updated. I will take a look now. Thanks again!