From patchwork Tue Mar 20 14:28:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Xavier Roirand X-Patchwork-Id: 26388 Received: (qmail 82615 invoked by alias); 20 Mar 2018 14:28:32 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 82605 invoked by uid 89); 20 Mar 2018 14:28:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=Alves, alves, HTo:U*palves X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 20 Mar 2018 14:28:29 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 5D04081386; Tue, 20 Mar 2018 15:28:26 +0100 (CET) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id F_s_T03nVJRJ; Tue, 20 Mar 2018 15:28:26 +0100 (CET) Received: from Xaviers-MacBook-Pro.local (unknown [10.10.8.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id 813DF8138F; Tue, 20 Mar 2018 15:28:25 +0100 (CET) Subject: [RFA v2] (x86) Fix watchpoint using hardware breakpoint for some distro To: Pedro Alves , gdb-patches@sourceware.org Cc: brobecker@adacore.com References: <1521209212-11264-1-git-send-email-roirand@adacore.com> From: Xavier Roirand Message-ID: <9c7c8586-2940-bea9-d3fb-a13b0d38a32e@adacore.com> Date: Tue, 20 Mar 2018 15:28:24 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: X-IsSubscribed: yes Hello Pedro, I've replied to your comments and attached a v2 patch. Le 3/19/18 à 3:28 PM, Pedro Alves a écrit : > A few things are missing here: > > #1 - kernel versions where this was observed. > CentOS: 2.6.18-419.el5 Suse: 2.6.27.19-5-pae > #2 - If it's not equal to TRAP_HWBKPT, then what's it equal to? > I assume zero? No, it's equal to 1. > Take a look at the big comment and table in nat/linux-ptrace.h -- is > this is the only case that is different on these kernels? > AFAIK, yes. > I think that we should update the table a bit here, at least > something like: > > - | hardware breakpoints/watchpoints | TRAP_HWBKPT | > + | hardware breakpoints/watchpoints | TRAP_HWBKPT (*) | > > (*) - Kernels x.y.z (CentOS 5, Suse 11) leave this as zero > If other cases are different, then that might affect how to best > address this. > Agree. > This comment only makes complete sense with the context in the > git log in mind: > > - This code is run by all architectures, so the comment should mention x86. > - The comment reads a bit backwards to me -- talks about what it should > be before talking about watchpoints. > > I'd suggest something like this: > > /* On some kernels (such as x86-64 x.y.z/CentOS 5, x.y.z/Suse 11), > when we continue into a watchpoint, si_code indicates 0 instead of > TRAP_HWBKPT so we need to check debug registers separately. */ > Agree. > Does the step-into-watchpoint case result in TRAP_TRACE, or does > that result in 0 too? That affects the "continue" in the comment above. This results to a value of 1 too. > Thanks, > Pedro Alves > Regards From 995662a4ffd5901511053c228e4ad6b396de9197 Mon Sep 17 00:00:00 2001 From: Xavier Roirand Date: Tue, 13 Mar 2018 03:52:14 +0100 Subject: [PATCH] (x86) Fix watchpoint using hardware breakpoint for some distro Running watch*.exp tests in gdb.base shows this: on x86_64/Ubuntu 16.04: # of expected passes 2631 # of unexpected failures 0 on x86_64/Ubuntu CentOS 5.11: # of expected passes 2535 # of unexpected failures 96 The problem can be easily shown in a debug session: (gdb) watch val Hardware watchpoint 2: val (gdb) c Continuing. Program received signal SIGTRAP, Trace/breakpoint trap. ... Whereas it should be: (gdb) watch val Hardware watchpoint 2: val (gdb) c Continuing. val before change = 0 Hardware watchpoint 2: val Old value = ... New value = ... The Linux target and gdbserver now check the siginfo si_code reported on a SIGTRAP to detect whether the trap indicates a hardware breakpoint was hit. Unfortunately, on some distro (CentOS 5, Suse 11) the returned si_code value is not equal to TRAP_HWBKPT when a hardware breakpoint is hit thus the hardware breakpoint is not handled as it should be, which is also the case for watchpoint when based on hardware breakpoint. This patch adds an additional check when the inferior reported a SIGTRAP in order to detect this case. No test have been created since all the existing ones are enough to validate the fix. BTW, with this fix, the tests results for the watchpoint tests are (for CentOS 5.11): # of expected passes 2630 # of unexpected failures 1 The remaining failure is located in watch-vfork test which explicitly disable the use of hardware breakpoint which is out of scope here. gdb/ChangeLog: * linux-nat.c (save_stop_reason): Add an additional check to detect hardware watchpoint. nat/ChangeLog: * linux-ptrace.h: Update comment before GDB_ARCH_IS_TRAP_XX macros to reflect CentOS 5 & Suse 11 specific case. gdbserver/ChangeLog: * linux-low.c (save_stop_reason): Add an additional check to detect hardware watchpoint. For R309-004 Test: x86_64/gdb /ubuntu 16.04 x86_64/gdbserver/ubuntu 16.04 x86 /gdbs /ubuntu 16.04 x86 /gdbserver/ubuntu 16.04 x86_64/gdb /centos 5.11 x86_64/gdbserver/centos 5.11 x86 /gdb /centos 5.11 x86 /gdbserver/centos 5.11 Change-Id: I2546aca9827d9ae12ab86deb7aa4acc60c82b4b4 --- gdb/gdbserver/linux-low.c | 8 ++++++++ gdb/linux-nat.c | 8 ++++++++ gdb/nat/linux-ptrace.h | 18 ++++++++++-------- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 2e5e19d..3fee99a 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -866,6 +866,14 @@ save_stop_reason (struct lwp_info *lwp) if (!check_stopped_by_watchpoint (lwp)) lwp->stop_reason = TARGET_STOPPED_BY_SINGLE_STEP; } + else + { + /* On some kernels (such as x86-64 2.6.18/CentOS 5, + 2.6.27/Suse 11), when we continue into a watchpoint, + si_code indicates 1 instead of TRAP_HWBKPT so we + need to check debug registers separately. */ + check_stopped_by_watchpoint (lwp); + } } } #else diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 1bbad7b..2e3fa29 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -2798,6 +2798,14 @@ save_stop_reason (struct lwp_info *lp) the debug registers separately. */ check_stopped_by_watchpoint (lp); } + else + { + /* On some kernels (such as x86-64 2.6.18/CentOS 5, + 2.6.27/Suse 11), when we continue into a watchpoint, + si_code indicates 1 instead of TRAP_HWBKPT so we + need to check debug registers separately. */ + check_stopped_by_watchpoint (lp); + } } } #else diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h index dc180fb..7e10fb3 100644 --- a/gdb/nat/linux-ptrace.h +++ b/gdb/nat/linux-ptrace.h @@ -120,14 +120,16 @@ struct buffer; /* The x86 kernel gets some of the si_code values backwards, like this: - | what | si_code | - |------------------------------------------+-------------| - | software breakpoints (int3) | SI_KERNEL | - | single-steps | TRAP_TRACE | - | single-stepping a syscall | TRAP_BRKPT | - | user sent SIGTRAP | 0 | - | exec SIGTRAP (when no PTRACE_EVENT_EXEC) | 0 | - | hardware breakpoints/watchpoints | TRAP_HWBKPT | + | what | si_code | + |------------------------------------------+-----------------| + | software breakpoints (int3) | SI_KERNEL | + | single-steps | TRAP_TRACE | + | single-stepping a syscall | TRAP_BRKPT | + | user sent SIGTRAP | 0 | + | exec SIGTRAP (when no PTRACE_EVENT_EXEC) | 0 | + | hardware breakpoints/watchpoints | TRAP_HWBKPT (*) | + + (*) - Kernels 2.6.18 (CentOS 5), 2.6.27 (Suse 11) set this to 1. That is, it reports SI_KERNEL for software breakpoints (and only for those), and TRAP_BRKPT for single-stepping a syscall... If the