From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oo1-xc36.google.com (mail-oo1-xc36.google.com [IPv6:2607:f8b0:4864:20::c36]) by sourceware.org (Postfix) with ESMTPS id 7440A3858C60 for ; Fri, 15 Sep 2023 18:36:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7440A3858C60 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com Received: by mail-oo1-xc36.google.com with SMTP id 006d021491bc7-5738cb00eebso1448225eaf.2 for ; Fri, 15 Sep 2023 11:36:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore.com; s=google; t=1694802964; x=1695407764; darn=sourceware.org; h=to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=LLhYDs3+nIsSTAO7blRs4ykDx2M0WqCTGjNUYide9zk=; b=eYG0tYE1gd4iVFw6fa/LlwCeEYx3GbGh4itUiUgvO0dVlMpwk5zgwSBwHAmywDmrjq vPk0M3Dxx6Pyi5pvuBfVy3/pm+5PvH/Oi1tf7ObI7D+U9nSBFVlMqSn79JUPyloC+3A8 RpTY9sDgwlz34qAxMQuipZlF445dTgGoEkqCP2zJLPVcKVsZv2UxvJAWtABlUWy7V8Uy zL3ToAImrH+EvwygOEyNiDs/Uh/5iPxvUFm2ZkQ5H1CDf64vtQmXCsIItTGCRce+o4Mg q2CVeH0MXT5p9q6QC8jPb2ambUsqxb5FIbrQXXW3l3yG8r78djtU97YG6bo0GmX+SJZB 24+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694802964; x=1695407764; h=to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=LLhYDs3+nIsSTAO7blRs4ykDx2M0WqCTGjNUYide9zk=; b=pFLeYzM23XmHqfmdWAg6vBKYc/lS01apWuev0P+I1WxzR3iwS87SU6zx2+e4fJVa0r IS6Nw4LOrUo0+sS4BEkYDFHnL0HV7sCSxQwIFbTn/iBewg5xa+kT/XUowXpbZEjQUFDB Jsj4OKfM51/lqXKF7D8VHw6Boo8w6lHDJuaA/HydWyYm2ggZPDaFdT3VOVVVpgkRSHjh xB5TxXKYw5NzT85Qe4uor78jPjLV/p+G4Mio0l/X9jPyVmJYdXr6tov6QEXdEAjtFgdk CniDgdUtVxq2UI2Eoc1/JkGrwqVKUI48A0LdLTw20tM7JA3l/y4sXnu5tjEgldujXx+p jZug== X-Gm-Message-State: AOJu0Yy6SSB/Ehu11nKW0iga0H0QfEYOkp9JVh7cji9PK1lVegovXj9h e0KnYA/HCCT+s84Kcmwzh4WmZWF58jyQ686yLAlmCQ== X-Google-Smtp-Source: AGHT+IEHGQFcO/uaVl7sWYILce02IKd0dhLnyp0IV1jrkysSrO2UI9/Vq7gfYKzsXXAO+iwkGghtAw== X-Received: by 2002:a05:6870:414e:b0:1d0:cbc7:1c92 with SMTP id r14-20020a056870414e00b001d0cbc71c92mr2648988oad.28.1694802963638; Fri, 15 Sep 2023 11:36:03 -0700 (PDT) Received: from localhost.localdomain (71-211-130-31.hlrn.qwest.net. [71.211.130.31]) by smtp.gmail.com with ESMTPSA id w25-20020a02cf99000000b0042b0a6d899fsm1235997jar.60.2023.09.15.11.36.03 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Sep 2023 11:36:03 -0700 (PDT) From: Tom Tromey Date: Fri, 15 Sep 2023 12:36:01 -0600 Subject: [PATCH 1/4] Use gdb::checked_static_cast for watchpoints MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20230915-watchpoint-casts-v1-1-a4ff35c9644e@adacore.com> References: <20230915-watchpoint-casts-v1-0-a4ff35c9644e@adacore.com> In-Reply-To: <20230915-watchpoint-casts-v1-0-a4ff35c9644e@adacore.com> To: gdb-patches@sourceware.org X-Mailer: b4 0.12.3 X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This replaces some casts to 'watchpoint *' with checked_static_cast. In one spot, an unnecessary block is also removed. --- gdb/breakpoint.c | 298 +++++++++++++++++++++++---------------------- gdb/guile/scm-breakpoint.c | 2 +- gdb/python/py-breakpoint.c | 2 +- 3 files changed, 153 insertions(+), 149 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index c429af455ff..c0d86f38c73 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -5249,7 +5249,7 @@ watchpoint_check (bpstat *bs) /* BS is built from an existing struct breakpoint. */ gdb_assert (bs->breakpoint_at != NULL); - b = (struct watchpoint *) bs->breakpoint_at; + b = gdb::checked_static_cast (bs->breakpoint_at); /* If this is a local watchpoint, we only want to check if the watchpoint frame is in scope if the current thread is the thread @@ -5410,158 +5410,155 @@ bpstat_check_watchpoint (bpstat *bs) /* BS is built for existing struct breakpoint. */ bl = bs->bp_location_at.get (); gdb_assert (bl != NULL); - b = (struct watchpoint *) bs->breakpoint_at; - gdb_assert (b != NULL); - - { - bool must_check_value = false; - - if (b->type == bp_watchpoint) - /* For a software watchpoint, we must always check the - watched value. */ - must_check_value = true; - else if (b->watchpoint_triggered == watch_triggered_yes) - /* We have a hardware watchpoint (read, write, or access) - and the target earlier reported an address watched by - this watchpoint. */ - must_check_value = true; - else if (b->watchpoint_triggered == watch_triggered_unknown - && b->type == bp_hardware_watchpoint) - /* We were stopped by a hardware watchpoint, but the target could - not report the data address. We must check the watchpoint's - value. Access and read watchpoints are out of luck; without - a data address, we can't figure it out. */ - must_check_value = true; + b = gdb::checked_static_cast (bs->breakpoint_at); + + bool must_check_value = false; + + if (b->type == bp_watchpoint) + /* For a software watchpoint, we must always check the + watched value. */ + must_check_value = true; + else if (b->watchpoint_triggered == watch_triggered_yes) + /* We have a hardware watchpoint (read, write, or access) + and the target earlier reported an address watched by + this watchpoint. */ + must_check_value = true; + else if (b->watchpoint_triggered == watch_triggered_unknown + && b->type == bp_hardware_watchpoint) + /* We were stopped by a hardware watchpoint, but the target could + not report the data address. We must check the watchpoint's + value. Access and read watchpoints are out of luck; without + a data address, we can't figure it out. */ + must_check_value = true; + + if (must_check_value) + { + wp_check_result e; - if (must_check_value) + try + { + e = watchpoint_check (bs); + } + catch (const gdb_exception_error &ex) { - wp_check_result e; + exception_fprintf (gdb_stderr, ex, + "Error evaluating expression " + "for watchpoint %d\n", + b->number); - try - { - e = watchpoint_check (bs); - } - catch (const gdb_exception_error &ex) + SWITCH_THRU_ALL_UIS () { - exception_fprintf (gdb_stderr, ex, - "Error evaluating expression " - "for watchpoint %d\n", - b->number); - - SWITCH_THRU_ALL_UIS () - { - gdb_printf (_("Watchpoint %d deleted.\n"), - b->number); - } - watchpoint_del_at_next_stop (b); - e = WP_DELETED; + gdb_printf (_("Watchpoint %d deleted.\n"), + b->number); } + watchpoint_del_at_next_stop (b); + e = WP_DELETED; + } - switch (e) + switch (e) + { + case WP_DELETED: + /* We've already printed what needs to be printed. */ + bs->print_it = print_it_done; + /* Stop. */ + break; + case WP_IGNORE: + bs->print_it = print_it_noop; + bs->stop = false; + break; + case WP_VALUE_CHANGED: + if (b->type == bp_read_watchpoint) { - case WP_DELETED: - /* We've already printed what needs to be printed. */ - bs->print_it = print_it_done; - /* Stop. */ - break; - case WP_IGNORE: - bs->print_it = print_it_noop; - bs->stop = false; - break; - case WP_VALUE_CHANGED: - if (b->type == bp_read_watchpoint) + /* There are two cases to consider here: + + 1. We're watching the triggered memory for reads. + In that case, trust the target, and always report + the watchpoint hit to the user. Even though + reads don't cause value changes, the value may + have changed since the last time it was read, and + since we're not trapping writes, we will not see + those, and as such we should ignore our notion of + old value. + + 2. We're watching the triggered memory for both + reads and writes. There are two ways this may + happen: + + 2.1. This is a target that can't break on data + reads only, but can break on accesses (reads or + writes), such as e.g., x86. We detect this case + at the time we try to insert read watchpoints. + + 2.2. Otherwise, the target supports read + watchpoints, but, the user set an access or write + watchpoint watching the same memory as this read + watchpoint. + + If we're watching memory writes as well as reads, + ignore watchpoint hits when we find that the + value hasn't changed, as reads don't cause + changes. This still gives false positives when + the program writes the same value to memory as + what there was already in memory (we will confuse + it for a read), but it's much better than + nothing. */ + + int other_write_watchpoint = 0; + + if (bl->watchpoint_type == hw_read) { - /* There are two cases to consider here: - - 1. We're watching the triggered memory for reads. - In that case, trust the target, and always report - the watchpoint hit to the user. Even though - reads don't cause value changes, the value may - have changed since the last time it was read, and - since we're not trapping writes, we will not see - those, and as such we should ignore our notion of - old value. - - 2. We're watching the triggered memory for both - reads and writes. There are two ways this may - happen: - - 2.1. This is a target that can't break on data - reads only, but can break on accesses (reads or - writes), such as e.g., x86. We detect this case - at the time we try to insert read watchpoints. - - 2.2. Otherwise, the target supports read - watchpoints, but, the user set an access or write - watchpoint watching the same memory as this read - watchpoint. - - If we're watching memory writes as well as reads, - ignore watchpoint hits when we find that the - value hasn't changed, as reads don't cause - changes. This still gives false positives when - the program writes the same value to memory as - what there was already in memory (we will confuse - it for a read), but it's much better than - nothing. */ - - int other_write_watchpoint = 0; - - if (bl->watchpoint_type == hw_read) - { - for (breakpoint &other_b : all_breakpoints ()) - if (other_b.type == bp_hardware_watchpoint - || other_b.type == bp_access_watchpoint) + for (breakpoint &other_b : all_breakpoints ()) + if (other_b.type == bp_hardware_watchpoint + || other_b.type == bp_access_watchpoint) + { + watchpoint &other_w = + gdb::checked_static_cast (other_b); + + if (other_w.watchpoint_triggered + == watch_triggered_yes) { - watchpoint &other_w = - gdb::checked_static_cast (other_b); - - if (other_w.watchpoint_triggered - == watch_triggered_yes) - { - other_write_watchpoint = 1; - break; - } + other_write_watchpoint = 1; + break; } - } - - if (other_write_watchpoint - || bl->watchpoint_type == hw_access) - { - /* We're watching the same memory for writes, - and the value changed since the last time we - updated it, so this trap must be for a write. - Ignore it. */ - bs->print_it = print_it_noop; - bs->stop = false; - } + } } - break; - case WP_VALUE_NOT_CHANGED: - if (b->type == bp_hardware_watchpoint - || b->type == bp_watchpoint) + + if (other_write_watchpoint + || bl->watchpoint_type == hw_access) { - /* Don't stop: write watchpoints shouldn't fire if - the value hasn't changed. */ + /* We're watching the same memory for writes, + and the value changed since the last time we + updated it, so this trap must be for a write. + Ignore it. */ bs->print_it = print_it_noop; bs->stop = false; } - /* Stop. */ - break; - default: - /* Can't happen. */ - break; } + break; + case WP_VALUE_NOT_CHANGED: + if (b->type == bp_hardware_watchpoint + || b->type == bp_watchpoint) + { + /* Don't stop: write watchpoints shouldn't fire if + the value hasn't changed. */ + bs->print_it = print_it_noop; + bs->stop = false; + } + /* Stop. */ + break; + default: + /* Can't happen. */ + break; } - else /* !must_check_value */ - { - /* This is a case where some watchpoint(s) triggered, but - not at the address of this watchpoint, or else no - watchpoint triggered after all. So don't print - anything for this watchpoint. */ - bs->print_it = print_it_noop; - bs->stop = false; - } + } + else /* !must_check_value */ + { + /* This is a case where some watchpoint(s) triggered, but + not at the address of this watchpoint, or else no + watchpoint triggered after all. So don't print + anything for this watchpoint. */ + bs->print_it = print_it_noop; + bs->stop = false; } } @@ -5625,7 +5622,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) if (is_watchpoint (b)) { - struct watchpoint *w = (struct watchpoint *) b; + struct watchpoint *w = gdb::checked_static_cast (b); cond = w->cond_exp.get (); } @@ -5644,7 +5641,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) scoped_value_mark mark; if (is_watchpoint (b)) - w = (struct watchpoint *) b; + w = gdb::checked_static_cast (b); else w = NULL; @@ -5794,7 +5791,8 @@ build_bpstat_chain (const address_space *aspace, CORE_ADDR bp_addr, iteration. */ if (b.type == bp_watchpoint_scope && b.related_breakpoint != &b) { - struct watchpoint *w = (struct watchpoint *) b.related_breakpoint; + struct watchpoint *w + = gdb::checked_static_cast (b.related_breakpoint); w->watchpoint_triggered = watch_triggered_yes; } @@ -5918,7 +5916,8 @@ bpstat_stop_status (const address_space *aspace, && bs->breakpoint_at && is_hardware_watchpoint (bs->breakpoint_at)) { - struct watchpoint *w = (struct watchpoint *) bs->breakpoint_at; + struct watchpoint *w + = gdb::checked_static_cast (bs->breakpoint_at); update_watchpoint (w, false /* don't reparse. */); need_remove_insert = 1; @@ -6568,7 +6567,8 @@ print_one_breakpoint_location (struct breakpoint *b, { if (is_watchpoint (b)) { - struct watchpoint *w = (struct watchpoint *) b; + struct watchpoint *w + = gdb::checked_static_cast (b); /* Field 4, the address, is omitted (which makes the columns not line up too nicely with the headers, but the effect @@ -6828,7 +6828,8 @@ print_one_breakpoint_location (struct breakpoint *b, { if (is_watchpoint (b)) { - struct watchpoint *w = (struct watchpoint *) b; + struct watchpoint *w + = gdb::checked_static_cast (b); uiout->field_string ("original-location", w->exp_string.get ()); } @@ -7257,8 +7258,10 @@ static bool watchpoint_locations_match (const struct bp_location *loc1, const struct bp_location *loc2) { - struct watchpoint *w1 = (struct watchpoint *) loc1->owner; - struct watchpoint *w2 = (struct watchpoint *) loc2->owner; + struct watchpoint *w1 + = gdb::checked_static_cast (loc1->owner); + struct watchpoint *w2 + = gdb::checked_static_cast (loc2->owner); /* Both of them must exist. */ gdb_assert (w1 != NULL); @@ -12604,9 +12607,9 @@ delete_breakpoint (struct breakpoint *bpt) struct watchpoint *w; if (bpt->type == bp_watchpoint_scope) - w = (struct watchpoint *) bpt->related_breakpoint; + w = gdb::checked_static_cast (bpt->related_breakpoint); else if (bpt->related_breakpoint->type == bp_watchpoint_scope) - w = (struct watchpoint *) bpt; + w = gdb::checked_static_cast (bpt); else w = NULL; if (w != NULL) @@ -13770,7 +13773,8 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition, try { - struct watchpoint *w = (struct watchpoint *) bpt; + struct watchpoint *w + = gdb::checked_static_cast (bpt); orig_enable_state = bpt->enable_state; bpt->enable_state = bp_enabled; diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c index 59254646bcc..541e1037a5e 100644 --- a/gdb/guile/scm-breakpoint.c +++ b/gdb/guile/scm-breakpoint.c @@ -895,7 +895,7 @@ gdbscm_breakpoint_expression (SCM self) if (!is_watchpoint (bp_smob->bp)) return SCM_BOOL_F; - wp = (struct watchpoint *) bp_smob->bp; + wp = gdb::checked_static_cast (bp_smob->bp); const char *str = wp->exp_string.get (); if (! str) diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c index cb064515690..c6d23b65768 100644 --- a/gdb/python/py-breakpoint.c +++ b/gdb/python/py-breakpoint.c @@ -556,7 +556,7 @@ bppy_get_expression (PyObject *self, void *closure) if (!is_watchpoint (obj->bp)) Py_RETURN_NONE; - wp = (struct watchpoint *) obj->bp; + wp = gdb::checked_static_cast (obj->bp); str = wp->exp_string.get (); if (! str) -- 2.40.1