From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id uKteNpjj/mYLMAAAWB0awg (envelope-from ) for ; Thu, 03 Oct 2024 14:34:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1727980440; bh=50t1ISTN3a0TdInRiDYAJZdM6jk232EyaR8u783yAkI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=fnI4bBsVQ1iLOAshydXZhT7Zu8ajT5fMnsR3BEedX2SamjkQ1h5qA5KvWlZ/EUhzv Dm1tH6vgmVht88P4zBmkKkZLcigZtAUBIuVW2auUQgT1+LPjMa7cYN/8kez31/LgRs YXnLzttQRdXyxyfmql6p5fCQaL/wXElzRSBysLxs= Received: by simark.ca (Postfix, from userid 112) id CA6581E356; Thu, 3 Oct 2024 14:34:00 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.8 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_BLOCKED,RCVD_IN_VALIDITY_CERTIFIED,RCVD_IN_VALIDITY_RPBL, RCVD_IN_VALIDITY_SAFE,URIBL_BLOCKED,URIBL_DBL_BLOCKED_OPENDNS autolearn=unavailable autolearn_force=no version=4.0.0 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=ZENGo/Nw; dkim=pass (1024-bit key) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=Bh8xi6qd; dkim-atps=neutral Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 0AE481E05C for ; Thu, 3 Oct 2024 14:34:00 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 76D483846456 for ; Thu, 3 Oct 2024 18:33:59 +0000 (GMT) Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id EE404385DDD1 for ; Thu, 3 Oct 2024 18:33:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EE404385DDD1 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org EE404385DDD1 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727980419; cv=none; b=Av6yR5mWrktgvbqP92C5zXjv4AEib5xV6u3/4ieJLXUndW7YiBja3amC4buGyjUpglDxQUq+WFIcI0xz6MdpT0xQS3iOAOlZ2EbKNw5//s3FDcprIVQMeSUtU31X8gyrJ6NCFxKoLBI3rJ3gIfET6zMX+eaqgGgvnIDnz5AkF3U= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727980419; c=relaxed/simple; bh=50t1ISTN3a0TdInRiDYAJZdM6jk232EyaR8u783yAkI=; h=DKIM-Signature:DKIM-Signature:Message-ID:Date:MIME-Version: Subject:To:From; b=pI75sAgxmCHnjEhQVriK2R/4L6+1dWYcvkmmx38mjztZq8iadwQGKUmIR1aSPuvy9OedlwWRJ3EP5lD/GE/s11xp34HWnff/q/hxUSqCk/pdOVUBTvaEP1SNDfcOWLeUr23w/jtLkPqyIhPaFa9rPBmrtjVaTfbJuFeMAFDhDa4= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1727980416; bh=50t1ISTN3a0TdInRiDYAJZdM6jk232EyaR8u783yAkI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=ZENGo/NwdLMVJC0d66gxMsbduUVLltQb+ItqUWpd2Tr9AdM6xid2G/sFAcXeMgqTQ tztIddDtKif8olGR98JZOOy9kPLojvhMU9Jb4JuhrshC8Tt0tNvKQfv+iUJb6u7jBy hCxpQwyCgxz1VhZklmumJaCAureDFodI3nTyFoR8= Received: by simark.ca (Postfix, from userid 112) id C69F41E05C; Thu, 3 Oct 2024 14:33:36 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1727980415; bh=50t1ISTN3a0TdInRiDYAJZdM6jk232EyaR8u783yAkI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=Bh8xi6qdiD7SHPeJ6OVrQo5IIU0FDvNTFoXrYIj5k0oXAcKSKWHQgIyRZUKNQIBzR d8e110qkLTVdGZaj0aGgz845VMG5QCEGhW8iYp9BuKaxb64BF7G5Xwb3u36EPt2Zrq jNmHaN0k9v8lME7iXDx1aWjIfYMfakOFivuF1C8k= Received: from [172.16.0.192] (96-127-217-162.qc.cable.ebox.net [96.127.217.162]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id DC2931E05C; Thu, 3 Oct 2024 14:33:34 -0400 (EDT) Message-ID: <1ca86959-4d08-43cf-812e-487e29a25f4b@simark.ca> Date: Thu, 3 Oct 2024 14:33:34 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 1/5] gdb: make gdbarch store a vector of frame unwinders To: Guinevere Larsen , gdb-patches@sourceware.org Cc: Guinevere Larsen References: <20241001184235.3710608-1-guinevere@redhat.com> <20241001184235.3710608-2-guinevere@redhat.com> Content-Language: fr From: Simon Marchi In-Reply-To: <20241001184235.3710608-2-guinevere@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 10/1/24 2:42 PM, Guinevere Larsen wrote: > From: Guinevere Larsen > > Before this commit, all frame unwinders would be stored in the obstack > of a gdbarch and accessed by using the registry system. This made for > unwieldy code, and unnecessarily complex logic in the frame_unwinder > implementation, along with making frame_unwind structs be unable to have > non-trivial destructors. > > Seeing as a future patch of this series wants to refactor the > frame_unwind struct to use inheritance, and we'd like to not restrict > the future derived classes on what destructors are allowed. In > preparation for that change, this commit adds an std::vector to gdbarch > to store the unwinders in. > > There should be no user-visible changes. Just some nits, but with those addressed: Approved-By: Simon Marchi > --- > gdb/arch-utils.c | 8 ++++ > gdb/frame-unwind.c | 113 +++++++++++++++++---------------------------- > gdb/gdbarch-gen.c | 6 +++ > gdb/gdbarch.h | 5 ++ > gdb/gdbarch.py | 6 +++ > 5 files changed, 67 insertions(+), 71 deletions(-) > > diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c > index 6ffa4109765..5a4ed097321 100644 > --- a/gdb/arch-utils.c > +++ b/gdb/arch-utils.c > @@ -1225,6 +1225,14 @@ obstack *gdbarch_obstack (gdbarch *arch) > > /* See gdbarch.h. */ > > +std::vector & > +gdbarch_unwinder_list (struct gdbarch *arch) > +{ > + return arch->unwinders; > +} > + > +/* See gdbarch.h. */ > + > char * > gdbarch_obstack_strdup (struct gdbarch *arch, const char *string) > { > diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c > index e5f108d3257..b69ae8596a2 100644 > --- a/gdb/frame-unwind.c > +++ b/gdb/frame-unwind.c > @@ -31,62 +31,46 @@ > #include "cli/cli-cmds.h" > #include "inferior.h" > > -struct frame_unwind_table_entry > +/* Default sniffers, that must always be the first in the unwinder list, > + no matter the architecture. */ > +static constexpr auto standard_unwinders = > { > - const struct frame_unwind *unwinder; > - struct frame_unwind_table_entry *next; > -}; > + &dummy_frame_unwind, > + /* The DWARF tailcall sniffer must come before the inline sniffer. > + Otherwise, we can end up in a situation where a DWARF frame finds > + tailcall information, but then the inline sniffer claims a frame > + before the tailcall sniffer, resulting in confusion. This is > + safe to do always because the tailcall sniffer can only ever be > + activated if the newer frame was created using the DWARF > + unwinder, and it also found tailcall information. */ > + &dwarf2_tailcall_frame_unwind, > + &inline_frame_unwind, > > -struct frame_unwind_table > -{ > - struct frame_unwind_table_entry *list = nullptr; > - /* The head of the OSABI part of the search list. */ > - struct frame_unwind_table_entry **osabi_head = nullptr; > }; > > -static const registry::key > - frame_unwind_data; > +/* If an unwinder should be prepended to the list, this is the > + index in which it should be inserted. */ > +static constexpr int prepend_unwinder_index = standard_unwinders.size (); > > -/* A helper function to add an unwinder to a list. LINK says where to > - install the new unwinder. The new link is returned. */ > - > -static struct frame_unwind_table_entry ** > -add_unwinder (struct obstack *obstack, const struct frame_unwind *unwinder, > - struct frame_unwind_table_entry **link) > +/* Start the table out with a few default sniffers. OSABI code > + can't override this. */ > +static void > +initialize_frame_unwind_table (std::vector& table) > { > - *link = OBSTACK_ZALLOC (obstack, struct frame_unwind_table_entry); > - (*link)->unwinder = unwinder; > - return &(*link)->next; > + gdb_assert (table.empty ()); > + > + table.insert(table.begin (), standard_unwinders.begin (), standard_unwinders.end ()); Line a bit too long and missing space. > } > > -static struct frame_unwind_table * > +/* This function call retrieves the list of frame unwinders available in > + GDBARCH. If this list is empty, it is initialized before being > + returned. */ For brevity, I think you should remove "This function call", just start with "Retrieve" or "Retrieves", depending of which school you are. > +static std::vector & > get_frame_unwind_table (struct gdbarch *gdbarch) > { > - struct frame_unwind_table *table = frame_unwind_data.get (gdbarch); > - if (table != nullptr) > - return table; > - > - table = new frame_unwind_table; > - > - /* Start the table out with a few default sniffers. OSABI code > - can't override this. */ > - struct frame_unwind_table_entry **link = &table->list; > - > - struct obstack *obstack = gdbarch_obstack (gdbarch); > - link = add_unwinder (obstack, &dummy_frame_unwind, link); > - /* The DWARF tailcall sniffer must come before the inline sniffer. > - Otherwise, we can end up in a situation where a DWARF frame finds > - tailcall information, but then the inline sniffer claims a frame > - before the tailcall sniffer, resulting in confusion. This is > - safe to do always because the tailcall sniffer can only ever be > - activated if the newer frame was created using the DWARF > - unwinder, and it also found tailcall information. */ > - link = add_unwinder (obstack, &dwarf2_tailcall_frame_unwind, link); > - link = add_unwinder (obstack, &inline_frame_unwind, link); > - > - /* The insertion point for OSABI sniffers. */ > - table->osabi_head = link; > - frame_unwind_data.set (gdbarch, table); > + std::vector &table = gdbarch_unwinder_list (gdbarch); > + if (table.size () == 0) table.empty () Simon