From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 84152 invoked by alias); 4 Sep 2015 15:04:38 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 83256 invoked by uid 89); 4 Sep 2015 15:04:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg21.ericsson.net Received: from usevmg21.ericsson.net (HELO usevmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Fri, 04 Sep 2015 15:04:35 +0000 Received: from EUSAAHC002.ericsson.se (Unknown_Domain [147.117.188.78]) by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id F2.75.26730.3B849E55; Fri, 4 Sep 2015 09:30:59 +0200 (CEST) Received: from [142.133.110.95] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.80) with Microsoft SMTP Server id 14.3.248.2; Fri, 4 Sep 2015 11:04:32 -0400 Subject: Re: [PATCH 1/2] [gdbserver] Rename supports_conditional_breakpoints to supports_hardware_single_step To: , Yao Qi References: <1441096915-23615-1-git-send-email-yao.qi@linaro.org> <1441096915-23615-2-git-send-email-yao.qi@linaro.org> <55E74C68.8020608@ericsson.com> <55E8638C.1010806@ericsson.com> <55E9AD46.9070002@ericsson.com> From: Antoine Tremblay Message-ID: <55E9B301.2080905@ericsson.com> Date: Fri, 04 Sep 2015 15:04:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <55E9AD46.9070002@ericsson.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00068.txt.bz2 On 09/04/2015 10:40 AM, Antoine Tremblay wrote: > > > On 09/03/2015 11:13 AM, Antoine Tremblay wrote: >> >> >> On 09/02/2015 03:22 PM, Antoine Tremblay wrote: >>> >>> On 09/01/2015 04:41 AM, Yao Qi wrote: >>>> In my patch https://sourceware.org/ml/gdb-patches/2015-04/msg01110.html >>>> a new target_ops hook supports_conditional_breakpoints was added to >>>> disable conditional breakpoints if target doesn't have hardware single >>>> step. This patch is to generalize this hook from >>>> supports_conditional_breakpoints to supports_hardware_single_step, >>>> so that the following patch can use it. >>>> >>> >>> Could we generalize this even more to supports_single_step like your >>> next patch ? >>> >>> Since I'm working on software single stepping for ARM, if this patch >>> goes in I'll need to implement a supports_software_single_step and >>> enable ConditionalBreakpoints for this case too... >>> >>> Is there a need to know that it's really hardware or just knowing that >>> it can single step would be ok ? >>> >> >> Note also that this way would force supports_conditional_breakpoints to >> check for more than can_hardware_single_step and require a target >> function set manually to 1 on targets that we know have a proper >> software or hardware single step like : >> >> static int >> linux_supports_conditional_breakpoints (void) >> { >> return the_low_target.supports_conditional_breakpoints (); >> } >> >> I had initially added that in my set but we could change it for >> the_low_target_.can_single_step () ? >> >> This way targets that have proper software single step can be handled. >> Otherwise every software single step implementation is deemed too simple >> to be used... >> >> Would that seem ok ? >> >> > > After more analysis, I think ConditionalBreakpoints should be enabled > for software/hardware single step. > > But for the vCont s:S I don't think we can assume that gdbserver's > software single step is better then GDB's, it should be the opposite. So > it should only be enabled when hardware single step is present so we > can't do away with supports_hardware_single_step. > > So to support all features I suggest we have in GDBServer : > > supports_hardware_single_step > supports_software_single_step > > Enable ConditionalBreakpoints for supports_hardware_single_step || > supports_software_single_step > > And enable vCont s:s only for supports_hardware_single_step > > I can add the supports_software_single_step in my patchset.. > > Does that sounds right ? So your current patch would be fine.. > Another option would also be to remove the reinsert_addr implementations and have can_hardware_single_step still check if that implementation exists. Thus removing the need for a manual supports_software_single_step ... If the implementations were disabled for Conditional Breakpoints, I don't see why they should be kept ? I would prefer that option but maybe there's a scenario I"m not seeing?