From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 100038 invoked by alias); 23 Sep 2015 14:56:55 -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 100022 invoked by uid 89); 23 Sep 2015 14:56:54 -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; Wed, 23 Sep 2015 14:56:53 +0000 Received: from EUSAAHC007.ericsson.se (Unknown_Domain [147.117.188.93]) by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id 9F.50.26730.A6252065; Wed, 23 Sep 2015 09:19:07 +0200 (CEST) Received: from [142.133.110.95] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.95) with Microsoft SMTP Server id 14.3.248.2; Wed, 23 Sep 2015 10:56:50 -0400 Subject: Re: [PATCH 1/5] Support multiple breakpoint types per target in GDBServer. To: Yao Qi References: <1442577749-6650-1-git-send-email-antoine.tremblay@ericsson.com> <1442577749-6650-2-git-send-email-antoine.tremblay@ericsson.com> <86oagt9znr.fsf@gmail.com> <56029CF3.8000001@ericsson.com> <86h9ml9osh.fsf@gmail.com> CC: From: Antoine Tremblay Message-ID: <5602BDB2.3050004@ericsson.com> Date: Wed, 23 Sep 2015 14:56: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: <86h9ml9osh.fsf@gmail.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00548.txt.bz2 On 09/23/2015 10:46 AM, Yao Qi wrote: > Antoine Tremblay writes: > >>> PC can't be NULL after your patch #2. You can remove the second >>> sentence in this patch or patch #2. >>> >> I think you mean after patch #3 ? >> >> But it can still be NULL see in #3 : >> >> /* Default if no pc is set to arm breakpoint. */ >> + if (pcptr == NULL) >> + { >> + *lenptr = arm_breakpoint_len; >> + return (unsigned char *) &arm_breakpoint; >> + } > > I meant this change below in patch #2, > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index bb08761..06387a0 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -7069,16 +7069,10 @@ void > initialize_low (void) > { > struct sigaction sigchld_action; > - int breakpoint_len = 0; > - const unsigned char *breakpoint = NULL; > > memset (&sigchld_action, 0, sizeof (sigchld_action)); > set_target_ops (&linux_target_ops); > > - breakpoint = the_target->breakpoint_from_pc (NULL, &breakpoint_len); > - > - set_breakpoint_data (breakpoint, > - breakpoint_len); > linux_init_signals (); > linux_ptrace_init_warnings (); > > We only pass NULL to breakpoint_from_pc here, and we remove it from > patch #2. That is why I suggest that PCPTR can't be NULL. > Ok I see, indeed it is never called again with NULL, good point I'll remove that and also remove the default NULL handling in the arm implementation. > [I am still reviewing this patch series. I get something I can't > explain after I enable thread event breakpoint in order to exercise > your patches. I'll send out my comments once I understand the them > fully.] > Ok thank you, note I'm on IRC as hexa if you ever need some quick answer.