From 0b63c811efc38f50b72422ed6a9db1de053e8736 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 25 Nov 2022 13:56:14 -0500 Subject: [PATCH] gdb: introduce bp_loc_tracepoint Since commit cb1e4e32c2d9 ("catch catch/throw/rethrow", breakpoint -> catchpoint), this simple tracing scenario does not work: $ gdb/gdb -nx -q --data-directory=gdb/data-directory ./test Reading symbols from ./test... (gdb) tar rem :1234 Remote debugging using :1234 Reading symbols from /lib64/ld-linux-x86-64.so.2... (No debugging symbols found in /lib64/ld-linux-x86-64.so.2) 0x00007ffff7fe5730 in ?? () from /lib64/ld-linux-x86-64.so.2 (gdb) trace do_something Tracepoint 1 at 0x555555555144: file test.c, line 5. (gdb) tstart (gdb) continue Continuing. Target returns error code '01'. The root cause is that the bp_location::nserted flag does not transfer anymore from an old bp_location to the new matching one. When a shared library gets loaded, GDB computes new breakpoint locations for each breakpoint in update_breakpoint_locations. The new locations are in the breakpoint::loc chain, while the old locations are still in the bp_locations global vector. Later, update_global_location_list is called. It tries to map old locations to new locations, and if necessary transfer some properties, like the inserted flag. Since commit cb1e4e32c2d9, the inserted flag isn't transferred for locations of tracepoints. This is because bl_address_is_meaningful used to be implemented like this: static int breakpoint_address_is_meaningful (struct breakpoint *bpt) { enum bptype type = bpt->type; return (type != bp_watchpoint && type != bp_catchpoint); } and was changed to this: static bool bl_address_is_meaningful (bp_location *loc) { return loc->loc_type != bp_loc_other; } Because locations for tracepoints have the bp_loc_other type, bl_address_is_meaningful started to return false for them, where it returned true before. This made update_global_location_list skip the part where it calls swap_insertion. I think this can be solved by introduced a new bp_loc_tracepoint bp_loc_type. I don't know if it's accurate, but my understanding is that bp_loc_type describes roughly "how do we ask the target to insert that location". bp_loc_software_breakpoint are inserted using target_ops::insert_breakpoint_location. bp_loc_hardware_breakpoint are inserted using target_ops::insert_hw_breakpoint. bp_loc_software_watchpoint and bp_loc_hardware_watchpoint are inserted using target_ops::insert_watchpoint. For all these, the address is meaningful, as we ask the target to insert the point at a specific address. bp_loc_other is a catch-all for "the rest", in practice for catchpoints that don't have a specific address (hence why bl_address_is_meaningful returns false for them). For instance, inserting a signal catchpoint is done by asking the target to report that specific signal. GDB doesn't associate an address to that. But tracepoints do have a meaningful address to thems, so they can't be bp_loc_other, with that logic. They also can't be bp_loc_software_breakpoint, because we don't want GDB to insert breakpoints for them (even though they might be implemented using software breakpoints by the remote side). So, the new bp_loc_tracepoint type describes that the way to insert these locations is with target_ops::download_tracepoint. It makes bl_address_is_meaningful return true for them. And they'll be ignored by insert_bp_location and GDB won't try to insert a memory breakpoint for them. With this, I see a few instances of 'Target returns error code: 01' disappearing from gdb.log, and the results of gdb.trace/*.exp improve a little bit: -# of expected passes 3765 +# of expected passes 3781 -# of unexpected failures 518 +# of unexpected failures 498 Things remain quite broken in that area though. Change-Id: Ic40935c450410f4bfaba397c9ebc7faf97320dd3 --- gdb/breakpoint.c | 9 ++++++++- gdb/breakpoint.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index abee22cd162..51bea46256f 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -7359,20 +7359,27 @@ bp_location_from_bp_type (bptype type) case bp_gnu_ifunc_resolver_return: case bp_dprintf: return bp_loc_software_breakpoint; + case bp_hardware_breakpoint: return bp_loc_hardware_breakpoint; + case bp_hardware_watchpoint: case bp_read_watchpoint: case bp_access_watchpoint: return bp_loc_hardware_watchpoint; + case bp_watchpoint: return bp_loc_software_watchpoint; - case bp_catchpoint: + case bp_tracepoint: case bp_fast_tracepoint: case bp_static_tracepoint: case bp_static_marker_tracepoint: + return bp_loc_tracepoint; + + case bp_catchpoint: return bp_loc_other; + default: internal_error (_("unknown breakpoint type")); } diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 03aecd15eff..7c5cf3f2bef 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -317,6 +317,7 @@ enum bp_loc_type bp_loc_hardware_breakpoint, bp_loc_software_watchpoint, bp_loc_hardware_watchpoint, + bp_loc_tracepoint, bp_loc_other /* Miscellaneous... */ }; -- 2.30.2