From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 45582 invoked by alias); 2 Sep 2016 08:05:21 -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 45534 invoked by uid 89); 2 Sep 2016 08:05:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=H*RU:209.85.220.68, Hx-spam-relays-external:209.85.220.68, sk:status_ X-HELO: mail-pa0-f68.google.com Received: from mail-pa0-f68.google.com (HELO mail-pa0-f68.google.com) (209.85.220.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 02 Sep 2016 08:05:08 +0000 Received: by mail-pa0-f68.google.com with SMTP id ez1so5259503pab.3 for ; Fri, 02 Sep 2016 01:05:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=Ao9AFhsUN2IuLPCyG8R3itfWnKAMkPD4r9x6kRsgSbw=; b=dLlJREMbBqrY0Ld+vtBcscmQ8UHseVDNwawNPhzyRjhkvq/aXRAfjorV2NfKRf1xoq 9OXxMMW0I59NQE/C0za1m3Ww1h/gj4ZCX8oRdrZaTkZ4Z69IqEt7eXsdqY2WcMR+JLmO bBzkXzbvX9cy97/iHE1fPIEusHSTWen2gsSB9q5rXsfBUKRT7pIpC+zu7QTuk19vt2Vo w4gZElK5sJwfq5fCaC9avMZhaFc6Az4SpLYW3ku7Evy+Z8m+Z4m6xPilSJZNwY/61Bcv jSAJUK/7f2bjG8gFBawVgDFRfogg/wqaty16xYGlF/22ZzlwTzBdK9ItTjpFgQelFIPc qiCw== X-Gm-Message-State: AE9vXwPeb7u4O/36Azx5EOSMtvi66Zd8g2pxpFLKXHoCXJvhJ8d/MrtR3rmsLPYTFETi9g== X-Received: by 10.66.173.14 with SMTP id bg14mr34602578pac.42.1472803506760; Fri, 02 Sep 2016 01:05:06 -0700 (PDT) Received: from E107787-LIN (gcc115.osuosl.org. [140.211.9.73]) by smtp.gmail.com with ESMTPSA id d9sm12678802pan.7.2016.09.02.01.05.01 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Fri, 02 Sep 2016 01:05:06 -0700 (PDT) From: Yao Qi To: Pedro Alves Cc: Yao Qi , gdb-patches@sourceware.org Subject: Re: [PATCH] [GDBserver] Replace "reinsert_breakpoint" with "singlestep_breakpoint" References: <1472720480-4651-1-git-send-email-yao.qi@linaro.org> Date: Fri, 02 Sep 2016 08:05:00 -0000 In-Reply-To: (Pedro Alves's message of "Thu, 1 Sep 2016 17:18:32 +0100") Message-ID: <86r392buc6.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-09/txt/msg00032.txt.bz2 Pedro Alves writes: > I hate to be this picky, but I have to say that it itches me that both > gdb and gdbserver currently largely use "single-step" instead of "singles= tep" > in comments and largely uses "single_step" instead of "singlestep" in > function signatures: > > $ cd gdb > $ grep -rn "single-step" --exclude=3D'ChangeLog*' --exclude=3D"gdbarch.c" > --exclude=3D"gdbarch.h" | wc -l > 371 > $ grep -rn "single_step" --exclude=3D'ChangeLog*' --exclude=3D"gdbarch.c" > --exclude=3D"gdbarch.h" | wc -l > 276 > $ grep -rn "singlestep" --exclude=3D'ChangeLog*' --exclude=3D"gdbarch.c" > --exclude=3D"gdbarch.h" | wc -l > 37 > > Did you consider following this scheme? Sure. Let us use "single_step" in code and "single-step" in comments. How about the patch below? --=20 Yao (=E9=BD=90=E5=B0=A7) From: Yao Qi Date: Thu, 1 Sep 2016 09:58:47 +0100 Subject: [PATCH] [GDBserver] Replace "reinsert_breakpoint" with "single_ste= p_breakpoint" reinsert_breakpoint is used for software single step, so it is more clear to rename it to single_step_breakpoint. This was pointed out in the review https://sourceware.org/ml/gdb-patches/2016-05/msg00429.html I don't rename "other_breakpoint" in this patch. gdb/gdbserver: 2016-09-02 Yao Qi * linux-low.c: Replace "reinsert_breakpoints" with "single_step_breakpoints". Replace "reinsert breakpoints" with "single-step breakpoints". * mem-break.c: Likewise. * mem-break.h: Likewise. --- gdb/gdbserver/linux-low.c | 85 ++++++++++++++++++++++++-------------------= ---- gdb/gdbserver/mem-break.c | 66 ++++++++++++++++++------------------ gdb/gdbserver/mem-break.h | 26 +++++++-------- 3 files changed, 89 insertions(+), 88 deletions(-) diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 45061ac..21bc06e 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -550,11 +550,11 @@ handle_extended_wait (struct lwp_info **orig_event_lw= p, int wstat) && can_software_single_step () && event =3D=3D PTRACE_EVENT_VFORK) { - /* If we leave reinsert breakpoints there, child will - hit it, so uninsert reinsert breakpoints from parent + /* If we leave single-step breakpoints there, child will + hit it, so uninsert single-step breakpoints from parent (and child). Once vfork child is done, reinsert them back to parent. */ - uninsert_reinsert_breakpoints (event_thr); + uninsert_single_step_breakpoints (event_thr); } =20 clone_all_breakpoints (child_thr, event_thr); @@ -581,8 +581,8 @@ handle_extended_wait (struct lwp_info **orig_event_lwp,= int wstat) event_lwp->status_pending_p =3D 1; event_lwp->status_pending =3D wstat; =20 - /* If the parent thread is doing step-over with reinsert - breakpoints, the list of reinsert breakpoints are cloned + /* If the parent thread is doing step-over with single-step + breakpoints, the list of single-step breakpoints are cloned from the parent's. Remove them from the child process. In case of vfork, we'll reinsert them back once vforked child is done. */ @@ -592,10 +592,10 @@ handle_extended_wait (struct lwp_info **orig_event_lw= p, int wstat) /* The child process is forked and stopped, so it is safe to access its memory without stopping all other threads from other processes. */ - delete_reinsert_breakpoints (child_thr); + delete_single_step_breakpoints (child_thr); =20 - gdb_assert (has_reinsert_breakpoints (event_thr)); - gdb_assert (!has_reinsert_breakpoints (child_thr)); + gdb_assert (has_single_step_breakpoints (event_thr)); + gdb_assert (!has_single_step_breakpoints (child_thr)); } =20 /* Report the event. */ @@ -649,9 +649,9 @@ handle_extended_wait (struct lwp_info **orig_event_lwp,= int wstat) =20 if (event_lwp->bp_reinsert !=3D 0 && can_software_single_step ()) { - reinsert_reinsert_breakpoints (event_thr); + reinsert_single_step_breakpoints (event_thr); =20 - gdb_assert (has_reinsert_breakpoints (event_thr)); + gdb_assert (has_single_step_breakpoints (event_thr)); } =20 /* Report the event. */ @@ -2622,9 +2622,9 @@ maybe_hw_step (struct thread_info *thread) return 1; else { - /* GDBserver must insert reinsert breakpoint for software + /* GDBserver must insert single-step breakpoint for software single step. */ - gdb_assert (has_reinsert_breakpoints (thread)); + gdb_assert (has_single_step_breakpoints (thread)); return 0; } } @@ -3330,13 +3330,13 @@ linux_wait_1 (ptid_t ptid, the breakpoint address. So in the case of the hardware single step advance the PC manually past the breakpoint and in the case of software single step advance o= nly - if it's not the reinsert_breakpoint we are hitting. + if it's not the single_step_breakpoint we are hitting. This avoids that a program would keep trapping a permanent breakpoint forever. */ if (!ptid_equal (step_over_bkpt, null_ptid) && event_child->stop_reason =3D=3D TARGET_STOPPED_BY_SW_BREAKPOINT && (event_child->stepping - || !reinsert_breakpoint_inserted_here (event_child->stop_pc))) + || !single_step_breakpoint_inserted_here (event_child->stop_pc))) { int increment_pc =3D 0; int breakpoint_kind =3D 0; @@ -3390,8 +3390,8 @@ linux_wait_1 (ptid_t ptid, =20 /* We have a SIGTRAP, possibly a step-over dance has just finished. If so, tweak the state machine accordingly, - reinsert breakpoints and delete any reinsert (software - single-step) breakpoints. */ + reinsert breakpoints and delete any single-step + breakpoints. */ step_over_finished =3D finish_step_over (event_child); =20 /* Now invoke the callbacks of any internal breakpoints there. */ @@ -3705,48 +3705,48 @@ linux_wait_1 (ptid_t ptid, =20 /* Alright, we're going to report a stop. */ =20 - /* Remove reinsert breakpoints. */ + /* Remove single-step breakpoints. */ if (can_software_single_step ()) { - /* Remove reinsert breakpoints or not. It it is true, stop all + /* Remove single-step breakpoints or not. It it is true, stop all lwps, so that other threads won't hit the breakpoint in the staled memory. */ - int remove_reinsert_breakpoints_p =3D 0; + int remove_single_step_breakpoints_p =3D 0; =20 if (non_stop) { - remove_reinsert_breakpoints_p - =3D has_reinsert_breakpoints (current_thread); + remove_single_step_breakpoints_p + =3D has_single_step_breakpoints (current_thread); } else { /* In all-stop, a stop reply cancels all previous resume - requests. Delete all reinsert breakpoints. */ + requests. Delete all single-step breakpoints. */ struct inferior_list_entry *inf, *tmp; =20 ALL_INFERIORS (&all_threads, inf, tmp) { struct thread_info *thread =3D (struct thread_info *) inf; =20 - if (has_reinsert_breakpoints (thread)) + if (has_single_step_breakpoints (thread)) { - remove_reinsert_breakpoints_p =3D 1; + remove_single_step_breakpoints_p =3D 1; break; } } } =20 - if (remove_reinsert_breakpoints_p) + if (remove_single_step_breakpoints_p) { - /* If we remove reinsert breakpoints from memory, stop all lwps, + /* If we remove single-step breakpoints from memory, stop all lwps, so that other threads won't hit the breakpoint in the staled memory. */ stop_all_lwps (0, event_child); =20 if (non_stop) { - gdb_assert (has_reinsert_breakpoints (current_thread)); - delete_reinsert_breakpoints (current_thread); + gdb_assert (has_single_step_breakpoints (current_thread)); + delete_single_step_breakpoints (current_thread); } else { @@ -3756,8 +3756,8 @@ linux_wait_1 (ptid_t ptid, { struct thread_info *thread =3D (struct thread_info *) inf; =20 - if (has_reinsert_breakpoints (thread)) - delete_reinsert_breakpoints (thread); + if (has_single_step_breakpoints (thread)) + delete_single_step_breakpoints (thread); } } =20 @@ -4279,7 +4279,7 @@ install_software_single_step_breakpoints (struct lwp_= info *lwp) next_pcs =3D (*the_low_target.get_next_pcs) (regcache); =20 for (i =3D 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); ++i) - set_reinsert_breakpoint (pc, current_ptid); + set_single_step_breakpoint (pc, current_ptid); =20 do_cleanups (old_chain); } @@ -4880,7 +4880,7 @@ start_step_over (struct lwp_info *lwp) } =20 /* Finish a step-over. Reinsert the breakpoint we had uninserted in - start_step_over, if still there, and delete any reinsert + start_step_over, if still there, and delete any single-step breakpoints we've set, on non hardware single-step targets. */ =20 static int @@ -4902,15 +4902,15 @@ finish_step_over (struct lwp_info *lwp) =20 lwp->bp_reinsert =3D 0; =20 - /* Delete any software-single-step reinsert breakpoints. No - longer needed. We don't have to worry about other threads - hitting this trap, and later not being able to explain it, - because we were stepping over a breakpoint, and we hold all - threads but LWP stopped while doing that. */ + /* Delete any single-step breakpoints. No longer needed. We + don't have to worry about other threads hitting this trap, + and later not being able to explain it, because we were + stepping over a breakpoint, and we hold all threads but + LWP stopped while doing that. */ if (!can_hardware_single_step ()) { - gdb_assert (has_reinsert_breakpoints (current_thread)); - delete_reinsert_breakpoints (current_thread); + gdb_assert (has_single_step_breakpoints (current_thread)); + delete_single_step_breakpoints (current_thread); } =20 step_over_bkpt =3D null_ptid; @@ -5226,10 +5226,11 @@ proceed_one_lwp (struct inferior_list_entry *entry,= void *except) debug_printf (" stepping LWP %ld, client wants it stepping\n", lwpid_of (thread)); =20 - /* If resume_step is requested by GDB, install reinsert + /* If resume_step is requested by GDB, install single-step breakpoints when the thread is about to be actually resumed if - the reinsert breakpoints weren't removed. */ - if (can_software_single_step () && !has_reinsert_breakpoints (thread= )) + the single-step breakpoints weren't removed. */ + if (can_software_single_step () + && !has_single_step_breakpoints (thread)) install_software_single_step_breakpoints (lwp); =20 step =3D maybe_hw_step (thread); diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c index 7cd037f..a8e12d8 100644 --- a/gdb/gdbserver/mem-break.c +++ b/gdb/gdbserver/mem-break.c @@ -134,7 +134,7 @@ enum bkpt_type gdb_breakpoint_Z4, =20 /* A basic-software-single-step breakpoint. */ - reinsert_breakpoint, + single_step_breakpoint, =20 /* Any other breakpoint type that doesn't require specific treatment goes here. E.g., an event breakpoint. */ @@ -206,9 +206,9 @@ struct other_breakpoint int (*handler) (CORE_ADDR); }; =20 -/* Reinsert breakpoint. */ +/* Breakpoint for single step. */ =20 -struct reinsert_breakpoint +struct single_step_breakpoint { struct breakpoint base; =20 @@ -828,12 +828,12 @@ set_breakpoint (enum bkpt_type type, enum raw_bkpt_ty= pe raw_type, other_bp->handler =3D handler; bp =3D (struct breakpoint *) other_bp; } - else if (type =3D=3D reinsert_breakpoint) + else if (type =3D=3D single_step_breakpoint) { - struct reinsert_breakpoint *reinsert_bp - =3D XCNEW (struct reinsert_breakpoint); + struct single_step_breakpoint *ss_bp + =3D XCNEW (struct single_step_breakpoint); =20 - bp =3D (struct breakpoint *) reinsert_bp; + bp =3D (struct breakpoint *) ss_bp; } else gdb_assert_not_reached ("unhandled breakpoint type"); @@ -1479,19 +1479,19 @@ gdb_breakpoint_here (CORE_ADDR where) } =20 void -set_reinsert_breakpoint (CORE_ADDR stop_at, ptid_t ptid) +set_single_step_breakpoint (CORE_ADDR stop_at, ptid_t ptid) { - struct reinsert_breakpoint *bp; + struct single_step_breakpoint *bp; =20 gdb_assert (ptid_get_pid (current_ptid) =3D=3D ptid_get_pid (ptid)); =20 - bp =3D (struct reinsert_breakpoint *) set_breakpoint_type_at (reinsert_b= reakpoint, - stop_at, NULL); + bp =3D (struct single_step_breakpoint *) set_breakpoint_type_at (single_= step_breakpoint, + stop_at, NULL); bp->ptid =3D ptid; } =20 void -delete_reinsert_breakpoints (struct thread_info *thread) +delete_single_step_breakpoints (struct thread_info *thread) { struct process_info *proc =3D get_thread_process (thread); struct breakpoint *bp, **bp_link; @@ -1501,8 +1501,8 @@ delete_reinsert_breakpoints (struct thread_info *thre= ad) =20 while (bp) { - if (bp->type =3D=3D reinsert_breakpoint - && ptid_equal (((struct reinsert_breakpoint *) bp)->ptid, + if (bp->type =3D=3D single_step_breakpoint + && ptid_equal (((struct single_step_breakpoint *) bp)->ptid, ptid_of (thread))) { struct thread_info *saved_thread =3D current_thread; @@ -1591,15 +1591,15 @@ uninsert_all_breakpoints (void) } =20 void -uninsert_reinsert_breakpoints (struct thread_info *thread) +uninsert_single_step_breakpoints (struct thread_info *thread) { struct process_info *proc =3D get_thread_process (thread); struct breakpoint *bp; =20 for (bp =3D proc->breakpoints; bp !=3D NULL; bp =3D bp->next) { - if (bp->type =3D=3D reinsert_breakpoint - && ptid_equal (((struct reinsert_breakpoint *) bp)->ptid, + if (bp->type =3D=3D single_step_breakpoint + && ptid_equal (((struct single_step_breakpoint *) bp)->ptid, ptid_of (thread))) { gdb_assert (bp->raw->inserted > 0); @@ -1663,7 +1663,7 @@ reinsert_breakpoints_at (CORE_ADDR pc) } =20 int -has_reinsert_breakpoints (struct thread_info *thread) +has_single_step_breakpoints (struct thread_info *thread) { struct process_info *proc =3D get_thread_process (thread); struct breakpoint *bp, **bp_link; @@ -1673,8 +1673,8 @@ has_reinsert_breakpoints (struct thread_info *thread) =20 while (bp) { - if (bp->type =3D=3D reinsert_breakpoint - && ptid_equal (((struct reinsert_breakpoint *) bp)->ptid, + if (bp->type =3D=3D single_step_breakpoint + && ptid_equal (((struct single_step_breakpoint *) bp)->ptid, ptid_of (thread))) return 1; else @@ -1701,15 +1701,15 @@ reinsert_all_breakpoints (void) } =20 void -reinsert_reinsert_breakpoints (struct thread_info *thread) +reinsert_single_step_breakpoints (struct thread_info *thread) { struct process_info *proc =3D get_thread_process (thread); struct breakpoint *bp; =20 for (bp =3D proc->breakpoints; bp !=3D NULL; bp =3D bp->next) { - if (bp->type =3D=3D reinsert_breakpoint - && ptid_equal (((struct reinsert_breakpoint *) bp)->ptid, + if (bp->type =3D=3D single_step_breakpoint + && ptid_equal (((struct single_step_breakpoint *) bp)->ptid, ptid_of (thread))) { gdb_assert (bp->raw->inserted > 0); @@ -1839,13 +1839,13 @@ hardware_breakpoint_inserted_here (CORE_ADDR addr) /* See mem-break.h. */ =20 int -reinsert_breakpoint_inserted_here (CORE_ADDR addr) +single_step_breakpoint_inserted_here (CORE_ADDR addr) { struct process_info *proc =3D current_process (); struct breakpoint *bp; =20 for (bp =3D proc->breakpoints; bp !=3D NULL; bp =3D bp->next) - if (bp->type =3D=3D reinsert_breakpoint + if (bp->type =3D=3D single_step_breakpoint && bp->raw->pc =3D=3D addr && bp->raw->inserted) return 1; @@ -1885,9 +1885,9 @@ delete_disabled_breakpoints (void) next =3D bp->next; if (bp->raw->inserted < 0) { - /* If reinsert_breakpoints become disabled, that means the + /* If single_step_breakpoints become disabled, that means the manipulations (insertion and removal) of them are wrong. */ - gdb_assert (bp->type !=3D reinsert_breakpoint); + gdb_assert (bp->type !=3D single_step_breakpoint); delete_breakpoint_1 (proc, bp); } } @@ -2200,15 +2200,15 @@ clone_one_breakpoint (const struct breakpoint *src,= ptid_t ptid) other_dest->handler =3D ((struct other_breakpoint *) src)->handler; dest =3D (struct breakpoint *) other_dest; } - else if (src->type =3D=3D reinsert_breakpoint) + else if (src->type =3D=3D single_step_breakpoint) { - struct reinsert_breakpoint *reinsert_dest - =3D XCNEW (struct reinsert_breakpoint); + struct single_step_breakpoint *ss_dest + =3D XCNEW (struct single_step_breakpoint); =20 - dest =3D (struct breakpoint *) reinsert_dest; - /* Since reinsert breakpoint is thread specific, don't copy + dest =3D (struct breakpoint *) ss_dest; + /* Since single-step breakpoint is thread specific, don't copy thread id from SRC, use ID instead. */ - reinsert_dest->ptid =3D ptid; + ss_dest->ptid =3D ptid; } else gdb_assert_not_reached ("unhandled breakpoint type"); diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h index 3322ec5..9e7ee93 100644 --- a/gdb/gdbserver/mem-break.h +++ b/gdb/gdbserver/mem-break.h @@ -101,9 +101,9 @@ int software_breakpoint_inserted_here (CORE_ADDR addr); =20 int hardware_breakpoint_inserted_here (CORE_ADDR addr); =20 -/* Returns TRUE if there's any reinsert breakpoint at ADDR. */ +/* Returns TRUE if there's any single-step breakpoint at ADDR. */ =20 -int reinsert_breakpoint_inserted_here (CORE_ADDR addr); +int single_step_breakpoint_inserted_here (CORE_ADDR addr); =20 /* Clear all breakpoint conditions and commands associated with a breakpoint. */ @@ -152,32 +152,32 @@ struct breakpoint *set_breakpoint_at (CORE_ADDR where, =20 int delete_breakpoint (struct breakpoint *bkpt); =20 -/* Set a reinsert breakpoint at STOP_AT for thread represented by +/* Set a single-step breakpoint at STOP_AT for thread represented by PTID. */ =20 -void set_reinsert_breakpoint (CORE_ADDR stop_at, ptid_t ptid); +void set_single_step_breakpoint (CORE_ADDR stop_at, ptid_t ptid); =20 -/* Delete all reinsert breakpoints of THREAD. */ +/* Delete all single-step breakpoints of THREAD. */ =20 -void delete_reinsert_breakpoints (struct thread_info *thread); +void delete_single_step_breakpoints (struct thread_info *thread); =20 -/* Reinsert all reinsert breakpoints of THREAD. */ +/* Reinsert all single-step breakpoints of THREAD. */ =20 -void reinsert_reinsert_breakpoints (struct thread_info *thread); +void reinsert_single_step_breakpoints (struct thread_info *thread); =20 -/* Uninsert all reinsert breakpoints of THREAD. This still leaves - the reinsert breakpoints in the table. */ +/* Uninsert all single-step breakpoints of THREAD. This still leaves + the single-step breakpoints in the table. */ =20 -void uninsert_reinsert_breakpoints (struct thread_info *thread); +void uninsert_single_step_breakpoints (struct thread_info *thread); =20 /* Reinsert breakpoints at WHERE (and change their status to inserted). */ =20 void reinsert_breakpoints_at (CORE_ADDR where); =20 -/* The THREAD has reinsert breakpoints or not. */ +/* The THREAD has single-step breakpoints or not. */ =20 -int has_reinsert_breakpoints (struct thread_info *thread); +int has_single_step_breakpoints (struct thread_info *thread); =20 /* Uninsert breakpoints at WHERE (and change their status to uninserted). This still leaves the breakpoints in the table. */ --=20 1.9.1