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 <simon.marchi@efficios.com>
This commit is contained in:
Tom Tromey 2023-09-15 11:56:35 -06:00
parent 093da43d2a
commit bcafd1c19e
3 changed files with 146 additions and 153 deletions

View File

@ -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<watchpoint *> (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,163 +5404,159 @@ 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);
watchpoint *b = gdb::checked_static_cast<watchpoint *> (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)
{
bool must_check_value = false;
wp_check_result e;
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)
try
{
wp_check_result e;
e = watchpoint_check (bs);
}
catch (const gdb_exception_error &ex)
{
exception_fprintf (gdb_stderr, ex,
"Error evaluating expression "
"for watchpoint %d\n",
b->number);
try
SWITCH_THRU_ALL_UIS ()
{
e = watchpoint_check (bs);
gdb_printf (_("Watchpoint %d deleted.\n"),
b->number);
}
catch (const gdb_exception_error &ex)
watchpoint_del_at_next_stop (b);
e = WP_DELETED;
}
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)
{
exception_fprintf (gdb_stderr, ex,
"Error evaluating expression "
"for watchpoint %d\n",
b->number);
/* There are two cases to consider here:
SWITCH_THRU_ALL_UIS ()
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)
{
gdb_printf (_("Watchpoint %d deleted.\n"),
b->number);
}
watchpoint_del_at_next_stop (b);
e = WP_DELETED;
}
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<watchpoint &> (other_b);
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)
{
/* 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)
if (other_w.watchpoint_triggered
== watch_triggered_yes)
{
watchpoint &other_w =
gdb::checked_static_cast<watchpoint &> (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 +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<watchpoint *> (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<watchpoint *> (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<watchpoint *> (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<watchpoint *> (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<watchpoint *> (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<watchpoint *> (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<watchpoint *> (loc1->owner);
watchpoint *w2 = gdb::checked_static_cast<watchpoint *> (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<watchpoint *> (bpt->related_breakpoint);
else if (bpt->related_breakpoint->type == bp_watchpoint_scope)
w = (struct watchpoint *) bpt;
w = gdb::checked_static_cast<watchpoint *> (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<watchpoint *> (bpt);
orig_enable_state = bpt->enable_state;
bpt->enable_state = bp_enabled;

View File

@ -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<watchpoint *> (bp_smob->bp);
const char *str = wp->exp_string.get ();
if (! str)

View File

@ -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<watchpoint *> (obj->bp);
str = wp->exp_string.get ();
if (! str)