From b6433ede0708af00be520abdf9209cd776aab2e2 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 11 Dec 2020 09:21:53 -0700 Subject: [PATCH] Make bp_location derive from refcounted_object This changes bp_location to derive from refcounted_object, introduces a ref_ptr specialization for this type, and then changes bpstats::bp_location_at to use that specialization. This removes some manual reference counting and simplifies the code. gdb/ChangeLog 2020-12-11 Tom Tromey * inline-frame.c (stopped_by_user_bp_inline_frame): Update. * ada-lang.c (check_status_exception): Update. * breakpoint.c (free_bp_location): Remove. (decref_bp_location): Use bp_location_ref_policy. (bpstats::bpstats): Don't call incref_bp_location. (bpstats::~bpstats): Remove. (bpstats::bpstats): Update. (bpstat_check_watchpoint, bpstat_check_breakpoint_conditions) (bp_location::bp_location): Update. (incref_bp_location): Remove. (bkpt_print_it): Update. * breakpoint.h (class bp_location): Derive from refcounted_object. (struct bpstats): Remove destructor. : Now a bp_location_ref_ptr. : Remove. (bp_location_ref_ptr): New typedef. (struct bp_location_ref_policy): New. --- gdb/ChangeLog | 21 +++++++++++++++++++++ gdb/ada-lang.c | 2 +- gdb/breakpoint.c | 43 ++++++------------------------------------- gdb/breakpoint.h | 30 ++++++++++++++++++++++++------ gdb/inline-frame.c | 2 +- 5 files changed, 53 insertions(+), 45 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 691e583578..61997f6e17 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,24 @@ +2020-12-11 Tom Tromey + + * inline-frame.c (stopped_by_user_bp_inline_frame): Update. + * ada-lang.c (check_status_exception): Update. + * breakpoint.c (free_bp_location): Remove. + (decref_bp_location): Use bp_location_ref_policy. + (bpstats::bpstats): Don't call incref_bp_location. + (bpstats::~bpstats): Remove. + (bpstats::bpstats): Update. + (bpstat_check_watchpoint, bpstat_check_breakpoint_conditions) + (bp_location::bp_location): Update. + (incref_bp_location): Remove. + (bkpt_print_it): Update. + * breakpoint.h (class bp_location): Derive from + refcounted_object. + (struct bpstats): Remove destructor. + : Now a bp_location_ref_ptr. + : Remove. + (bp_location_ref_ptr): New typedef. + (struct bp_location_ref_policy): New. + 2020-12-11 Tom Tromey * thread.c (class scoped_inc_dec_ref): Remove. diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index 39546de41b..b41d2bfc61 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -12255,7 +12255,7 @@ should_stop_exception (const struct bp_location *bl) static void check_status_exception (bpstat bs) { - bs->stop = should_stop_exception (bs->bp_location_at); + bs->stop = should_stop_exception (bs->bp_location_at.get ()); } /* Implement the PRINT_IT method in the breakpoint_ops structure diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index baf80b6b19..933fe90a7b 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -172,8 +172,6 @@ static int hw_watchpoint_used_count_others (struct breakpoint *except, static void enable_breakpoint_disp (struct breakpoint *, enum bpdisp, int count); -static void free_bp_location (struct bp_location *loc); -static void incref_bp_location (struct bp_location *loc); static void decref_bp_location (struct bp_location **loc); static struct bp_location *allocate_bp_location (struct breakpoint *bpt); @@ -4242,15 +4240,6 @@ is_catchpoint (struct breakpoint *b) return (b->type == bp_catchpoint); } -/* Frees any storage that is part of a bpstat. Does not walk the - 'next' chain. */ - -bpstats::~bpstats () -{ - if (bp_location_at != NULL) - decref_bp_location (&bp_location_at); -} - /* Clear a bpstat so that it says we are not at any breakpoint. Also free any storage that is part of a bpstat. */ @@ -4283,7 +4272,6 @@ bpstats::bpstats (const bpstats &other) { if (other.old_val != NULL) old_val = release_value (value_copy (other.old_val.get ())); - incref_bp_location (bp_location_at); } /* Return a copy of a bpstat. Like "bs1 = bs2" but all storage that @@ -4768,21 +4756,19 @@ breakpoint_cond_eval (expression *exp) bpstats::bpstats (struct bp_location *bl, bpstat **bs_link_pointer) : next (NULL), - bp_location_at (bl), + bp_location_at (bp_location_ref_ptr::new_reference (bl)), breakpoint_at (bl->owner), commands (NULL), print (0), stop (0), print_it (print_it_normal) { - incref_bp_location (bl); **bs_link_pointer = this; *bs_link_pointer = &next; } bpstats::bpstats () : next (NULL), - bp_location_at (NULL), breakpoint_at (NULL), commands (NULL), print (0), @@ -5060,7 +5046,7 @@ bpstat_check_watchpoint (bpstat bs) struct watchpoint *b; /* BS is built for existing struct breakpoint. */ - bl = bs->bp_location_at; + bl = bs->bp_location_at.get (); gdb_assert (bl != NULL); b = (struct watchpoint *) bs->breakpoint_at; gdb_assert (b != NULL); @@ -5236,7 +5222,7 @@ bpstat_check_breakpoint_conditions (bpstat bs, thread_info *thread) gdb_assert (bs->stop); /* BS is built for existing struct breakpoint. */ - bl = bs->bp_location_at; + bl = bs->bp_location_at.get (); gdb_assert (bl != NULL); b = bs->breakpoint_at; gdb_assert (b != NULL); @@ -7101,7 +7087,7 @@ bp_location::bp_location (breakpoint *owner, bp_loc_type type) || this->loc_type == bp_loc_hardware_breakpoint) mark_breakpoint_location_modified (this); - this->refc = 1; + incref (); } bp_location::bp_location (breakpoint *owner) @@ -7118,30 +7104,13 @@ allocate_bp_location (struct breakpoint *bpt) return bpt->ops->allocate_location (bpt); } -static void -free_bp_location (struct bp_location *loc) -{ - delete loc; -} - -/* Increment reference count. */ - -static void -incref_bp_location (struct bp_location *bl) -{ - ++bl->refc; -} - /* Decrement reference count. If the reference count reaches 0, destroy the bp_location. Sets *BLP to NULL. */ static void decref_bp_location (struct bp_location **blp) { - gdb_assert ((*blp)->refc > 0); - - if (--(*blp)->refc == 0) - free_bp_location (*blp); + bp_location_ref_policy::decref (*blp); *blp = NULL; } @@ -12680,7 +12649,7 @@ bkpt_print_it (bpstat bs) gdb_assert (bs->bp_location_at != NULL); - bl = bs->bp_location_at; + bl = bs->bp_location_at.get (); b = bs->breakpoint_at; bp_temp = b->disposition == disp_del; diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 4a65dd2dd4..2859552c52 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -29,6 +29,7 @@ #include #include "gdbsupport/array-view.h" #include "gdbsupport/function-view.h" +#include "gdbsupport/refcounted-object.h" #include "cli/cli-script.h" struct block; @@ -311,7 +312,7 @@ enum bp_loc_type bp_loc_other /* Miscellaneous... */ }; -class bp_location +class bp_location : public refcounted_object { public: bp_location () = default; @@ -329,9 +330,6 @@ public: the same parent breakpoint. */ bp_location *next = NULL; - /* The reference count. */ - int refc = 0; - /* Type of this breakpoint location. */ bp_loc_type loc_type {}; @@ -510,6 +508,27 @@ public: const struct objfile *objfile = NULL; }; +/* A policy class for bp_location reference counting. */ +struct bp_location_ref_policy +{ + static void incref (bp_location *loc) + { + loc->incref (); + } + + static void decref (bp_location *loc) + { + gdb_assert (loc->refcount () > 0); + loc->decref (); + if (loc->refcount () == 0) + delete loc; + } +}; + +/* A gdb::ref_ptr that has been specialized for bp_location. */ +typedef gdb::ref_ptr + bp_location_ref_ptr; + /* The possible return values for print_bpstat, print_it_normal, print_it_done, print_it_noop. */ enum print_stop_action @@ -1130,7 +1149,6 @@ struct bpstats { bpstats (); bpstats (struct bp_location *bl, bpstat **bs_link_pointer); - ~bpstats (); bpstats (const bpstats &); bpstats &operator= (const bpstats &) = delete; @@ -1155,7 +1173,7 @@ struct bpstats What this means is that we should not (in most cases) follow the `bpstat->bp_location->owner' link, but instead use the `breakpoint_at' field below. */ - struct bp_location *bp_location_at; + bp_location_ref_ptr bp_location_at; /* Breakpoint that caused the stop. This is nullified if the breakpoint ends up being deleted. See comments on diff --git a/gdb/inline-frame.c b/gdb/inline-frame.c index 92a7d562ea..439f3633fd 100644 --- a/gdb/inline-frame.c +++ b/gdb/inline-frame.c @@ -313,7 +313,7 @@ stopped_by_user_bp_inline_frame (const block *frame_block, bpstat stop_chain) if (bpt != NULL && (user_breakpoint_p (bpt) || bpt->type == bp_until)) { - bp_location *loc = s->bp_location_at; + bp_location *loc = s->bp_location_at.get (); enum bp_loc_type t = loc->loc_type; if (t == bp_loc_software_breakpoint -- 2.34.1