From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by sourceware.org (Postfix) with ESMTPS id 5ADDA3858D20 for ; Wed, 17 Apr 2024 16:40:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5ADDA3858D20 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 5ADDA3858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.44 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713372037; cv=none; b=YbNo6sHWPvvcISfrMCWpFcdhy2GYPCARPdL0iPozy8l5OuTaFa8IJ3sTuWvWYrWw7XwTCqGpIDrDntaTOCqMzpGCP/A1LTQKbRgRNcWj1HAGi2kpkWyyw1JMR0951UZLnkIzPkqdoIRa+VdDoAAxn62BrPAP2yLC/xqWqRhg93o= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713372037; c=relaxed/simple; bh=aVGy4smoxs976G8O0Dc1Y0d2N9XxRpzfexBMiyIyvQU=; h=Message-ID:Date:MIME-Version:Subject:From:To; b=SY9mkaBuc+xq6FP0XZmpw55IcMvwCbySjdqweoCqFrFb9gMc7a6XVI3/aiy8SctfL0uCtyBiHWoGXo6J8cJzEqMYwwg14MmL11beVYC7m+1oEO+jRjcXxrhC4zel3MB0ZvSugf5yGiFck22MsE4CszHBiJZPpKB0cFQHu2IRWmg= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-416a8ec0239so6801135e9.0 for ; Wed, 17 Apr 2024 09:40:34 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713372033; x=1713976833; h=content-transfer-encoding:in-reply-to:content-language:references :to:from:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=oyWb+LbOv+7iGd/UXoKcJ4aLbvQW6dgGbHnonahlx/Y=; b=feFTjZydmZZ3iS7KXhnTsyl8Fc9tvm7dvlPSsfh9M/h1AcmRLAa/NQ+fo1MxgQOB69 rP9fDb+T2Y47Jz3r7DcuIks7qqGvb3QIWLY9qKyB39/XZpDiGMIkNWdK45J3lWlY0wht G6PKmVtz8Gk8fHKhOZyh4ELmArgAp1UhfLobmTH/cRUGdyKyylAuVUd1B8+OGtm8IL/L 8cdPbfpRS7BVaz2VPRe8pafdl6hIatgExi2LQH8rUhU2xWLofXVV9krTHvBHbD92OUKZ OjH14MMVK8l7XF5XcMc3Z9Op8xe1PcGFL6d3XfwmNuc4m2xHLxlCDDPnbgkWt6mdrkGT tc7Q== X-Gm-Message-State: AOJu0Yzvi5ofElSkUEveS1pgekJL1pEXUqx7jR3C5B3SfYkjdN+L0zZZ P+nsx1S8BLQ5CsCMypHi3HsuNMjBBDowUZAvwNPDLseCmPmWWnBJE3jJ8sVt X-Google-Smtp-Source: AGHT+IEMrL0Bj7R9xFGOHNOL1DVeuO3jCOKLLJYUH9IZnk3NcY5MdxUdAL/0jw0PvS/eFsY79/m2sg== X-Received: by 2002:a05:600c:3393:b0:418:5e81:415f with SMTP id o19-20020a05600c339300b004185e81415fmr2700309wmp.14.1713372032551; Wed, 17 Apr 2024 09:40:32 -0700 (PDT) Received: from ?IPV6:2001:8a0:f93d:b900:e643:3adf:640c:b3ad? ([2001:8a0:f93d:b900:e643:3adf:640c:b3ad]) by smtp.gmail.com with ESMTPSA id m16-20020a05600c3b1000b0041496734318sm3426995wms.24.2024.04.17.09.40.31 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 17 Apr 2024 09:40:32 -0700 (PDT) Message-ID: <93b4f656-a10c-4bda-8c85-6cb41fecc7fe@palves.net> Date: Wed, 17 Apr 2024 17:40:30 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] gdb+gdbserver/Linux: Remove USE_SIGTRAP_SIGINFO fallback From: Pedro Alves To: gdb-patches@sourceware.org References: <20220627211045.2339994-1-pedro@palves.net> Content-Language: en-US In-Reply-To: <20220627211045.2339994-1-pedro@palves.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.8 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 autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2022-06-27 22:10, Pedro Alves wrote: > It's been over 7 years (since commit faf09f0119da) since Linux GDB and > GDBserver started relying on SIGTRAP si_code to tell whether a > breakpoint triggered, which is important for non-stop mode. When that > then-new code was added, I had left the then-old code as fallback, in > case some architectured still needed it. Given AFAIK there haven't > been complaints since, this commit finally removes the fallback code, > along with USE_SIGTRAP_SIGINFO. > > Change-Id: I140a5333a9fe70e90dbd186aca1f081549b2e63d W00t. I was looking at linux-nat.c today, and noticed the USE_SIGTRAP_SIGINFO code was still around, which made me go like "but hadn't I removed this before??". Well, turns out I forgot to merge this. I'm going to merge it, after I download the patch from the list and update that "7 years". :-) Pedro Alves > --- > gdb/linux-nat.c | 53 ++---------------------------------------- > gdb/nat/linux-ptrace.h | 11 ++++----- > gdbserver/linux-low.cc | 39 ++----------------------------- > 3 files changed, 8 insertions(+), 95 deletions(-) > > diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c > index b9164e621db..008791c12dc 100644 > --- a/gdb/linux-nat.c > +++ b/gdb/linux-nat.c > @@ -2439,17 +2439,6 @@ status_callback (struct lwp_info *lp) > discard = 1; > } > > -#if !USE_SIGTRAP_SIGINFO > - else if (!breakpoint_inserted_here_p (regcache->aspace (), pc)) > - { > - linux_nat_debug_printf ("previous breakpoint of %s, at %s gone", > - lp->ptid.to_string ().c_str (), > - paddress (target_gdbarch (), lp->stop_pc)); > - > - discard = 1; > - } > -#endif > - > if (discard) > { > linux_nat_debug_printf ("pending event of %s cancelled.", > @@ -2529,9 +2518,7 @@ save_stop_reason (struct lwp_info *lp) > struct gdbarch *gdbarch; > CORE_ADDR pc; > CORE_ADDR sw_bp_pc; > -#if USE_SIGTRAP_SIGINFO > siginfo_t siginfo; > -#endif > > gdb_assert (lp->stop_reason == TARGET_STOPPED_BY_NO_REASON); > gdb_assert (lp->status != 0); > @@ -2545,7 +2532,6 @@ save_stop_reason (struct lwp_info *lp) > pc = regcache_read_pc (regcache); > sw_bp_pc = pc - gdbarch_decr_pc_after_break (gdbarch); > > -#if USE_SIGTRAP_SIGINFO > if (linux_nat_get_siginfo (lp->ptid, &siginfo)) > { > if (siginfo.si_signo == SIGTRAP) > @@ -2588,22 +2574,6 @@ save_stop_reason (struct lwp_info *lp) > } > } > } > -#else > - if ((!lp->step || lp->stop_pc == sw_bp_pc) > - && software_breakpoint_inserted_here_p (regcache->aspace (), > - sw_bp_pc)) > - { > - /* The LWP was either continued, or stepped a software > - breakpoint instruction. */ > - lp->stop_reason = TARGET_STOPPED_BY_SW_BREAKPOINT; > - } > - > - if (hardware_breakpoint_inserted_here_p (regcache->aspace (), pc)) > - lp->stop_reason = TARGET_STOPPED_BY_HW_BREAKPOINT; > - > - if (lp->stop_reason == TARGET_STOPPED_BY_NO_REASON) > - check_stopped_by_watchpoint (lp); > -#endif > > if (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT) > { > @@ -2649,7 +2619,7 @@ linux_nat_target::stopped_by_sw_breakpoint () > bool > linux_nat_target::supports_stopped_by_sw_breakpoint () > { > - return USE_SIGTRAP_SIGINFO; > + return true; > } > > /* Returns true if the LWP had stopped for a hardware > @@ -2670,7 +2640,7 @@ linux_nat_target::stopped_by_hw_breakpoint () > bool > linux_nat_target::supports_stopped_by_hw_breakpoint () > { > - return USE_SIGTRAP_SIGINFO; > + return true; > } > > /* Select one LWP out of those that have events pending. */ > @@ -3252,25 +3222,6 @@ linux_nat_wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus, > > gdb_assert (lp != NULL); > > - /* Now that we've selected our final event LWP, un-adjust its PC if > - it was a software breakpoint, and we can't reliably support the > - "stopped by software breakpoint" stop reason. */ > - if (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT > - && !USE_SIGTRAP_SIGINFO) > - { > - struct regcache *regcache = get_thread_regcache (linux_target, lp->ptid); > - struct gdbarch *gdbarch = regcache->arch (); > - int decr_pc = gdbarch_decr_pc_after_break (gdbarch); > - > - if (decr_pc != 0) > - { > - CORE_ADDR pc; > - > - pc = regcache_read_pc (regcache); > - regcache_write_pc (regcache, pc + decr_pc); > - } > - } > - > /* We'll need this to determine whether to report a SIGSTOP as > GDB_SIGNAL_0. Need to take a copy because resume_clear_callback > clears it. */ > diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h > index 4694046e1e8..3edc2363aa2 100644 > --- a/gdb/nat/linux-ptrace.h > +++ b/gdb/nat/linux-ptrace.h > @@ -97,9 +97,9 @@ struct buffer; > #define __WALL 0x40000000 /* Wait for any child. */ > #endif > > -/* True if whether a breakpoint/watchpoint triggered can be determined > - from the si_code of SIGTRAP's siginfo_t (TRAP_BRKPT/TRAP_HWBKPT). > - That is, if the kernel can tell us whether the thread executed a > +/* Whether a breakpoint/watchpoint triggered can be determined from > + the si_code of SIGTRAP's siginfo_t (TRAP_BRKPT/TRAP_HWBKPT). That > + is, since the kernel can tell us whether the thread executed a > software breakpoint, we trust it. The kernel will be determining > that from the hardware (e.g., from which exception was raised in > the CPU). Relying on whether a breakpoint is planted in memory at > @@ -112,10 +112,7 @@ struct buffer; > architectures. The moribund location mechanism helps with that > somewhat but it is an heuristic, and can well fail. Getting that > information out of the kernel and ultimately out of the CPU is the > - way to go. That said, some architecture may get the si_code wrong, > - and as such we're leaving fallback code in place. We'll remove > - this after a while if no problem is reported. */ > -#define USE_SIGTRAP_SIGINFO 1 > + way to go. */ > > /* The x86 kernel gets some of the si_code values backwards, like > this: > diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc > index 8b8614f6ed4..9305928bbbf 100644 > --- a/gdbserver/linux-low.cc > +++ b/gdbserver/linux-low.cc > @@ -790,9 +790,7 @@ linux_process_target::save_stop_reason (lwp_info *lwp) > { > CORE_ADDR pc; > CORE_ADDR sw_breakpoint_pc; > -#if USE_SIGTRAP_SIGINFO > siginfo_t siginfo; > -#endif > > if (!low_supports_breakpoints ()) > return false; > @@ -804,7 +802,6 @@ linux_process_target::save_stop_reason (lwp_info *lwp) > scoped_restore_current_thread restore_thread; > switch_to_thread (get_lwp_thread (lwp)); > > -#if USE_SIGTRAP_SIGINFO > if (ptrace (PTRACE_GETSIGINFO, lwpid_of (current_thread), > (PTRACE_TYPE_ARG3) 0, &siginfo) == 0) > { > @@ -846,21 +843,6 @@ linux_process_target::save_stop_reason (lwp_info *lwp) > } > } > } > -#else > - /* We may have just stepped a breakpoint instruction. E.g., in > - non-stop mode, GDB first tells the thread A to step a range, and > - then the user inserts a breakpoint inside the range. In that > - case we need to report the breakpoint PC. */ > - if ((!lwp->stepping || lwp->stop_pc == sw_breakpoint_pc) > - && low_breakpoint_at (sw_breakpoint_pc)) > - lwp->stop_reason = TARGET_STOPPED_BY_SW_BREAKPOINT; > - > - if (hardware_breakpoint_inserted_here (pc)) > - lwp->stop_reason = TARGET_STOPPED_BY_HW_BREAKPOINT; > - > - if (lwp->stop_reason == TARGET_STOPPED_BY_NO_REASON) > - check_stopped_by_watchpoint (lwp); > -#endif > > if (lwp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT) > { > @@ -1648,23 +1630,6 @@ linux_process_target::thread_still_has_status_pending (thread_info *thread) > discard = 1; > } > > -#if !USE_SIGTRAP_SIGINFO > - else if (lp->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT > - && !low_breakpoint_at (pc)) > - { > - threads_debug_printf ("previous SW breakpoint of %ld gone", > - lwpid_of (thread)); > - discard = 1; > - } > - else if (lp->stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT > - && !hardware_breakpoint_inserted_here (pc)) > - { > - threads_debug_printf ("previous HW breakpoint of %ld gone", > - lwpid_of (thread)); > - discard = 1; > - } > -#endif > - > if (discard) > { > threads_debug_printf ("discarding pending breakpoint status"); > @@ -5555,7 +5520,7 @@ linux_process_target::stopped_by_sw_breakpoint () > bool > linux_process_target::supports_stopped_by_sw_breakpoint () > { > - return USE_SIGTRAP_SIGINFO; > + return true; > } > > /* Implement the stopped_by_hw_breakpoint target_ops > @@ -5575,7 +5540,7 @@ linux_process_target::stopped_by_hw_breakpoint () > bool > linux_process_target::supports_stopped_by_hw_breakpoint () > { > - return USE_SIGTRAP_SIGINFO; > + return true; > } > > /* Implement the supports_hardware_single_step target_ops method. */ > > base-commit: f18acc9c4e5d18f4783f3a7d59e3ec95d7af0199