"catch catch/throw/rethrow", breakpoint -> catchpoint

Currently, with:

 (gdb) catch catch
 Catchpoint 1 (catch)
 (gdb) catch throw
 Catchpoint 2 (throw)
 (gdb) catch rethrow
 Catchpoint 3 (rethrow)

You get:

(gdb) info breakpoints
 Num     Type           Disp Enb Address            What
 1       breakpoint     keep y   0x0000000000b122af exception catch
 2       breakpoint     keep y   0x0000000000b1288d exception throw
 3       breakpoint     keep y   0x0000000000b12931 exception rethrow

I think it doesn't make much sense usability-wise, to show a
catchpoint as a breakpoint.  The fact that GDB sets a breakpoint at
some magic address in the C++ run time is an implementation detail,
IMO.  And as seen in the previous patch, such a catchpoint can end up
with more than one location/address even, so showing a single address
isn't entirely accurate.

This commit hides the addresses from view, and makes GDB show
"catchpoint" for type as well:

  (gdb) info breakpoints
  Num     Type           Disp Enb Address            What
  1       catchpoint     keep y                      exception catch
  2       catchpoint     keep y                      exception throw
  3       catchpoint     keep y                      exception rethrow

This comment in the code seems telling:

  /* We need to reset 'type' in order for code in breakpoint.c to do
     the right thing.  */
  cp->type = bp_breakpoint;

It kind of suggests that the reason catchpoints end up shown as
breakpoints was that it was easier to implement them that way, rather
than a desired property.

This commit fixes things up to make it possible to have bp_catch
breakpoints have software/hardware breakpoint locations, thus
eliminating the need for that hack:

 - redo breakpoint_address_is_meaningful in terms of the location's
   type rather than breakpoint type.
 - teach bpstat_what about stepping over the catchpoint locations.
 - install a allocate_location method for "catch catch/throw/rethrow",
   one that forces the location type.

Note that this also reverts the gdb hunk from:

  commit 2a8be20359dba9cc684fd3ffa222d985399f3b18
  Commit:     Tom Tromey <tom@tromey.com>
  CommitDate: Sat Oct 6 22:17:45 2018 -0600

      Fix Python gdb.Breakpoint.location crash

because now "catch throw" catchpoints hit the

   if (obj->bp->type != bp_breakpoint)
     Py_RETURN_NONE;

check above, and, adjusts the testcase to no longer expect to see the
catchpoint in the gdb.breakpoints() list.

(Note: might make sense to do the same to Ada exception catchpoints.)

gdb/ChangeLog:
2019-07-09  Pedro Alves  <palves@redhat.com>

	* break-catch-throw.c (print_one_exception_catchpoint): Skip the
	"addr" field.
	(allocate_location_exception_catchpoint): New.
	(handle_gnu_v3_exceptions): Don't reset 'type' to bp_breakpoint.
	(initialize_throw_catchpoint_ops): Install
	allocate_location_exception_catchpoint as allocate_location
	method.
	* breakpoint.c (bpstat_what) <bp_catch>: Set action to
	BPSTAT_WHAT_SINGLE if not stopping and the location's type is not
	bp_loc_other.
	(breakpoint_address_is_meaningful): Delete.
	(bl_address_is_meaningful): New.
	(breakpoint_locations_match): Adjust comment.
	(bp_location_from_bp_type): New, factored out of...
	(bp_location::bp_location(breakpoint *)): ... this.
	(bp_location::bp_location(breakpoint *, bp_loc_type)): New,
	factored out of...
	(bp_location::bp_location(breakpoint *)): ... this.  Reimplement.
	(bp_loc_is_permanent): Use bl_address_is_meaningful instead of
	breakpoint_address_is_meaningful.
	(bp_locations_compare): Adjust comment.
	(update_global_location_list): Use bl_address_is_meaningful
	instead of breakpoint_address_is_meaningful.
	* breakpoint.h (bp_location::bp_location(breakpoint *)): New
	explicit.
	(bp_location::bp_location(breakpoint *, bp_loc_type)): Declare.
	* python/py-breakpoint.c (bppy_get_location): No longer check
	whether location is null.

gdb/doc/ChangeLog:
2019-07-09  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (C++ Exception GDB/MI Catchpoint Commands): Adjust
	examples to show type=catchpoint instead of type=breakpoint and an
	address.

gdb/testsuite/ChangeLog:
2019-07-09  Pedro Alves  <palves@redhat.com>

	* gdb.cp/catch-multi-stdlib.exp: Adjust expected "info
	breakpoints" output.
	* gdb.cp/exception.exp: Adjust expected "info breakpoints" output.
	* gdb.python/py-breakpoint.exp: No longer expect that "catch
	throw" creates breakpoint.
	* gdb.mi/mi-catch-cpp-exceptions.exp (setup_catchpoint): Expect
	'type="catchpoint"'.
This commit is contained in:
Pedro Alves
2019-07-09 19:26:16 +01:00
parent b58a68fe57
commit cb1e4e32c2
12 changed files with 141 additions and 103 deletions

View File

@ -5603,8 +5603,10 @@ bpstat_what (bpstat bs_head)
}
else
{
/* There was a catchpoint, but we're not stopping.
This requires no further action. */
/* Some catchpoints are implemented with breakpoints.
For those, we need to step over the breakpoint. */
if (bs->bp_location_at->loc_type != bp_loc_other)
this_action = BPSTAT_WHAT_SINGLE;
}
break;
case bp_jit_event:
@ -6686,27 +6688,21 @@ describe_other_breakpoints (struct gdbarch *gdbarch,
}
/* Return true iff it is meaningful to use the address member of
BPT locations. For some breakpoint types, the locations' address members
are irrelevant and it makes no sense to attempt to compare them to other
addresses (or use them for any other purpose either).
/* Return true iff it is meaningful to use the address member of LOC.
For some breakpoint types, the locations' address members are
irrelevant and it makes no sense to attempt to compare them to
other addresses (or use them for any other purpose either).
More specifically, each of the following breakpoint types will
always have a zero valued location address and we don't want to mark
breakpoints of any of these types to be a duplicate of an actual
breakpoint location at address zero:
More specifically, software watchpoints and catchpoints that are
not backed by breakpoints always have a zero valued location
address and we don't want to mark breakpoints of any of these types
to be a duplicate of an actual breakpoint location at address
zero. */
bp_watchpoint
bp_catchpoint
*/
static int
breakpoint_address_is_meaningful (struct breakpoint *bpt)
static bool
bl_address_is_meaningful (bp_location *loc)
{
enum bptype type = bpt->type;
return (type != bp_watchpoint && type != bp_catchpoint);
return loc->loc_type != bp_loc_other;
}
/* Assuming LOC1 and LOC2's owners are hardware watchpoints, returns
@ -6838,8 +6834,8 @@ tracepoint_locations_match (struct bp_location *loc1,
}
/* Assuming LOC1 and LOC2's types' have meaningful target addresses
(breakpoint_address_is_meaningful), returns true if LOC1 and LOC2
represent the same location. */
(bl_address_is_meaningful), returns true if LOC1 and LOC2 represent
the same location. */
static int
breakpoint_locations_match (struct bp_location *loc1,
@ -6937,16 +6933,10 @@ adjust_breakpoint_address (struct gdbarch *gdbarch,
}
}
bp_location::bp_location (breakpoint *owner)
static bp_loc_type
bp_location_from_bp_type (bptype type)
{
bp_location *loc = this;
loc->owner = owner;
loc->cond_bytecode = NULL;
loc->shlib_disabled = 0;
loc->enabled = 1;
switch (owner->type)
switch (type)
{
case bp_breakpoint:
case bp_single_step:
@ -6972,30 +6962,44 @@ bp_location::bp_location (breakpoint *owner)
case bp_gnu_ifunc_resolver:
case bp_gnu_ifunc_resolver_return:
case bp_dprintf:
loc->loc_type = bp_loc_software_breakpoint;
mark_breakpoint_location_modified (loc);
break;
return bp_loc_software_breakpoint;
case bp_hardware_breakpoint:
loc->loc_type = bp_loc_hardware_breakpoint;
mark_breakpoint_location_modified (loc);
break;
return bp_loc_hardware_breakpoint;
case bp_hardware_watchpoint:
case bp_read_watchpoint:
case bp_access_watchpoint:
loc->loc_type = bp_loc_hardware_watchpoint;
break;
return bp_loc_hardware_watchpoint;
case bp_watchpoint:
case bp_catchpoint:
case bp_tracepoint:
case bp_fast_tracepoint:
case bp_static_tracepoint:
loc->loc_type = bp_loc_other;
break;
return bp_loc_other;
default:
internal_error (__FILE__, __LINE__, _("unknown breakpoint type"));
}
}
loc->refc = 1;
bp_location::bp_location (breakpoint *owner, bp_loc_type type)
{
this->owner = owner;
this->cond_bytecode = NULL;
this->shlib_disabled = 0;
this->enabled = 1;
this->loc_type = type;
if (this->loc_type == bp_loc_software_breakpoint
|| this->loc_type == bp_loc_hardware_breakpoint)
mark_breakpoint_location_modified (this);
this->refc = 1;
}
bp_location::bp_location (breakpoint *owner)
: bp_location::bp_location (owner,
bp_location_from_bp_type (owner->type))
{
}
/* Allocate a struct bp_location. */
@ -8639,11 +8643,12 @@ bp_loc_is_permanent (struct bp_location *loc)
{
gdb_assert (loc != NULL);
/* If we have a catchpoint or a watchpoint, just return 0. We should not
attempt to read from the addresses the locations of these breakpoint types
point to. program_breakpoint_here_p, below, will attempt to read
/* If we have a non-breakpoint-backed catchpoint or a software
watchpoint, just return 0. We should not attempt to read from
the addresses the locations of these breakpoint types point to.
program_breakpoint_here_p, below, will attempt to read
memory. */
if (!breakpoint_address_is_meaningful (loc->owner))
if (!bl_address_is_meaningful (loc))
return 0;
scoped_restore_current_pspace_and_thread restore_pspace_thread;
@ -11451,10 +11456,9 @@ breakpoint_auto_delete (bpstat bs)
/* A comparison function for bp_location AP and BP being interfaced to
qsort. Sort elements primarily by their ADDRESS (no matter what
does breakpoint_address_is_meaningful say for its OWNER),
secondarily by ordering first permanent elements and
terciarily just ensuring the array is sorted stable way despite
qsort being an unstable algorithm. */
bl_address_is_meaningful says), secondarily by ordering first
permanent elements and terciarily just ensuring the array is sorted
stable way despite qsort being an unstable algorithm. */
static int
bp_locations_compare (const void *ap, const void *bp)
@ -11794,7 +11798,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
this one from the target. */
/* OLD_LOC comes from existing struct breakpoint. */
if (breakpoint_address_is_meaningful (old_loc->owner))
if (bl_address_is_meaningful (old_loc))
{
for (loc2p = locp;
(loc2p < bp_locations + bp_locations_count
@ -11934,7 +11938,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
b = loc->owner;
if (!unduplicated_should_be_inserted (loc)
|| !breakpoint_address_is_meaningful (b)
|| !bl_address_is_meaningful (loc)
/* Don't detect duplicate for tracepoint locations because they are
never duplicated. See the comments in field `duplicate' of
`struct bp_location'. */