From patchwork Thu Apr 30 16:23:49 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yao Qi X-Patchwork-Id: 47855 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-lb0-f198.google.com (mail-lb0-f198.google.com [209.85.217.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id EDB042121F for ; Thu, 30 Apr 2015 16:24:16 +0000 (UTC) Received: by lbos2 with SMTP id s2sf16076298lbo.2 for ; Thu, 30 Apr 2015 09:24:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:mailing-list :precedence:list-id:list-unsubscribe:list-subscribe:list-archive :list-post:list-help:sender:delivered-to:from:to:subject:date :message-id:x-original-sender:x-original-authentication-results; bh=0/6ZfMrM7lyJeXSVSHNLfsenasmqxFRjuchEL15bX3o=; b=jM7/w465vlPCKqy1qlI+sR8kkrHb/ljldzA7MZiVrocvV5xpe9RGE+/xAF7JXL0vYD TiijH2hVAjSFsLgQu04ZcKHTkbSK28obA80guoTjsG7ATY3th8k24wu1gDBX2QXX+4CF IYyiUm6T0yw4yiD9XWAdIGaI0Rx9S7nhOPsYuO4GdlK/E5xss7KuB6buoedzK+3j3SO8 w+3co+FBFp8dx7KDug76tTLi9fx9XtgbtDBpVifM7GxyQIAICP6DMLa8l+R2eTjdKeUI +R8kehSXfapWJq8eYe8k5i5dr5NFniLohseZSAn5ZU+o+VZsFA+5gZPpRmUl6RcLMYJR ip5w== X-Gm-Message-State: ALoCoQlU0MmZJteln3ydV4Je7o3Jv1K4y3MsUXpwXbZQ3rxEMSlORmVe2wUdeK/SNiCmt/V6s9Vf X-Received: by 10.112.13.200 with SMTP id j8mr2974936lbc.14.1430411055935; Thu, 30 Apr 2015 09:24:15 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.153.7.67 with SMTP id da3ls457564lad.49.gmail; Thu, 30 Apr 2015 09:24:15 -0700 (PDT) X-Received: by 10.152.36.136 with SMTP id q8mr4509895laj.96.1430411055793; Thu, 30 Apr 2015 09:24:15 -0700 (PDT) Received: from mail-lb0-x22a.google.com (mail-lb0-x22a.google.com. [2a00:1450:4010:c04::22a]) by mx.google.com with ESMTPS id bb9si2079862lab.145.2015.04.30.09.24.15 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 30 Apr 2015 09:24:15 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c04::22a as permitted sender) client-ip=2a00:1450:4010:c04::22a; Received: by lbbzk7 with SMTP id zk7so48812836lbb.0 for ; Thu, 30 Apr 2015 09:24:15 -0700 (PDT) X-Received: by 10.152.4.137 with SMTP id k9mr4532380lak.29.1430411055337; Thu, 30 Apr 2015 09:24:15 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.67.65 with SMTP id l1csp3215402lbt; Thu, 30 Apr 2015 09:24:14 -0700 (PDT) X-Received: by 10.70.123.168 with SMTP id mb8mr9760341pdb.100.1430411053058; Thu, 30 Apr 2015 09:24:13 -0700 (PDT) Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id b5si4210196pdk.61.2015.04.30.09.24.11 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 30 Apr 2015 09:24:13 -0700 (PDT) Received-SPF: pass (google.com: domain of gdb-patches-return-122525-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Received: (qmail 83636 invoked by alias); 30 Apr 2015 16:24:02 -0000 Mailing-List: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org Precedence: list 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 83608 invoked by uid 89); 30 Apr 2015 16:24:02 -0000 X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f170.google.com Received: from mail-pd0-f170.google.com (HELO mail-pd0-f170.google.com) (209.85.192.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 30 Apr 2015 16:23:58 +0000 Received: by pdbnk13 with SMTP id nk13so65405568pdb.0 for ; Thu, 30 Apr 2015 09:23:55 -0700 (PDT) X-Received: by 10.70.96.162 with SMTP id dt2mr9698885pdb.20.1430411035675; Thu, 30 Apr 2015 09:23:55 -0700 (PDT) Received: from E107787-LIN.cambridge.arm.com (gcc1-power7.osuosl.org. [140.211.15.137]) by mx.google.com with ESMTPSA id ki3sm2608703pdb.74.2015.04.30.09.23.53 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 30 Apr 2015 09:23:54 -0700 (PDT) From: Yao Qi To: gdb-patches@sourceware.org Subject: [PATCH] [gdbserver] Disable conditional breakpoints on no-hardware-single-step targets Date: Thu, 30 Apr 2015 17:23:49 +0100 Message-Id: <1430411029-12097-1-git-send-email-qiyaoltc@gmail.com> X-IsSubscribed: yes X-Original-Sender: qiyaoltc@gmail.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2a00:1450:4010:c04::22a as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=pass header.i=@sourceware.org; dmarc=fail (p=NONE dis=NONE) header.from=gmail.com X-Google-Group-Id: 836684582541 From: Yao Qi GDBserver steps over breakpoint if the condition is false, but if target doesn't support hardware single step, the step over is very simple, if not incorrect, in linux-arm-low.c: /* We only place breakpoints in empty marker functions, and thread locking is outside of the function. So rather than importing software single-step, we can just run until exit. */ static CORE_ADDR arm_reinsert_addr (void) { struct regcache *regcache = get_thread_regcache (current_thread, 1); unsigned long pc; collect_register_by_name (regcache, "lr", &pc); return pc; } and linux-mips-low.c does the same. GDBserver sets a breakpoint at the return address of the current function, resume and wait the program hits the breakpoint in order to achieve "breakpoint step over". What if program hits other user breakponits during this "step over"? It is worse if the arm/thumb interworking is considered. Nowadays, GDBserver arm backend unconditionally inserts arm breakpoint, /* Define an ARM-mode breakpoint; we only set breakpoints in the C library, which is most likely to be ARM. If the kernel supports clone events, we will never insert a breakpoint, so even a Thumb C library will work; so will mixing EABI/non-EABI gdbserver and application. */ #ifndef __ARM_EABI__ (const unsigned char *) &arm_breakpoint, #else (const unsigned char *) &arm_eabi_breakpoint, #endif note that the comments are no longer valid as C library can be compiled in thumb mode. When GDBserver steps over a breakpoint in arm mode function, which returns to thumb mode, GDBserver will insert arm mode breakpoint by mistake and the program will crash. GDBserver alone is unable to determine the arm/thumb mode given a PC address. See how GDB does it in arm-tdep.c:arm_pc_is_thumb. After thinking about how to teach GDBserver inserting right breakpoint (arm or thumb) for a while, I reconsider it from a different direction that it may be unreasonable to run target-side conditional breakpoint for targets without hardware single step. Pedro also pointed this out here https://sourceware.org/ml/gdb-patches/2015-04/msg00337.html This patch is to add a new target_ops hook supports_conditional_breakpoints, and only reply ";ConditionalBreakpoints+" if it is true. On linux targets, supports_conditional_breakpoints returns true if target has hardware single step, on other targets, (win32, lynx, nto, spu), set it to NULL, because conditional breakpoint is a linux-specific feature. Regression tested on x86_64-linux, rebuild mingw32 gdbserver. Don't to rebuild gdbserver for lynx, nto and spu. gdb/gdbserver: 2015-04-30 Yao Qi * linux-low.c (linux_supports_conditional_breakpoints): New function. (linux_target_ops): Install new target method. * lynx-low.c (lynx_target_ops): Install NULL hook for supports_conditional_breakpoints. * nto-low.c (nto_target_ops): Likewise. * spu-low.c (spu_target_ops): Likewise. * win32-low.c (win32_target_ops): Likewise. * server.c (handle_query): Check target_supports_conditional_breakpoints. * target.h (struct target_ops) : New field. (target_supports_conditional_breakpoints): New macro. --- gdb/gdbserver/linux-low.c | 14 ++++++++++++++ gdb/gdbserver/lynx-low.c | 3 +++ gdb/gdbserver/nto-low.c | 3 +++ gdb/gdbserver/server.c | 3 ++- gdb/gdbserver/spu-low.c | 1 + gdb/gdbserver/target.h | 8 ++++++++ gdb/gdbserver/win32-low.c | 3 +++ 7 files changed, 34 insertions(+), 1 deletion(-) diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 1c4c2d7..bc76ffc 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -5177,6 +5177,19 @@ linux_supports_stopped_by_hw_breakpoint (void) return USE_SIGTRAP_SIGINFO; } +/* Implement the supports_conditional_breakpoints target_ops + method. */ + +static int +linux_supports_conditional_breakpoints (void) +{ + /* GDBserver needs to step over the breakpoint if the condition is + false. GDBserver software single step is too simple, so disable + conditional breakpoints if the target doesn't have hardware single + step. */ + return can_hardware_single_step (); +} + static int linux_stopped_by_watchpoint (void) { @@ -6375,6 +6388,7 @@ static struct target_ops linux_target_ops = { linux_supports_stopped_by_sw_breakpoint, linux_stopped_by_hw_breakpoint, linux_supports_stopped_by_hw_breakpoint, + linux_supports_conditional_breakpoints, linux_stopped_by_watchpoint, linux_stopped_data_address, #if defined(__UCLIBC__) && defined(HAS_NOMMU) \ diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c index 2f85829..ac83cfd 100644 --- a/gdb/gdbserver/lynx-low.c +++ b/gdb/gdbserver/lynx-low.c @@ -746,6 +746,9 @@ static struct target_ops lynx_target_ops = { NULL, /* supports_stopped_by_sw_breakpoint */ NULL, /* stopped_by_hw_breakpoint */ NULL, /* supports_stopped_by_hw_breakpoint */ + /* Although lynx has hardware single step, still disable this + feature for lynx, because it is quite GNU/Linux specific. */ + NULL, /* supports_conditional_breakpoints */ NULL, /* stopped_by_watchpoint */ NULL, /* stopped_data_address */ NULL, /* read_offsets */ diff --git a/gdb/gdbserver/nto-low.c b/gdb/gdbserver/nto-low.c index 801d76a..686f293 100644 --- a/gdb/gdbserver/nto-low.c +++ b/gdb/gdbserver/nto-low.c @@ -949,6 +949,9 @@ static struct target_ops nto_target_ops = { NULL, /* supports_stopped_by_sw_breakpoint */ NULL, /* stopped_by_hw_breakpoint */ NULL, /* supports_stopped_by_hw_breakpoint */ + /* Although nto has hardware single step, still disable this + feature for not, because it is quite GNU/Linux specific. */ + NULL, /* supports_conditional_breakpoints */ nto_stopped_by_watchpoint, nto_stopped_data_address, NULL, /* nto_read_offsets */ diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index d2e20d9..8e7ca57 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -2105,7 +2105,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) } /* Support target-side breakpoint conditions and commands. */ - strcat (own_buf, ";ConditionalBreakpoints+"); + if (target_supports_conditional_breakpoints ()) + strcat (own_buf, ";ConditionalBreakpoints+"); strcat (own_buf, ";BreakpointCommands+"); if (target_supports_agent ()) diff --git a/gdb/gdbserver/spu-low.c b/gdb/gdbserver/spu-low.c index 73f1786..a56a889 100644 --- a/gdb/gdbserver/spu-low.c +++ b/gdb/gdbserver/spu-low.c @@ -662,6 +662,7 @@ static struct target_ops spu_target_ops = { NULL, /* supports_stopped_by_sw_breakpoint */ NULL, /* stopped_by_hw_breakpoint */ NULL, /* supports_stopped_by_hw_breakpoint */ + NULL, /* supports_conditional_breakpoints */ NULL, NULL, NULL, diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h index 6280c26..b3d08cd 100644 --- a/gdb/gdbserver/target.h +++ b/gdb/gdbserver/target.h @@ -222,6 +222,10 @@ struct target_ops HW breakpoint triggering. */ int (*supports_stopped_by_hw_breakpoint) (void); + /* Returns true if the target can evaluate conditions of + breakpoints. */ + int (*supports_conditional_breakpoints) (void); + /* Returns 1 if target was stopped due to a watchpoint hit, 0 otherwise. */ int (*stopped_by_watchpoint) (void); @@ -550,6 +554,10 @@ int kill_inferior (int); (the_target->supports_stopped_by_hw_breakpoint ? \ (*the_target->supports_stopped_by_hw_breakpoint) () : 0) +#define target_supports_conditional_breakpoints() \ + (the_target->supports_conditional_breakpoints ? \ + (*the_target->supports_conditional_breakpoints) () : 0) + #define target_stopped_by_hw_breakpoint() \ (the_target->stopped_by_hw_breakpoint ? \ (*the_target->stopped_by_hw_breakpoint) () : 0) diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c index 6c86765..91b6d8e 100644 --- a/gdb/gdbserver/win32-low.c +++ b/gdb/gdbserver/win32-low.c @@ -1811,6 +1811,9 @@ static struct target_ops win32_target_ops = { NULL, /* supports_stopped_by_sw_breakpoint */ NULL, /* stopped_by_hw_breakpoint */ NULL, /* supports_stopped_by_hw_breakpoint */ + /* Although win32-i386 has hardware single step, still disable this + feature for win32, because it is quite GNU/Linux specific. */ + NULL, /* supports_conditional_breakpoints */ win32_stopped_by_watchpoint, win32_stopped_data_address, NULL, /* read_offsets */