From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id hcCBJTlLdWbfEwMAWB0awg (envelope-from ) for ; Fri, 21 Jun 2024 05:43:21 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=2TgUgkHM; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=y+/sk0gn; dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=2TgUgkHM; dkim=neutral header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=y+/sk0gn; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 8C2E61E0C1; Fri, 21 Jun 2024 05:43:21 -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 6F94E1E030 for ; Fri, 21 Jun 2024 05:43:19 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EB21538983B9 for ; Fri, 21 Jun 2024 09:43:18 +0000 (GMT) Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by sourceware.org (Postfix) with ESMTPS id C3FC13896C3A for ; Fri, 21 Jun 2024 09:42:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C3FC13896C3A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C3FC13896C3A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.135.223.131 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718962971; cv=none; b=h8ve21Ex3zMV/4KP8J7yweXu6HJGVAHX09OXe6R7oBBckQYfQfFIxRj8TRXWhC5e4REbksmbEqdMCIFNw60rvBil9GZN3Hc2xk7+3YB63FCgI1VVyVTw1HqrNtT4iZy752x24n320035QPGPvaODLqlOb+Nac2cWCQO/qVdoIAY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718962971; c=relaxed/simple; bh=cvp3e3uDbjB2XmIjyO5C4QR9bF1q5X45/yoa16KLOV0=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature: Message-ID:Date:MIME-Version:Subject:To:From; b=NqCdZu7IXjzKHWlTZMm5UudbI11Yywcetj4+wojC40tJB5wqI3PvxizXUBB9au5AoF6vmK40iw1AIIe3ivYTA3wlkxrImkJqOCUb9i/lEIYjqMG0NAIvnfyZkgNtVDF1wj9BrsWgACMPJ8PYIrwrgTI22oILJmzH+BaUsINtq4o= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id A24D31FB5B; Fri, 21 Jun 2024 09:42:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1718962968; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ibYQoIrodVkBWVJIWX35h6yLgsVJZXG9oWyWe/L3VLk=; b=2TgUgkHMFp2750g+DpZ42ikQJUYuvH1F0u1jHkTWz66+HHtilDLNz/KflQagkEqbGnXzUA feo5p2EbQCaSqJJeAQVGDoxUGxaxJuHxbkXIvW98Kv80unh2PgO5rvjP0iGR+qp3Albu6k JbHtwNeKpy2qOlW63/Vy14Fz7Jca6Ck= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1718962968; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ibYQoIrodVkBWVJIWX35h6yLgsVJZXG9oWyWe/L3VLk=; b=y+/sk0gn2iE372vo+qEfDqqRpvgP9Hr8xBuZbUFTQ+2QyYtoSLXGk6FD9PcSNT9MXFicxG mZTLMYw2RbRpxACg== Authentication-Results: smtp-out2.suse.de; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=2TgUgkHM; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b="y+/sk0gn" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1718962968; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ibYQoIrodVkBWVJIWX35h6yLgsVJZXG9oWyWe/L3VLk=; b=2TgUgkHMFp2750g+DpZ42ikQJUYuvH1F0u1jHkTWz66+HHtilDLNz/KflQagkEqbGnXzUA feo5p2EbQCaSqJJeAQVGDoxUGxaxJuHxbkXIvW98Kv80unh2PgO5rvjP0iGR+qp3Albu6k JbHtwNeKpy2qOlW63/Vy14Fz7Jca6Ck= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1718962968; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ibYQoIrodVkBWVJIWX35h6yLgsVJZXG9oWyWe/L3VLk=; b=y+/sk0gn2iE372vo+qEfDqqRpvgP9Hr8xBuZbUFTQ+2QyYtoSLXGk6FD9PcSNT9MXFicxG mZTLMYw2RbRpxACg== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 8891C13AAA; Fri, 21 Jun 2024 09:42:48 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id qhb2HxhLdWa1RAAAD6G6ig (envelope-from ); Fri, 21 Jun 2024 09:42:48 +0000 Message-ID: Date: Fri, 21 Jun 2024 11:43:52 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] [gdb/tdep] Fix gdb.base/watchpoint-running on {arm, ppc64le}-linux To: Pedro Alves , 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> Content-Language: en-US From: Tom de Vries In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spamd-Result: default: False [-6.50 / 50.00]; BAYES_HAM(-3.00)[100.00%]; DWL_DNSWL_MED(-2.00)[suse.de:dkim]; NEURAL_HAM_LONG(-1.00)[-1.000]; R_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-0.20)[-1.000]; MIME_GOOD(-0.10)[text/plain]; XM_UA_NO_VERSION(0.01)[]; MX_GOOD(-0.01)[]; RCVD_TLS_ALL(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; FUZZY_BLOCKED(0.00)[rspamd.com]; MIME_TRACE(0.00)[0:+]; MID_RHS_MATCH_FROM(0.00)[]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; TO_DN_SOME(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_TWO(0.00)[2]; RCVD_COUNT_TWO(0.00)[2]; TO_MATCH_ENVRCPT_ALL(0.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:dkim]; DNSWL_BLOCKED(0.00)[2a07:de40:b281:104:10:150:64:97:from,2a07:de40:b281:106:10:150:64:167:received]; DKIM_TRACE(0.00)[suse.de:+] X-Rspamd-Action: no action X-Rspamd-Server: rspamd2.dmz-prg2.suse.org X-Rspamd-Queue-Id: A24D31FB5B X-Spam-Score: -6.50 X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham 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 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. 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. Anyway, since this is a point of contention, I didn't include it in the tested patch, so ... moving on. >> So, I don't mind your patch, it's certainly cleaner, but I don't mind a functional default implementation either. >> So, I'd move the target_can_use_hardware_watchpoint call to the default implementation of low_init_process. > > I'd rather not. Let low level handle low level things using low level details. > >> I ran into trouble building the patch due to type of pid parameter mismatches. >> >> After fixing that, I ran into trouble on ppc64le, because low_prepare_to_resume is called before low_init_process. > > I guess it's while running through the shell, now that I think about it. > >> I fixed that by sinking this code in the function a bit: >> ... >>   if (m_dreg_interface.unavailable_p ()) >>     return; >> ... > > Makes sense. > >> >> And after fixing this, I still ran into failures and identified at least two more locations that needed fixing due to the cleanup, at which point I decided that the cleanup part is out-of-scope for the patch fixing the PR. > > OK. > >> >> 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. Thanks, - Tom