From patchwork Tue Aug 22 03:55:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 22300 Received: (qmail 980 invoked by alias); 22 Aug 2017 03:55:50 -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 952 invoked by uid 89); 22 Aug 2017 03:55:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=momentary, how's, bget, hows X-HELO: gproxy9-pub.mail.unifiedlayer.com Received: from gproxy9-pub.mail.unifiedlayer.com (HELO gproxy9-pub.mail.unifiedlayer.com) (69.89.20.122) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 22 Aug 2017 03:55:47 +0000 Received: from cmgw2 (unknown [10.0.90.83]) by gproxy9.mail.unifiedlayer.com (Postfix) with ESMTP id B53311E0607 for ; Mon, 21 Aug 2017 21:55:44 -0600 (MDT) Received: from box522.bluehost.com ([74.220.219.122]) by cmgw2 with id 03vh1w00S2f2jeq013vkqA; Mon, 21 Aug 2017 21:55:44 -0600 X-Authority-Analysis: v=2.2 cv=T7z8d7CQ c=1 sm=1 tr=0 a=GsOEXm/OWkKvwdLVJsfwcA==:117 a=GsOEXm/OWkKvwdLVJsfwcA==:17 a=KeKAF7QvOSUA:10 a=20KFwNOVAAAA:8 a=zstS-IiYAAAA:8 a=6I5d2MoRAAAA:8 a=ZYI8HJPr-dStd5q5UYkA:9 a=OrGvTv6wRj8-QAbw:21 a=mBwzwVKk9mhieqEy:21 a=4G6NA9xxw8l3yy4pmD5M:22 a=IjZwj45LgO3ly-622nXo:22 Received: from 75-166-24-97.hlrn.qwest.net ([75.166.24.97]:53182 helo=bapiya) by box522.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.87) (envelope-from ) id 1dk0I1-00094k-6n; Mon, 21 Aug 2017 21:55:41 -0600 From: Tom Tromey To: Pedro Alves Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [RFA 2/2] Change install_breakpoint to take a std::unique_ptr References: <20170820135402.1213-1-tom@tromey.com> <20170820135402.1213-3-tom@tromey.com> Date: Mon, 21 Aug 2017 21:55:33 -0600 In-Reply-To: (Pedro Alves's message of "Mon, 21 Aug 2017 11:12:35 +0100") Message-ID: <877exwjlxm.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 X-BWhitelist: no X-Exim-ID: 1dk0I1-00094k-6n X-Source-Sender: 75-166-24-97.hlrn.qwest.net (bapiya) [75.166.24.97]:53182 X-Source-Auth: tom+tromey.com X-Email-Count: 2 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTIyLmJsdWVob3N0LmNvbQ== X-Local-Domain: yes >>>>> "Pedro" == Pedro Alves writes: [...] Pedro> or change add_to_breakpoint_chain to take an rvalue-ref unique_ptr Pedro> too, and return the raw breakpoint, and then write here: Pedro> breakpoint *b = add_to_breakpoint_chain (std::move (arg)); Sounds good. I actually considered this but thought maybe it was a step too far... hah. How's this? Tom commit e13a6f18770ff6dc329202e68711b5b34fc1c897 Author: Tom Tromey Date: Sat Aug 19 22:26:20 2017 -0600 Change install_breakpoint to take a std::unique_ptr This changes install_breakpoint to take a std::unique_ptr rvalue-ref argument. This makes it clear that install_breakpoint takes ownership of the pointer, and prevents bugs like the one fixed by the previous patch. ChangeLog 2017-08-21 Tom Tromey * breakpoint.h (install_breakpoint): Update. * breakpoint.c (add_solib_catchpoint): Update. (install_breakpoint): Change argument to a std::unique_ptr. (create_fork_vfork_event_catchpoint): Use std::unique_ptr. (create_breakpoint_sal, create_breakpoint): Update. (watch_command_1, catch_exec_command_1) (strace_marker_create_breakpoints_sal): Use std::unique_ptr. (add_to_breakpoint_chain): Change argument to a std::unique_ptr. Return the breakpoint. (set_raw_breakpoint_without_location, set_raw_breakpoint) (new_single_step_breakpoint): Update. * break-catch-throw.c (handle_gnu_v3_exceptions): Use std::unique_ptr. * break-catch-syscall.c (create_syscall_event_catchpoint): Use std::unique_ptr. * break-catch-sig.c (create_signal_catchpoint): Use std::unique_ptr. * ada-lang.c (create_ada_exception_catchpoint): Use std::unique_ptr. diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b9de079..a7f13d4 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,27 @@ 2017-08-21 Tom Tromey + * breakpoint.h (install_breakpoint): Update. + * breakpoint.c (add_solib_catchpoint): Update. + (install_breakpoint): Change argument to a std::unique_ptr. + (create_fork_vfork_event_catchpoint): Use std::unique_ptr. + (create_breakpoint_sal, create_breakpoint): Update. + (watch_command_1, catch_exec_command_1) + (strace_marker_create_breakpoints_sal): Use std::unique_ptr. + (add_to_breakpoint_chain): Change argument to a std::unique_ptr. + Return the breakpoint. + (set_raw_breakpoint_without_location, set_raw_breakpoint) + (new_single_step_breakpoint): Update. + * break-catch-throw.c (handle_gnu_v3_exceptions): Use + std::unique_ptr. + * break-catch-syscall.c (create_syscall_event_catchpoint): Use + std::unique_ptr. + * break-catch-sig.c (create_signal_catchpoint): Use + std::unique_ptr. + * ada-lang.c (create_ada_exception_catchpoint): Use + std::unique_ptr. + +2017-08-21 Tom Tromey + * breakpoint.c (add_solib_catchpoint): Use std::unique_ptr. 2017-08-21 John Baldwin diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 280247b..476f700 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -13010,20 +13010,19 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch, int disabled, int from_tty) { - struct ada_catchpoint *c; char *addr_string = NULL; const struct breakpoint_ops *ops = NULL; struct symtab_and_line sal = ada_exception_sal (ex_kind, excep_string, &addr_string, &ops); - c = new ada_catchpoint (); - init_ada_exception_breakpoint (c, gdbarch, sal, addr_string, + std::unique_ptr c (new ada_catchpoint ()); + init_ada_exception_breakpoint (c.get (), gdbarch, sal, addr_string, ops, tempflag, disabled, from_tty); c->excep_string = excep_string; - create_excep_cond_exprs (c); + create_excep_cond_exprs (c.get ()); if (cond_string != NULL) - set_breakpoint_condition (c, cond_string, from_tty); - install_breakpoint (0, c, 1); + set_breakpoint_condition (c.get (), cond_string, from_tty); + install_breakpoint (0, std::move (c), 1); } /* Implement the "catch exception" command. */ diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c index 98888c9..9b8cf64 100644 --- a/gdb/break-catch-sig.c +++ b/gdb/break-catch-sig.c @@ -317,15 +317,14 @@ static void create_signal_catchpoint (int tempflag, std::vector &&filter, bool catch_all) { - struct signal_catchpoint *c; struct gdbarch *gdbarch = get_current_arch (); - c = new signal_catchpoint (); - init_catchpoint (c, gdbarch, tempflag, NULL, &signal_catchpoint_ops); + std::unique_ptr c (new signal_catchpoint ()); + init_catchpoint (c.get (), gdbarch, tempflag, NULL, &signal_catchpoint_ops); c->signals_to_be_caught = std::move (filter); c->catch_all = catch_all; - install_breakpoint (0, c, 1); + install_breakpoint (0, std::move (c), 1); } diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c index 701645e..1be29be 100644 --- a/gdb/break-catch-syscall.c +++ b/gdb/break-catch-syscall.c @@ -370,14 +370,13 @@ static void create_syscall_event_catchpoint (int tempflag, std::vector &&filter, const struct breakpoint_ops *ops) { - struct syscall_catchpoint *c; struct gdbarch *gdbarch = get_current_arch (); - c = new syscall_catchpoint (); - init_catchpoint (c, gdbarch, tempflag, NULL, ops); + std::unique_ptr c (new syscall_catchpoint ()); + init_catchpoint (c.get (), gdbarch, tempflag, NULL, ops); c->syscalls_to_be_caught = std::move (filter); - install_breakpoint (0, c, 1); + install_breakpoint (0, std::move (c), 1); } /* Splits the argument using space as delimiter. */ diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c index 5318e5f..b5322fc 100644 --- a/gdb/break-catch-throw.c +++ b/gdb/break-catch-throw.c @@ -387,8 +387,7 @@ handle_gnu_v3_exceptions (int tempflag, std::string &&except_rx, re_set_exception_catchpoint (cp.get ()); - install_breakpoint (0, cp.get (), 1); - cp.release (); + install_breakpoint (0, std::move (cp), 1); } /* Look for an "if" token in *STRING. The "if" token must be preceded diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 135741a..334c76a 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -7402,23 +7402,26 @@ decref_bp_location (struct bp_location **blp) /* Add breakpoint B at the end of the global breakpoint chain. */ -static void -add_to_breakpoint_chain (struct breakpoint *b) +static breakpoint * +add_to_breakpoint_chain (std::unique_ptr &&b) { struct breakpoint *b1; + struct breakpoint *result = b.get (); /* Add this breakpoint to the end of the chain so that a list of breakpoints will come out in order of increasing numbers. */ b1 = breakpoint_chain; if (b1 == 0) - breakpoint_chain = b; + breakpoint_chain = b.release (); else { while (b1->next) b1 = b1->next; - b1->next = b; + b1->next = b.release (); } + + return result; } /* Initializes breakpoint B with type BPTYPE and no locations yet. */ @@ -7450,9 +7453,7 @@ set_raw_breakpoint_without_location (struct gdbarch *gdbarch, std::unique_ptr b = new_breakpoint_from_type (bptype); init_raw_breakpoint_without_location (b.get (), gdbarch, bptype, ops); - add_to_breakpoint_chain (b.get ()); - - return b.release (); + return add_to_breakpoint_chain (std::move (b)); } /* Initialize loc->function_name. EXPLICIT_LOC says no indirect function @@ -7567,9 +7568,7 @@ set_raw_breakpoint (struct gdbarch *gdbarch, std::unique_ptr b = new_breakpoint_from_type (bptype); init_raw_breakpoint (b.get (), gdbarch, sal, bptype, ops); - add_to_breakpoint_chain (b.get ()); - - return b.release (); + return add_to_breakpoint_chain (std::move (b)); } /* Call this routine when stepping and nexting to enable a breakpoint @@ -8485,7 +8484,7 @@ add_solib_catchpoint (const char *arg, int is_load, int is_temp, int enabled) c->enable_state = enabled ? bp_enabled : bp_disabled; - install_breakpoint (0, c.release (), 1); + install_breakpoint (0, std::move (c), 1); } /* A helper function that does all the work for "catch load" and @@ -8540,9 +8539,9 @@ init_catchpoint (struct breakpoint *b, } void -install_breakpoint (int internal, struct breakpoint *b, int update_gll) +install_breakpoint (int internal, std::unique_ptr &&arg, int update_gll) { - add_to_breakpoint_chain (b); + breakpoint *b = add_to_breakpoint_chain (std::move (arg)); set_breakpoint_number (internal, b); if (is_tracepoint (b)) set_tracepoint_count (breakpoint_count); @@ -8559,13 +8558,13 @@ create_fork_vfork_event_catchpoint (struct gdbarch *gdbarch, int tempflag, const char *cond_string, const struct breakpoint_ops *ops) { - struct fork_catchpoint *c = new fork_catchpoint (); + std::unique_ptr c (new fork_catchpoint ()); - init_catchpoint (c, gdbarch, tempflag, cond_string, ops); + init_catchpoint (c.get (), gdbarch, tempflag, cond_string, ops); c->forked_inferior_pid = null_ptid; - install_breakpoint (0, c, 1); + install_breakpoint (0, std::move (c), 1); } /* Exec catchpoints. */ @@ -8810,9 +8809,9 @@ enable_breakpoints_after_startup (void) static struct breakpoint * new_single_step_breakpoint (int thread, struct gdbarch *gdbarch) { - struct breakpoint *b = new breakpoint (); + std::unique_ptr b (new breakpoint ()); - init_raw_breakpoint_without_location (b, gdbarch, bp_single_step, + init_raw_breakpoint_without_location (b.get (), gdbarch, bp_single_step, &momentary_breakpoint_ops); b->disposition = disp_donttouch; @@ -8821,9 +8820,7 @@ new_single_step_breakpoint (int thread, struct gdbarch *gdbarch) b->thread = thread; gdb_assert (b->thread != 0); - add_to_breakpoint_chain (b); - - return b; + return add_to_breakpoint_chain (std::move (b)); } /* Set a momentary breakpoint of type TYPE at address specified by @@ -9308,7 +9305,7 @@ create_breakpoint_sal (struct gdbarch *gdbarch, enabled, internal, flags, display_canonical); - install_breakpoint (internal, b.release (), 0); + install_breakpoint (internal, std::move (b), 0); } /* Add SALS.nelts breakpoints to the breakpoint table. For each @@ -9798,7 +9795,7 @@ create_breakpoint (struct gdbarch *gdbarch, && type_wanted != bp_hardware_breakpoint) || thread != -1) b->pspace = current_program_space; - install_breakpoint (internal, b.release (), 0); + install_breakpoint (internal, std::move (b), 0); } if (VEC_length (linespec_sals, canonical.sals) > 1) @@ -10961,7 +10958,6 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, the hardware watchpoint. */ int use_mask = 0; CORE_ADDR mask = 0; - struct watchpoint *w; char *expression; struct cleanup *back_to; @@ -11182,13 +11178,13 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, else bp_type = bp_hardware_watchpoint; - w = new watchpoint (); + std::unique_ptr w (new watchpoint ()); if (use_mask) - init_raw_breakpoint_without_location (w, NULL, bp_type, + init_raw_breakpoint_without_location (w.get (), NULL, bp_type, &masked_watchpoint_breakpoint_ops); else - init_raw_breakpoint_without_location (w, NULL, bp_type, + init_raw_breakpoint_without_location (w.get (), NULL, bp_type, &watchpoint_breakpoint_ops); w->thread = thread; w->disposition = disp_donttouch; @@ -11243,26 +11239,17 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, /* The scope breakpoint is related to the watchpoint. We will need to act on them together. */ w->related_breakpoint = scope_breakpoint; - scope_breakpoint->related_breakpoint = w; + scope_breakpoint->related_breakpoint = w.get (); } if (!just_location) value_free_to_mark (mark); - TRY - { - /* Finally update the new watchpoint. This creates the locations - that should be inserted. */ - update_watchpoint (w, 1); - } - CATCH (e, RETURN_MASK_ALL) - { - delete_breakpoint (w); - throw_exception (e); - } - END_CATCH + /* Finally update the new watchpoint. This creates the locations + that should be inserted. */ + update_watchpoint (w.get (), 1); - install_breakpoint (internal, w, 1); + install_breakpoint (internal, std::move (w), 1); do_cleanups (back_to); } @@ -11710,7 +11697,6 @@ catch_exec_command_1 (char *arg_entry, int from_tty, struct cmd_list_element *command) { const char *arg = arg_entry; - struct exec_catchpoint *c; struct gdbarch *gdbarch = get_current_arch (); int tempflag; const char *cond_string = NULL; @@ -11731,12 +11717,12 @@ catch_exec_command_1 (char *arg_entry, int from_tty, if ((*arg != '\0') && !isspace (*arg)) error (_("Junk at end of arguments.")); - c = new exec_catchpoint (); - init_catchpoint (c, gdbarch, tempflag, cond_string, + std::unique_ptr c (new exec_catchpoint ()); + init_catchpoint (c.get (), gdbarch, tempflag, cond_string, &catch_exec_breakpoint_ops); c->exec_pathname = NULL; - install_breakpoint (0, c, 1); + install_breakpoint (0, std::move (c), 1); } void @@ -13559,7 +13545,6 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch, for (i = 0; i < lsal->sals.nelts; ++i) { struct symtabs_and_lines expanded; - struct tracepoint *tp; event_location_up location; expanded.nelts = 1; @@ -13567,8 +13552,8 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch, location = copy_event_location (canonical->location.get ()); - tp = new tracepoint (); - init_breakpoint_sal (tp, gdbarch, expanded, + std::unique_ptr tp (new tracepoint ()); + init_breakpoint_sal (tp.get (), gdbarch, expanded, std::move (location), NULL, std::move (cond_string), std::move (extra_string), @@ -13584,7 +13569,7 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch, corresponds to this one */ tp->static_trace_marker_id_idx = i; - install_breakpoint (internal, tp, 0); + install_breakpoint (internal, std::move (tp), 0); } } diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index d955184..dc2b098 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1296,7 +1296,7 @@ extern void init_catchpoint (struct breakpoint *b, the internal breakpoint count. If UPDATE_GLL is non-zero, update_global_location_list will be called. */ -extern void install_breakpoint (int internal, struct breakpoint *b, +extern void install_breakpoint (int internal, std::unique_ptr &&b, int update_gll); /* Flags that can be passed down to create_breakpoint, etc., to affect