From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 49D013858C2D for ; Tue, 10 Jan 2023 16:25:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 49D013858C2D Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 30AGP10S020667 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 10 Jan 2023 11:25:05 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 30AGP10S020667 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1673367905; bh=pLrjzQZCRuwDYRZ7g9XMPKn8FnHJhnn/E6XqGvKvgGc=; h=Date:Subject:To:References:From:In-Reply-To:From; b=J8Dhaa6ghWtwyRbjWrOug8LNpsdRyMoF7MBdoyQU9iRlIVk4WbIEYeeHQ3f1cXxBH dDAUP74r07rXjvW3WIouFkI8WHnUf+/WgyJKtvCDZc0ybErObznbEn6QzLMD905eXR xqFanZJTGCm4i84oUjW+xdSolXwnIX6NDvC+OMLI= Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 0E05C1E110 for ; Tue, 10 Jan 2023 11:25:01 -0500 (EST) Message-ID: <28921357-bc3d-9383-7fe8-5ecef50e6dd6@polymtl.ca> Date: Tue, 10 Jan 2023 11:25:00 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH] gdb: introduce bp_loc_tracepoint Content-Language: en-US To: gdb-patches@sourceware.org References: <20221125185614.363400-1-simon.marchi@polymtl.ca> From: Simon Marchi In-Reply-To: <20221125185614.363400-1-simon.marchi@polymtl.ca> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Tue, 10 Jan 2023 16:25:01 +0000 X-Spam-Status: No, score=-3038.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,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: Ping. On 11/25/22 13:56, Simon Marchi wrote: > 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 f0276a963c00..caa7d325b12f 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -7345,20 +7345,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 f7633d29cbfd..661527a9e1a0 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... */ > }; > > > base-commit: 8654c01f085f77e443185d0a61a106880bb060b9