* [RFA 0/2] small clean ups in breakpoint allocation @ 2017-08-20 13:54 Tom Tromey 2017-08-20 14:06 ` [RFA 1/2] Fix erroneous cleanup use in add_solib_catchpoint Tom Tromey 2017-08-20 14:06 ` [RFA 2/2] Change install_breakpoint to take a std::unique_ptr Tom Tromey 0 siblings, 2 replies; 6+ messages in thread From: Tom Tromey @ 2017-08-20 13:54 UTC (permalink / raw) To: gdb-patches This short series fixes up breakpoint allocation a bit. The first patch fixes a real bug I happened across by accident -- a new/free discrepancy. The second patch aims to make these bugs much less likely in the future. Regression tested on the buildbot. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFA 1/2] Fix erroneous cleanup use in add_solib_catchpoint 2017-08-20 13:54 [RFA 0/2] small clean ups in breakpoint allocation Tom Tromey @ 2017-08-20 14:06 ` Tom Tromey 2017-08-20 14:06 ` [RFA 2/2] Change install_breakpoint to take a std::unique_ptr Tom Tromey 1 sibling, 0 replies; 6+ messages in thread From: Tom Tromey @ 2017-08-20 14:06 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey I happened to notice that add_solib_catchpoint allocated the new catchpoint with "new" but installed a cleanup using "xfree". This patch fixes the bug by changing the function to use std::unique_ptr instead. gdb/ChangeLog 2017-08-19 Tom Tromey <tom@tromey.com> * breakpoint.c (add_solib_catchpoint): Use std::unique_ptr. --- gdb/ChangeLog | 4 ++++ gdb/breakpoint.c | 10 +++------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 7ad3f4c..339ab3a 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,7 @@ +2017-08-19 Tom Tromey <tom@tromey.com> + + * breakpoint.c (add_solib_catchpoint): Use std::unique_ptr. + 2017-08-18 Tom Tromey <tom@tromey.com> Pedro Alves <palves@redhat.com> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index bc681cf..135741a 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8464,16 +8464,13 @@ static struct breakpoint_ops catch_solib_breakpoint_ops; void add_solib_catchpoint (const char *arg, int is_load, int is_temp, int enabled) { - struct solib_catchpoint *c; struct gdbarch *gdbarch = get_current_arch (); - struct cleanup *cleanup; if (!arg) arg = ""; arg = skip_spaces_const (arg); - c = new solib_catchpoint (); - cleanup = make_cleanup (xfree, c); + std::unique_ptr<solib_catchpoint> c (new solib_catchpoint ()); if (*arg != '\0') { @@ -8483,13 +8480,12 @@ add_solib_catchpoint (const char *arg, int is_load, int is_temp, int enabled) } c->is_load = is_load; - init_catchpoint (c, gdbarch, is_temp, NULL, + init_catchpoint (c.get (), gdbarch, is_temp, NULL, &catch_solib_breakpoint_ops); c->enable_state = enabled ? bp_enabled : bp_disabled; - discard_cleanups (cleanup); - install_breakpoint (0, c, 1); + install_breakpoint (0, c.release (), 1); } /* A helper function that does all the work for "catch load" and -- 2.9.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFA 2/2] Change install_breakpoint to take a std::unique_ptr 2017-08-20 13:54 [RFA 0/2] small clean ups in breakpoint allocation Tom Tromey 2017-08-20 14:06 ` [RFA 1/2] Fix erroneous cleanup use in add_solib_catchpoint Tom Tromey @ 2017-08-20 14:06 ` Tom Tromey 2017-08-21 10:12 ` Pedro Alves 1 sibling, 1 reply; 6+ messages in thread From: Tom Tromey @ 2017-08-20 14:06 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey 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. gdb/ChangeLog 2017-08-19 Tom Tromey <tom@tromey.com> * 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. * 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. --- gdb/ChangeLog | 18 +++++++++++++++ gdb/ada-lang.c | 11 +++++----- gdb/break-catch-sig.c | 7 +++--- gdb/break-catch-syscall.c | 7 +++--- gdb/break-catch-throw.c | 3 +-- gdb/breakpoint.c | 56 +++++++++++++++++++---------------------------- gdb/breakpoint.h | 2 +- 7 files changed, 54 insertions(+), 50 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 339ab3a..8f0a22f 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,23 @@ 2017-08-19 Tom Tromey <tom@tromey.com> + * 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. + * 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-19 Tom Tromey <tom@tromey.com> + * breakpoint.c (add_solib_catchpoint): Use std::unique_ptr. 2017-08-18 Tom Tromey <tom@tromey.com> 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<ada_catchpoint> 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<gdb_signal> &&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<signal_catchpoint> 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<int> &&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<syscall_catchpoint> 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..b0881ec 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -8485,7 +8485,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,8 +8540,10 @@ init_catchpoint (struct breakpoint *b, } void -install_breakpoint (int internal, struct breakpoint *b, int update_gll) +install_breakpoint (int internal, std::unique_ptr<breakpoint> &&arg, int update_gll) { + breakpoint *b = arg.release (); + add_to_breakpoint_chain (b); set_breakpoint_number (internal, b); if (is_tracepoint (b)) @@ -8559,13 +8561,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<fork_catchpoint> 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. */ @@ -9308,7 +9310,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 +9800,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 +10963,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 +11183,13 @@ watch_command_1 (const char *arg, int accessflag, int from_tty, else bp_type = bp_hardware_watchpoint; - w = new watchpoint (); + std::unique_ptr<watchpoint> 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 +11244,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 +11702,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 +11722,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<exec_catchpoint> 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 +13550,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 +13557,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<tracepoint> tp (new tracepoint ()); + init_breakpoint_sal (tp.get (), gdbarch, expanded, std::move (location), NULL, std::move (cond_string), std::move (extra_string), @@ -13584,7 +13574,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<breakpoint> &&b, int update_gll); /* Flags that can be passed down to create_breakpoint, etc., to affect -- 2.9.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 2/2] Change install_breakpoint to take a std::unique_ptr 2017-08-20 14:06 ` [RFA 2/2] Change install_breakpoint to take a std::unique_ptr Tom Tromey @ 2017-08-21 10:12 ` Pedro Alves 2017-08-22 3:55 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Pedro Alves @ 2017-08-21 10:12 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 08/20/2017 02:54 PM, Tom Tromey wrote: > @@ -8540,8 +8540,10 @@ init_catchpoint (struct breakpoint *b, > } > > void > -install_breakpoint (int internal, struct breakpoint *b, int update_gll) > +install_breakpoint (int internal, std::unique_ptr<breakpoint> &&arg, int update_gll) > { > + breakpoint *b = arg.release (); > + > add_to_breakpoint_chain (b); I think we should defer .release() until the ownership transfer actually happened, for stronger exception safety. I.e., either: add_to_breakpoint_chain (arg.get ()); breakpoint *b = arg.release (); or change add_to_breakpoint_chain to take an rvalue-ref unique_ptr too, and return the raw breakpoint, and then write here: breakpoint *b = add_to_breakpoint_chain (std::move (arg)); Some spots were/are already only doing the release after adding the breakpoint to the chain, but not all. See e.g., set_raw_breakpoint. The latter approach is my preferred for enforcing the pattern. Otherwise both patches LGTM. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 2/2] Change install_breakpoint to take a std::unique_ptr 2017-08-21 10:12 ` Pedro Alves @ 2017-08-22 3:55 ` Tom Tromey 2017-08-22 8:56 ` Pedro Alves 0 siblings, 1 reply; 6+ messages in thread From: Tom Tromey @ 2017-08-22 3:55 UTC (permalink / raw) To: Pedro Alves; +Cc: Tom Tromey, gdb-patches >>>>> "Pedro" == Pedro Alves <palves@redhat.com> 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 <tom@tromey.com> 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 <tom@tromey.com> * 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 <tom@tromey.com> + * 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 <tom@tromey.com> + * breakpoint.c (add_solib_catchpoint): Use std::unique_ptr. 2017-08-21 John Baldwin <jhb@FreeBSD.org> 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<ada_catchpoint> 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<gdb_signal> &&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<signal_catchpoint> 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<int> &&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<syscall_catchpoint> 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<breakpoint> &&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<breakpoint> 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<breakpoint> 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<breakpoint> &&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<fork_catchpoint> 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<breakpoint> 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<watchpoint> 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<exec_catchpoint> 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<tracepoint> 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<breakpoint> &&b, int update_gll); /* Flags that can be passed down to create_breakpoint, etc., to affect ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA 2/2] Change install_breakpoint to take a std::unique_ptr 2017-08-22 3:55 ` Tom Tromey @ 2017-08-22 8:56 ` Pedro Alves 0 siblings, 0 replies; 6+ messages in thread From: Pedro Alves @ 2017-08-22 8:56 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On 08/22/2017 04:55 AM, Tom Tromey wrote: >>>>>> "Pedro" == Pedro Alves <palves@redhat.com> 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? Perfect. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-22 8:56 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-08-20 13:54 [RFA 0/2] small clean ups in breakpoint allocation Tom Tromey 2017-08-20 14:06 ` [RFA 1/2] Fix erroneous cleanup use in add_solib_catchpoint Tom Tromey 2017-08-20 14:06 ` [RFA 2/2] Change install_breakpoint to take a std::unique_ptr Tom Tromey 2017-08-21 10:12 ` Pedro Alves 2017-08-22 3:55 ` Tom Tromey 2017-08-22 8:56 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox