From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id NUWbJ7N0bGaW1jUAWB0awg (envelope-from ) for ; Fri, 14 Jun 2024 12:49:55 -0400 Received: by simark.ca (Postfix, from userid 112) id 96C931E0C1; Fri, 14 Jun 2024 12:49:55 -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 77C741E092 for ; Fri, 14 Jun 2024 12:49:53 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 069F83882126 for ; Fri, 14 Jun 2024 16:49:53 +0000 (GMT) Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) by sourceware.org (Postfix) with ESMTPS id BD2EC3882165 for ; Fri, 14 Jun 2024 16:49:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BD2EC3882165 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 BD2EC3882165 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.221.46 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718383771; cv=none; b=K8a6JPT8kVEgweUONvKNgOH4ZMYmAJaSYvIILRzlcpT5sG9q1YKeQUlmKx0ika5R3G/2HE0B6Do8K/rK7UuUTFdrH4M6iA5zUzNWskRWD6xfyPpQqRkeUWQIrYVa4vQS751Hoi9Ir/WA+vgBJdvtaQN+ViXAoOtR/qSuRcuMZ4k= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718383771; c=relaxed/simple; bh=Y40Ga5wi1uW8H8wcdBHCaFQqtUGTuAq83g2xarBCJOA=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=oABXbZpC5Rnm/pX13/KuR8n9uNKxbdD0LgoLlWgSUJdFW/G/WLIT2+n7m7dmbdznkxxULQzwpJTVtV+HZNNOWiXZYdAtV8Tcy+1htfXDo7jBPSRgxCxQxs8j2zriD3zQWTXpWbIJOnfviLoTyead/vYJVifMBC6kqvLVBza+jm4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wr1-f46.google.com with SMTP id ffacd0b85a97d-35f1c209893so2654625f8f.2 for ; Fri, 14 Jun 2024 09:49:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718383766; x=1718988566; 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=UrNkbShHKawrpYgspH9q1K6lcyAOKI0t/34uq6NkX7I=; b=M9VKDjm8bwEs7zeoFVZ84LXnVeRHeX7+6e9Me7a+9EfwQy82lJHLSvJzd8I2X21lzY k2OvhGJyRwSVfkU9H1eGgW+Vb7uOsPDxA3C8esYMjcchxIcR2nCrVUiTBqr00gYgGv52 mmVfqcxDIrCRoY2XaIu0cXuAgEU381RUb3wIUI0iUrvUl9AAqjPiK/TUz2P73SI3AK/g lxmO02ssulRspXiGHAtey+s3NHdeKi4kxh3xrkCTVUgJS9wZwkL7WGJ8L3EHrGcvis1E Quyu9/lrvdZz3Og1wrb7OH7gdVXcPm+YSSHG16r4KlXklKO6YhkvzHF+HQs8UEH2hL+Y akTg== X-Forwarded-Encrypted: i=1; AJvYcCXaJKLTa695T7K8aDJo5Q/HsaVoS6+nMYHF1y17ypnH3wLfPiUoOsB8yePiu0LEqvjhxhtb0/wP8kC34ZPE/2xRDjwMBGP5f1/xaw== X-Gm-Message-State: AOJu0YwuFoPctC4ksF3DAI8m1KwOAO8gO4ZesUtTENQVCqw+NIkZTOZm wHAqHSa7ye8gpuFbcS5LosgzAQ1CdErtJ3biHmW+kbHAGML1FuEB X-Google-Smtp-Source: AGHT+IGmMLh0XpxBvtXoPIOEWSI7NF0ayVSs0hXko/7hQBCN4V6IYk2cqQtf5v4iLOmbb6edKmqEcw== X-Received: by 2002:a5d:4009:0:b0:360:7c23:89e2 with SMTP id ffacd0b85a97d-3607c238a95mr2392239f8f.6.1718383766156; Fri, 14 Jun 2024 09:49:26 -0700 (PDT) Received: from ?IPV6:2001:8a0:f907:4900:3943:9a6c:1c8:1686? ([2001:8a0:f907:4900:3943:9a6c:1c8:1686]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-3607509c8fbsm4892598f8f.43.2024.06.14.09.49.25 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 14 Jun 2024 09:49:25 -0700 (PDT) Message-ID: <9c0d2050-3555-47c9-bec8-1f2548eba9c6@palves.net> Date: Fri, 14 Jun 2024 17:49:16 +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> From: Pedro Alves Content-Language: en-US In-Reply-To: <20240607063525.9887-1-tdevries@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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 Hi! Sorry for the delay. I've been super swamped. :-/ On 2024-06-07 07:35, Tom de Vries wrote: > PR tdep/31834 > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31834 > PR tdep/31705 > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31705 > --- > gdb/linux-nat.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > index c95d420d416..d8b5a99269b 100644 > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c > @@ -454,6 +454,18 @@ linux_init_ptrace_procfs (pid_t pid, int attached) > linux_ptrace_init_warnings (); > linux_proc_init_warnings (); > proc_mem_file_is_writable (); > + > + /* Some targets (for instance ppc and arm) may call ptrace to answer a > + target_can_use_hardware_watchpoint query, and cache the result. However, > + the ptrace call will fail with errno ESRCH if the tracee is not > + ptrace-stopped, making the query fail. And if the caching mechanism does > + not disregard an ESRCH result, all subsequent queries will also fail. > + Call it now, where we known the tracee is ptrace-stopped. > + > + Other targets (for instance aarch64) do the relevant ptrace call and > + caching in their implementation of post_attach and post_startup_inferior, > + in which case this call is expected to have no effect. */ > + target_can_use_hardware_watchpoint (bp_hardware_watchpoint, 1, 0); To be honest, I kind of preferred the other version of the patch. This is a single call, yes, but then you have to explain details about the different backend implementations, anyhow, and it raises questions like, what if bp_hardware_watchpoint is the right type? What if some architecture caches the resources for bp_hardware_breakpoint differently? And, from another angle, why isn't aarch64 doing the same, why two mechanisms? I guess the wart with the other approach would be that you have to handle this from both post_startup_inferior and post_attach? I think we can fix that -- add a new low_init_process method that is called in both scenarios, where the backend can do what it needs to. I was going to write small draft patch that just adds the method in question, for discussion, but then as I was already looking at the code, I ended up implementing the arm, aarch64, ppc backend versions of it. I noticed that all the m_dreg_interface.detect and m_dreg_interface.detected_p calls throughout ppc-linux-nat.c could be removed, since we now always call m_dreg_interface.detect() early. I only build-tested this on x86_64, which of course is not sufficient testing. Overall it's a net reduction of code, which seems nice to me. Pedro Alves >From e4e232efc509f56ae9d183d69c77d8d3f3024452 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 14 Jun 2024 17:15:43 +0100 Subject: [PATCH] foo Change-Id: I25bc362da9e9c6ca3ad3a81395b5d442cba5febf --- gdb/aarch64-linux-nat.c | 30 +++++------------------ gdb/arm-linux-nat.c | 7 ++++++ gdb/linux-nat.c | 6 +++++ gdb/linux-nat.h | 6 +++++ gdb/ppc-linux-nat.c | 54 +++++++++++++---------------------------- 5 files changed, 42 insertions(+), 61 deletions(-) diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c index 4b2a0ba9f7b..c4f9f6c2f48 100644 --- a/gdb/aarch64-linux-nat.c +++ b/gdb/aarch64-linux-nat.c @@ -78,12 +78,6 @@ class aarch64_linux_nat_target final int can_do_single_step () override; - /* Override the GNU/Linux inferior startup hook. */ - void post_startup_inferior (ptid_t) override; - - /* Override the GNU/Linux post attach hook. */ - void post_attach (int pid) override; - /* These three defer to common nat/ code. */ void low_new_thread (struct lwp_info *lp) override { aarch64_linux_new_thread (lp); } @@ -93,6 +87,7 @@ class aarch64_linux_nat_target final { aarch64_linux_prepare_to_resume (lp); } void low_new_fork (struct lwp_info *parent, pid_t child_pid) override; + void low_init_process (ptid_t pid) override; void low_forget_process (pid_t pid) override; /* Add our siginfo layout converter. */ @@ -844,29 +839,16 @@ ps_get_thread_area (struct ps_prochandle *ph, } -/* Implement the virtual inf_ptrace_target::post_startup_inferior method. */ - -void -aarch64_linux_nat_target::post_startup_inferior (ptid_t ptid) -{ - low_forget_process (ptid.pid ()); - aarch64_linux_get_debug_reg_capacity (ptid.pid ()); - linux_nat_target::post_startup_inferior (ptid); -} - -/* Implement the "post_attach" target_ops method. */ +/* Implement the "low_init_process" target_ops method. */ void -aarch64_linux_nat_target::post_attach (int pid) +aarch64_linux_nat_target::low_init_process (int pid) { low_forget_process (pid); - /* Set the hardware debug register capacity. If - aarch64_linux_get_debug_reg_capacity is not called - (as it is in aarch64_linux_child_post_startup_inferior) then - software watchpoints will be used instead of hardware - watchpoints when attaching to a target. */ + /* Set the hardware debug register capacity. If this is not called + then software watchpoints will be used instead of hardware + watchpoints. */ aarch64_linux_get_debug_reg_capacity (pid); - linux_nat_target::post_attach (pid); } /* Implement the "read_description" target_ops method. */ diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c index 50c24ecfcd2..531d555474a 100644 --- a/gdb/arm-linux-nat.c +++ b/gdb/arm-linux-nat.c @@ -103,6 +103,7 @@ class arm_linux_nat_target final : public linux_nat_target /* Handle process creation and exit. */ void low_new_fork (struct lwp_info *parent, pid_t child_pid) override; + void low_init_process (pid_t pid) override; void low_forget_process (pid_t pid) override; }; @@ -805,6 +806,12 @@ arm_linux_process_info_get (pid_t pid) return proc; } +void +arm_linux_nat_target::low_init_process (pid_t pid) +{ + arm_linux_get_hwbp_cap (); +} + /* Called whenever GDB is no longer debugging process PID. It deletes data structures that keep track of debug register state. */ diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index c0fe08a2a8b..3f252370c7b 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -454,6 +454,12 @@ linux_init_ptrace_procfs (pid_t pid, int attached) linux_ptrace_init_warnings (); linux_proc_init_warnings (); proc_mem_file_is_writable (); + + /* Let the arch-specific native code do any needed initialization. + Some architectures need to call ptrace to check for hardware + watchpoints support, etc. Call it now, when we know the tracee + is ptrace-stopped. */ + linux_target->low_init_process (pid); } linux_nat_target::~linux_nat_target () diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h index f30a5f90e2a..ee8603743f6 100644 --- a/gdb/linux-nat.h +++ b/gdb/linux-nat.h @@ -164,6 +164,12 @@ class linux_nat_target : public inf_ptrace_target virtual void low_new_clone (struct lwp_info *parent, pid_t child_lwp) {} + /* The method to call, if any, when we have a new (from run/attach, + not fork) process to debug. The process is ptrace-stopped when + this is called. */ + virtual void low_init_process (pid_t pid) + {} + /* The method to call, if any, when a process is no longer attached. */ virtual void low_forget_process (pid_t pid) diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c index c73c7c90b4c..7ff12e19db3 100644 --- a/gdb/ppc-linux-nat.c +++ b/gdb/ppc-linux-nat.c @@ -370,19 +370,17 @@ class ppc_linux_dreg_interface return m_interface.has_value (); } - /* Detect the available interface, if any, if it hasn't been detected - before, using PTID for the necessary ptrace calls. */ + /* Detect the available interface, if any, if it hasn't been + detected before, using PID for the necessary ptrace calls. */ - void detect (const ptid_t &ptid) + void detect (const pid_t pid) { if (m_interface.has_value ()) return; - gdb_assert (ptid.lwp_p ()); - bool no_features = false; - if (ptrace (PPC_PTRACE_GETHWDBGINFO, ptid.lwp (), 0, &m_hwdebug_info) + if (ptrace (PPC_PTRACE_GETHWDBGINFO, pid, 0, &m_hwdebug_info) >= 0) { /* If there are no advertised features, we don't use the @@ -429,7 +427,7 @@ class ppc_linux_dreg_interface { unsigned long wp; - if (ptrace (PTRACE_GET_DEBUGREG, ptid.lwp (), 0, &wp) >= 0) + if (ptrace (PTRACE_GET_DEBUGREG, pid, 0, &wp) >= 0) { m_interface.emplace (DEBUGREG); return; @@ -2046,8 +2044,6 @@ ppc_linux_nat_target::can_use_hw_breakpoint (enum bptype type, int cnt, { int total_hw_wp, total_hw_bp; - m_dreg_interface.detect (inferior_ptid); - if (m_dreg_interface.unavailable_p ()) return 0; @@ -2101,8 +2097,6 @@ ppc_linux_nat_target::region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) if (len <= 0) return 0; - m_dreg_interface.detect (inferior_ptid); - if (m_dreg_interface.unavailable_p ()) return 0; @@ -2180,8 +2174,6 @@ ppc_linux_nat_target::hwdebug_point_cmp (const struct ppc_hw_breakpoint &a, int ppc_linux_nat_target::ranged_break_num_registers () { - m_dreg_interface.detect (inferior_ptid); - return ((m_dreg_interface.hwdebug_p () && (m_dreg_interface.hwdebug_info ().features & PPC_DEBUG_FEATURE_INSN_BP_RANGE))? @@ -2199,8 +2191,6 @@ ppc_linux_nat_target::insert_hw_breakpoint (struct gdbarch *gdbarch, { struct ppc_hw_breakpoint p; - m_dreg_interface.detect (inferior_ptid); - if (!m_dreg_interface.hwdebug_p ()) return -1; @@ -2240,8 +2230,6 @@ ppc_linux_nat_target::remove_hw_breakpoint (struct gdbarch *gdbarch, { struct ppc_hw_breakpoint p; - m_dreg_interface.detect (inferior_ptid); - if (!m_dreg_interface.hwdebug_p ()) return -1; @@ -2346,8 +2334,6 @@ ppc_linux_nat_target::remove_mask_watchpoint (CORE_ADDR addr, CORE_ADDR mask, bool ppc_linux_nat_target::can_use_watchpoint_cond_accel (void) { - m_dreg_interface.detect (inferior_ptid); - if (!m_dreg_interface.hwdebug_p ()) return false; @@ -2546,8 +2532,6 @@ ppc_linux_nat_target::can_accel_watchpoint_condition (CORE_ADDR addr, { CORE_ADDR data_value; - m_dreg_interface.detect (inferior_ptid); - return (m_dreg_interface.hwdebug_p () && (m_dreg_interface.hwdebug_info ().num_condition_regs > 0) && check_condition (addr, cond, &data_value, &len)); @@ -2618,8 +2602,6 @@ ppc_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len, enum target_hw_bp_type type, struct expression *cond) { - m_dreg_interface.detect (inferior_ptid); - if (m_dreg_interface.unavailable_p ()) return -1; @@ -2705,6 +2687,12 @@ ppc_linux_nat_target::remove_watchpoint (CORE_ADDR addr, int len, return 0; } +void +ppc_linux_nat_target::low_init_process (pid_t pid) +{ + m_dreg_interface.detect (pid); +} + /* Clean up the per-process info associated with PID. When using the HWDEBUG interface, we also erase the per-thread state of installed debug registers for all the threads that belong to the group of PID. @@ -2716,8 +2704,7 @@ ppc_linux_nat_target::remove_watchpoint (CORE_ADDR addr, int len, void ppc_linux_nat_target::low_forget_process (pid_t pid) { - if ((!m_dreg_interface.detected_p ()) - || (m_dreg_interface.unavailable_p ())) + if (m_dreg_interface.unavailable_p ()) return; ptid_t pid_ptid (pid, 0, 0); @@ -2761,8 +2748,7 @@ void ppc_linux_nat_target::low_new_fork (struct lwp_info *parent, pid_t child_pid) { - if ((!m_dreg_interface.detected_p ()) - || (m_dreg_interface.unavailable_p ())) + if (m_dreg_interface.unavailable_p ()) return; auto process_it = m_process_info.find (parent->ptid.pid ()); @@ -2788,8 +2774,7 @@ void ppc_linux_nat_target::low_new_clone (struct lwp_info *parent, pid_t child_lwp) { - if ((!m_dreg_interface.detected_p ()) - || (m_dreg_interface.unavailable_p ())) + if (m_dreg_interface.unavailable_p ()) return; if (m_dreg_interface.hwdebug_p ()) @@ -2821,8 +2806,7 @@ ppc_linux_nat_target::low_delete_thread (struct arch_lwp_info { if (lp_arch_info != NULL) { - if (m_dreg_interface.detected_p () - && m_dreg_interface.hwdebug_p ()) + if (m_dreg_interface.hwdebug_p ()) m_installed_hw_bps.erase (lp_arch_info->lwp_ptid); xfree (lp_arch_info); @@ -2835,8 +2819,7 @@ ppc_linux_nat_target::low_delete_thread (struct arch_lwp_info void ppc_linux_nat_target::low_prepare_to_resume (struct lwp_info *lp) { - if ((!m_dreg_interface.detected_p ()) - || (m_dreg_interface.unavailable_p ())) + if (m_dreg_interface.unavailable_p ()) return; /* We have to re-install or clear the debug registers if we set the @@ -3030,8 +3013,6 @@ int ppc_linux_nat_target::masked_watch_num_registers (CORE_ADDR addr, CORE_ADDR mask) { - m_dreg_interface.detect (inferior_ptid); - if (!m_dreg_interface.hwdebug_p () || (m_dreg_interface.hwdebug_info ().features & PPC_DEBUG_FEATURE_DATA_BP_MASK) == 0) @@ -3069,8 +3050,7 @@ ppc_linux_nat_target::copy_thread_dreg_state (const ptid_t &parent_ptid, void ppc_linux_nat_target::mark_thread_stale (struct lwp_info *lp) { - if ((!m_dreg_interface.detected_p ()) - || (m_dreg_interface.unavailable_p ())) + if (m_dreg_interface.unavailable_p ()) return; arch_lwp_info *lp_arch_info = get_arch_lwp_info (lp); base-commit: 5527eae3f1f28a628f6c73c7b5743cebf7df8a13 -- 2.45.2