From patchwork Thu May 11 14:48:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 69157 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D05823853576 for ; Thu, 11 May 2023 14:49:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D05823853576 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1683816588; bh=a4LLMZQXnnUdhXZfoUEofiOmAUh1KrUB217Auhi+4lA=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=cLhLEp3Sfu15jou+DKQp9FrPhyPQuamI7kX0GNYOb+xdX+QqH+DQq0XRuvt4V+rWK 7jgbROlx/g0tvD1ejsDcFcsOU6rqUhjiAUDz9zodBDhVZME6RUVaHQJh4C3x+Blkgu veUVQNWnlaoLBPXdDVhDZde9Ei8BowKZ5NLpngaw= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id C2E743858434 for ; Thu, 11 May 2023 14:48:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C2E743858434 Received: from smarchi-efficios.internal.efficios.com (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 6312C1E121; Thu, 11 May 2023 10:48:34 -0400 (EDT) To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 04/12] gdb: add breakpoint "has locations" methods Date: Thu, 11 May 2023 10:48:24 -0400 Message-Id: <20230511144832.17974-5-simon.marchi@efficios.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20230511144832.17974-1-simon.marchi@efficios.com> References: <20230511144832.17974-1-simon.marchi@efficios.com> MIME-Version: 1.0 X-Spam-Status: No, score=-3497.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_SOFTFAIL, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Simon Marchi via Gdb-patches From: Simon Marchi Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" Add three convenience methods to struct breakpoint: - has_locations: returns true if the breakpoint has at least one location - has_single_location: returns true if the breakpoint has exactly one location - has_multiple_locations: returns true if the breakpoint has more than one location A subsequent patch changes the list of breakpoints to be an intrusive_list, so all these spots would need to change. But in any case, I think that this: if (b->has_multiple_locations ()) conveys the intention better than: if (b->loc != nullptr && b->loc->next != nullptr) Change-Id: Ib18c3605fd35d425ef9df82cb7aacff1606c6747 --- gdb/ada-lang.c | 2 +- gdb/breakpoint.c | 62 +++++++++++++++++++++++-------------------- gdb/breakpoint.h | 12 +++++++++ gdb/elfread.c | 4 +-- gdb/tracectf.c | 2 +- gdb/tracefile-tfile.c | 2 +- gdb/tracefile.c | 4 +-- gdb/tracepoint.c | 2 +- 8 files changed, 53 insertions(+), 37 deletions(-) diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index d7316c845d6d..207d58e80a63 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -12204,7 +12204,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c, return; /* Same if there are no locations... */ - if (c->loc == NULL) + if (!c->has_locations ()) return; /* Compute the condition expression in text form, from the specific diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0ca4528faef2..fefe18644cf9 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -818,7 +818,7 @@ has_multiple_locations (int num) { for (breakpoint *b : all_breakpoints ()) if (b->number == num) - return b->loc != nullptr && b->loc->next != nullptr; + return b->has_multiple_locations (); return false; } @@ -1876,7 +1876,7 @@ static void add_dummy_location (struct breakpoint *b, struct program_space *pspace) { - gdb_assert (b->loc == NULL); + gdb_assert (!b->has_locations ()); b->loc = new bp_location (b, bp_loc_other); b->loc->pspace = pspace; @@ -2241,7 +2241,7 @@ update_watchpoint (struct watchpoint *b, bool reparse) above left it without any location set up. But, bpstat_stop_status requires a location to be able to report stops, so make sure there's at least a dummy one. */ - if (b->type == bp_watchpoint && b->loc == NULL) + if (b->type == bp_watchpoint && !b->has_locations ()) add_dummy_location (b, frame_pspace); } else if (!within_current_scope) @@ -4106,7 +4106,7 @@ breakpoint_init_inferior (enum inf_context context) for (breakpoint *b : all_breakpoints_safe ()) { - if (b->loc && b->loc->pspace != pspace) + if (b->has_locations () && b->loc->pspace != pspace) continue; switch (b->type) @@ -4520,7 +4520,7 @@ bpstat_locno (const bpstat *bs) int locno = 0; - if (b != nullptr && b->loc != nullptr && b->loc->next != nullptr) + if (b != nullptr && b->has_multiple_locations ()) { const bp_location *bl_i; @@ -6054,7 +6054,9 @@ bool bpstat_should_step () { for (breakpoint *b : all_breakpoints ()) - if (breakpoint_enabled (b) && b->type == bp_watchpoint && b->loc != NULL) + if (breakpoint_enabled (b) + && b->type == bp_watchpoint + && b->has_locations ()) return true; return false; @@ -6357,10 +6359,11 @@ print_one_breakpoint_location (struct breakpoint *b, /* See comment in print_one_breakpoint concerning treatment of breakpoints with single disabled location. */ if (loc == NULL - && (b->loc != NULL - && (b->loc->next != NULL + && (b->has_locations () + && (b->has_multiple_locations () || !b->loc->enabled || b->loc->disabled_by_cond))) header_of_multiple = true; + if (loc == NULL) loc = b->loc; @@ -6451,7 +6454,7 @@ print_one_breakpoint_location (struct breakpoint *b, if (header_of_multiple) uiout->field_string ("addr", "", metadata_style.style ()); - else if (b->loc == NULL || loc->shlib_disabled) + else if (!b->has_locations () || loc->shlib_disabled) uiout->field_string ("addr", "", metadata_style.style ()); else @@ -6461,7 +6464,7 @@ print_one_breakpoint_location (struct breakpoint *b, annotate_field (5); if (!header_of_multiple) print_breakpoint_location (b, loc); - if (b->loc) + if (b->has_locations ()) *last_loc = b->loc; } } @@ -6740,9 +6743,10 @@ print_one_breakpoint (breakpoint *b, const bp_location **last_loc, int allflag) && (!is_catchpoint (b) || is_exception_catchpoint (b) || is_ada_exception_catchpoint (b)) && (allflag - || (b->loc && (b->loc->next - || !b->loc->enabled - || b->loc->disabled_by_cond)))) + || (b->has_locations () + && (b->has_multiple_locations () + || !b->loc->enabled + || b->loc->disabled_by_cond)))) { gdb::optional locations_list; @@ -6805,7 +6809,7 @@ user_breakpoint_p (struct breakpoint *b) int pending_breakpoint_p (struct breakpoint *b) { - return b->loc == NULL; + return !b->has_locations (); } /* Print information on breakpoints (including watchpoints and tracepoints). @@ -7482,8 +7486,9 @@ set_breakpoint_location_function (struct bp_location *loc) function_name = loc->msymbol->linkage_name (); - if (b->type == bp_breakpoint && b->loc == loc - && loc->next == NULL && b->related_breakpoint == b) + if (b->type == bp_breakpoint + && b->has_single_location () + && b->related_breakpoint == b) { /* Create only the whole new breakpoint of this type but do not mess more complicated breakpoints with multiple locations. */ @@ -9374,13 +9379,12 @@ ranged_breakpoint::resources_needed (const struct bp_location *bl) enum print_stop_action ranged_breakpoint::print_it (const bpstat *bs) const { - struct bp_location *bl = loc; struct ui_out *uiout = current_uiout; gdb_assert (type == bp_hardware_breakpoint); /* Ranged breakpoints have only one location. */ - gdb_assert (bl && bl->next == NULL); + gdb_assert (this->has_single_location ()); annotate_breakpoint (number); @@ -9412,7 +9416,7 @@ ranged_breakpoint::print_one (const bp_location **last_loc) const struct ui_out *uiout = current_uiout; /* Ranged breakpoints have only one location. */ - gdb_assert (bl && bl->next == NULL); + gdb_assert (this->has_single_location ()); get_user_print_options (&opts); @@ -9939,7 +9943,7 @@ masked_watchpoint::print_it (const bpstat *bs) const struct ui_out *uiout = current_uiout; /* Masked watchpoints have only one location. */ - gdb_assert (this->loc && this->loc->next == nullptr); + gdb_assert (this->has_single_location ()); annotate_watchpoint (this->number); maybe_print_thread_hit_breakpoint (uiout); @@ -9985,7 +9989,7 @@ void masked_watchpoint::print_one_detail (struct ui_out *uiout) const { /* Masked watchpoints have only one location. */ - gdb_assert (loc && loc->next == NULL); + gdb_assert (this->has_single_location ()); uiout->text ("\tmask "); uiout->field_core_addr ("mask", loc->gdbarch, hw_wp_mask); @@ -11571,7 +11575,7 @@ code_breakpoint::say_where () const /* i18n: cagney/2005-02-11: Below needs to be merged into a single string. */ - if (loc == NULL) + if (!this->has_locations ()) { /* For pending locations, the output differs slightly based on extra_string. If this is non-NULL, it contains either @@ -11604,7 +11608,7 @@ code_breakpoint::say_where () const { /* If there is a single location, we can print the location more nicely. */ - if (loc->next == NULL) + if (!this->has_multiple_locations ()) { const char *filename = symtab_to_filename_for_display (loc->symtab); @@ -11620,7 +11624,7 @@ code_breakpoint::say_where () const gdb_printf (": %s.", locspec->to_string ()); } - if (loc->next) + if (this->has_multiple_locations ()) { struct bp_location *iter = loc; int n = 0; @@ -11879,7 +11883,7 @@ ordinary_breakpoint::print_recreate (struct ui_file *fp) const /* Print out extra_string if this breakpoint is pending. It might contain, for example, conditions that were set by the user. */ - if (loc == NULL && extra_string != NULL) + if (!this->has_locations () && extra_string != NULL) gdb_printf (fp, " %s", extra_string.get ()); print_recreate_thread (fp); @@ -12867,11 +12871,11 @@ code_breakpoint::location_spec_to_sals (location_spec *locspec, errors. */ if (e.error == NOT_FOUND_ERROR && (condition_not_parsed - || (loc != NULL + || (this->has_locations () && search_pspace != NULL && loc->pspace != search_pspace) - || (loc && loc->shlib_disabled) - || (loc && loc->pspace->executing_startup) + || (this->has_locations () && loc->shlib_disabled) + || (this->has_locations () && loc->pspace->executing_startup) || enable_state == bp_disabled)) not_found_and_ok = true; @@ -14265,7 +14269,7 @@ save_breakpoints (const char *filename, int from_tty, /* If this is a multi-location breakpoint, check if the locations should be individually disabled. Watchpoint locations are special, and not user visible. */ - if (!is_watchpoint (tp) && tp->loc && tp->loc->next) + if (!is_watchpoint (tp) && tp->has_multiple_locations ()) { int n = 1; diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 80cb91b1a911..2a3a5cf57b83 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -633,6 +633,18 @@ struct breakpoint /* Allocate a location for this breakpoint. */ virtual struct bp_location *allocate_location (); + /* Return true if this breakpoint has a least one location. */ + bool has_locations () const + { return this->loc != nullptr; } + + /* Return true if this breakpoint has a single location. */ + bool has_single_location () const + { return this->loc != nullptr && this->loc->next == nullptr; } + + /* Return true if this breakpoint has multiple locations. */ + bool has_multiple_locations () const + { return this->loc != nullptr && this->loc->next != nullptr; } + /* Reevaluate a breakpoint. This is necessary after symbols change (e.g., an executable or DSO was loaded, or the inferior just started). */ diff --git a/gdb/elfread.c b/gdb/elfread.c index 0305bf218946..7169ae8808a8 100644 --- a/gdb/elfread.c +++ b/gdb/elfread.c @@ -970,7 +970,7 @@ elf_gnu_ifunc_resolver_stop (code_breakpoint *b) b_return = b_return->related_breakpoint) { gdb_assert (b_return->type == bp_gnu_ifunc_resolver_return); - gdb_assert (b_return->loc != NULL && b_return->loc->next == NULL); + gdb_assert (b_return->has_single_location ()); gdb_assert (frame_id_p (b_return->frame_id)); if (b_return->thread == thread_id @@ -1039,7 +1039,7 @@ elf_gnu_ifunc_resolver_return_stop (code_breakpoint *b) b = (code_breakpoint *) b_next; } gdb_assert (b->type == bp_gnu_ifunc_resolver); - gdb_assert (b->loc->next == NULL); + gdb_assert (b->has_single_location ()); func_func = value::allocate (func_func_type); func_func->set_lval (lval_memory); diff --git a/gdb/tracectf.c b/gdb/tracectf.c index d8d0f05d049b..ab513b1fa158 100644 --- a/gdb/tracectf.c +++ b/gdb/tracectf.c @@ -1535,7 +1535,7 @@ ctf_get_traceframe_address (void) struct tracepoint *tp = get_tracepoint_by_number_on_target (tpnum); - if (tp && tp->loc) + if (tp != nullptr && tp->has_locations ()) addr = tp->loc->address; } diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c index 31eb974d4163..9c1adee11bc3 100644 --- a/gdb/tracefile-tfile.c +++ b/gdb/tracefile-tfile.c @@ -667,7 +667,7 @@ tfile_get_traceframe_address (off_t tframe_offset) tp = get_tracepoint_by_number_on_target (tpnum); /* FIXME this is a poor heuristic if multiple locations. */ - if (tp && tp->loc) + if (tp != nullptr && tp->has_locations ()) addr = tp->loc->address; /* Restore our seek position. */ diff --git a/gdb/tracefile.c b/gdb/tracefile.c index b4543c9bf5fd..883ce4bf375a 100644 --- a/gdb/tracefile.c +++ b/gdb/tracefile.c @@ -390,11 +390,11 @@ tracefile_fetch_registers (struct regcache *regcache, int regno) /* We can often usefully guess that the PC is going to be the same as the address of the tracepoint. */ - if (tp == NULL || tp->loc == NULL) + if (tp == nullptr || !tp->has_locations ()) return; /* But don't try to guess if tracepoint is multi-location... */ - if (tp->loc->next) + if (tp->has_multiple_locations ()) { warning (_("Tracepoint %d has multiple " "locations, cannot infer $pc"), diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c index ffda4a6439e4..4a9d8491a54b 100644 --- a/gdb/tracepoint.c +++ b/gdb/tracepoint.c @@ -1522,7 +1522,7 @@ process_tracepoint_on_disconnect (void) user that pending tracepoint will no longer work. */ for (breakpoint *b : all_tracepoints ()) { - if (b->loc == NULL) + if (!b->has_locations ()) { has_pending_p = 1; break;