From bcafd1c19e628d831cf3eb20229c42ad9db4b29c Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Fri, 15 Sep 2023 11:56:35 -0600 Subject: [PATCH] Use gdb::checked_static_cast for watchpoints This replaces some casts to 'watchpoint *' with checked_static_cast. In one spot, an unnecessary block is also removed. Approved-By: Simon Marchi --- gdb/breakpoint.c | 299 ++++++++++++++++++------------------- gdb/guile/scm-breakpoint.c | 3 +- gdb/python/py-breakpoint.c | 3 +- 3 files changed, 149 insertions(+), 156 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 4b3999a92ee..9a5e55df051 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -5243,13 +5243,12 @@ enum wp_check_result static wp_check_result watchpoint_check (bpstat *bs) { - struct watchpoint *b; frame_info_ptr fr; bool within_current_scope; /* BS is built from an existing struct breakpoint. */ gdb_assert (bs->breakpoint_at != NULL); - b = (struct watchpoint *) bs->breakpoint_at; + watchpoint *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 @@ -5405,164 +5404,160 @@ static void bpstat_check_watchpoint (bpstat *bs) { const struct bp_location *bl; - struct watchpoint *b; /* 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; + watchpoint *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; } - } - 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; + 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; + } } /* For breakpoints that are currently marked as telling gdb to stop, @@ -5625,7 +5620,7 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) if (is_watchpoint (b)) { - struct watchpoint *w = (struct watchpoint *) b; + watchpoint *w = gdb::checked_static_cast (b); cond = w->cond_exp.get (); } @@ -5635,7 +5630,6 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) if (cond != nullptr && b->disposition != disp_del_at_next_stop) { bool within_current_scope = true; - struct watchpoint * w; /* We use scoped_value_mark because it could be a long time before we return to the command level and call @@ -5643,10 +5637,9 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread) might be in the middle of evaluating a function call. */ scoped_value_mark mark; + watchpoint *w = nullptr; if (is_watchpoint (b)) - w = (struct watchpoint *) b; - else - w = NULL; + w = gdb::checked_static_cast (b); /* Need to select the frame, with all that implies so that the conditions will have the right context. Because we @@ -5794,7 +5787,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; + watchpoint *w + = gdb::checked_static_cast (b.related_breakpoint); w->watchpoint_triggered = watch_triggered_yes; } @@ -5918,7 +5912,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; + watchpoint *w + = gdb::checked_static_cast (bs->breakpoint_at); update_watchpoint (w, false /* don't reparse. */); need_remove_insert = 1; @@ -6568,7 +6563,7 @@ print_one_breakpoint_location (struct breakpoint *b, { if (is_watchpoint (b)) { - struct watchpoint *w = (struct watchpoint *) b; + 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 +6823,7 @@ print_one_breakpoint_location (struct breakpoint *b, { if (is_watchpoint (b)) { - struct watchpoint *w = (struct watchpoint *) b; + watchpoint *w = gdb::checked_static_cast (b); uiout->field_string ("original-location", w->exp_string.get ()); } @@ -7257,8 +7252,8 @@ 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; + watchpoint *w1 = gdb::checked_static_cast (loc1->owner); + watchpoint *w2 = gdb::checked_static_cast (loc2->owner); /* Both of them must exist. */ gdb_assert (w1 != NULL); @@ -12618,9 +12613,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) @@ -13784,7 +13779,7 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition, try { - struct watchpoint *w = (struct watchpoint *) bpt; + 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..85d7ac0f7bc 100644 --- a/gdb/guile/scm-breakpoint.c +++ b/gdb/guile/scm-breakpoint.c @@ -890,12 +890,11 @@ gdbscm_breakpoint_expression (SCM self) { breakpoint_smob *bp_smob = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME); - struct watchpoint *wp; if (!is_watchpoint (bp_smob->bp)) return SCM_BOOL_F; - wp = (struct watchpoint *) bp_smob->bp; + watchpoint *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..71182cc1b1b 100644 --- a/gdb/python/py-breakpoint.c +++ b/gdb/python/py-breakpoint.c @@ -549,14 +549,13 @@ bppy_get_expression (PyObject *self, void *closure) { const char *str; gdbpy_breakpoint_object *obj = (gdbpy_breakpoint_object *) self; - struct watchpoint *wp; BPPY_REQUIRE_VALID (obj); if (!is_watchpoint (obj->bp)) Py_RETURN_NONE; - wp = (struct watchpoint *) obj->bp; + watchpoint *wp = gdb::checked_static_cast (obj->bp); str = wp->exp_string.get (); if (! str) -- 2.30.2