From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id OJswFc3N3mLSYBoAWB0awg (envelope-from ) for ; Mon, 25 Jul 2022 13:07:25 -0400 Received: by simark.ca (Postfix, from userid 112) id 534CE1E9E9; Mon, 25 Jul 2022 13:07:25 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=ULkQ+G+3; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id A603B1E21F for ; Mon, 25 Jul 2022 13:07:24 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 57455383B7AC for ; Mon, 25 Jul 2022 17:07:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 57455383B7AC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1658768844; bh=98BRT3l+1QL0GXhq2k4ewDqm7HNS6UuECT0KP1LN8uc=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ULkQ+G+3Pmx+T9y8L2A0S7oVBjQAtXwroY+NYrx4of+e0Aqxss1pPK748JR19stZ0 I1TQVkSX9hobA4jv1JZBaZ10g4eOh/WdbMnkaq5jo4laXF3y2pwJTiLaW0uedqZ6y3 sEA3JsYcegpwW2wI6nw4AN12fvHH0Sk/Yhd8Kcns= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 57AAD383B7A7 for ; Mon, 25 Jul 2022 17:07:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 57AAD383B7A7 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-352-4cWjou-qOb26pTF4cVOtpg-1; Mon, 25 Jul 2022 13:07:00 -0400 X-MC-Unique: 4cWjou-qOb26pTF4cVOtpg-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 952F53C025CE; Mon, 25 Jul 2022 17:07:00 +0000 (UTC) Received: from blarsen.com (ovpn-116-28.gru2.redhat.com [10.97.116.28]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7D3311121314; Mon, 25 Jul 2022 17:06:58 +0000 (UTC) To: gdb-patches@sourceware.org Subject: [PATCH v3 2/5] Introduce frame_info_ptr smart pointer class Date: Mon, 25 Jul 2022 14:06:34 -0300 Message-Id: <20220725170637.79699-3-blarsen@redhat.com> In-Reply-To: <20220725170637.79699-1-blarsen@redhat.com> References: <20220725170637.79699-1-blarsen@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Bruno Larsen via Gdb-patches Reply-To: Bruno Larsen Cc: Tom Tromey Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" From: Tom Tromey This adds frame_info_ptr, a smart pointer class. Every instance of the class is kept on a circular, doubly-linked list. When reinit_frame_cache is called, the list is traversed and all the pointers are invalidated. This should help catch the typical GDB bug of keeping a frame_info pointer alive where a frame ID was needed instead. Co-Authored-By: Bruno Larsen --- gdb/frame-info.h | 180 ++++++++++++++++++++++++++++++++++++ gdb/frame.c | 6 ++ gdb/frame.h | 2 + gdbsupport/intrusive_list.h | 10 +- 4 files changed, 193 insertions(+), 5 deletions(-) create mode 100644 gdb/frame-info.h diff --git a/gdb/frame-info.h b/gdb/frame-info.h new file mode 100644 index 00000000000..ecbf4a56545 --- /dev/null +++ b/gdb/frame-info.h @@ -0,0 +1,180 @@ +/* Frame info pointer + + Copyright (C) 2022 Free Software Foundation, Inc. + + This file is part of GDB. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#ifndef GDB_FRAME_INFO_H +#define GDB_FRAME_INFO_H + +#include "gdbsupport/intrusive_list.h" + +struct frame_info; + +extern void reinit_frame_cache (); + +/* A wrapper for "frame_info *". frame_info objects are invalidated + whenever reinit_frame_cache is called. This class arranges to + invalidate the pointer when appropriate. This is done to help + detect a GDB bug that was relatively common. + + A small amount of code must still operate on raw pointers, so a + "get" method is provided. However, you should normally not use + this in new code. */ + +class frame_info_ptr : public intrusive_list_node +{ +public: + /* Create a frame_info_ptr from a raw pointer. */ + explicit frame_info_ptr (struct frame_info *ptr) + : m_ptr (ptr) + { + root.push_back (*this); + } + + /* Create a null frame_info_ptr. */ + frame_info_ptr () + { + root.push_back (*this); + } + + frame_info_ptr (std::nullptr_t) + { + root.push_back (*this); + } + + frame_info_ptr (const frame_info_ptr &other) + : m_ptr (other.m_ptr) + { + root.push_back (*this); + } + + frame_info_ptr (frame_info_ptr &&other) + : m_ptr (other.m_ptr) + { + other.m_ptr = nullptr; + root.push_back (*this); + } + + ~frame_info_ptr () + { + root.erase (*this); + } + + frame_info_ptr &operator= (const frame_info_ptr &other) + { + m_ptr = other.m_ptr; + return *this; + } + + frame_info_ptr &operator= (std::nullptr_t) + { + m_ptr = nullptr; + return *this; + } + + frame_info_ptr &operator= (frame_info_ptr &&other) + { + m_ptr = other.m_ptr; + other.m_ptr = nullptr; + return *this; + } + + frame_info *operator-> () const + { + return m_ptr; + } + + /* Fetch the underlying pointer. Note that new code should + generally not use this -- avoid it if at all possible. */ + frame_info *get () const + { + return m_ptr; + } + + /* This exists for compatibility with pre-existing code that checked + a "frame_info *" using "!". */ + bool operator! () const + { + return m_ptr == nullptr; + } + + /* This exists for compatibility with pre-existing code that checked + a "frame_info *" like "if (ptr)". */ + explicit operator bool () const + { + return m_ptr != nullptr; + } + + /* Invalidate this pointer. */ + void invalidate () + { + m_ptr = nullptr; + } + +private: + + /* The underlying pointer. */ + frame_info *m_ptr = nullptr; + + /* All frame_info_ptr objects are kept on a circular doubly-linked + list. This keeps their construction and destruction costs + reasonably small. To make the implementation a little simpler, + we guarantee that there is always at least one object on the list + -- this "root". */ + static intrusive_list root; + + /* A friend so it can invalidate the pointers. */ + friend void reinit_frame_cache (); +}; + +static inline bool +operator== (const frame_info *self, const frame_info_ptr &other) +{ + return self == other.get (); +} + +static inline bool +operator== (const frame_info_ptr &self, const frame_info_ptr &other) +{ + return self.get () == other.get (); +} + +static inline bool +operator== (const frame_info_ptr &self, const frame_info *other) +{ + return self.get () == other; +} + +static inline bool +operator!= (const frame_info *self, const frame_info_ptr &other) +{ + return self != other.get (); +} + +static inline bool +operator!= (const frame_info_ptr &self, const frame_info_ptr &other) +{ + return self.get () != other.get (); +} + +static inline bool +operator!= (const frame_info_ptr &self, const frame_info *other) +{ + return self.get () != other; +} + +#endif /* GDB_FRAME_INFO_H */ diff --git a/gdb/frame.c b/gdb/frame.c index c0cf3d585bf..7d62e01701d 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -56,6 +56,9 @@ static struct frame_info *sentinel_frame; /* Number of calls to reinit_frame_cache. */ static unsigned int frame_cache_generation = 0; +/* See frame-info.h. */ +intrusive_list frame_info_ptr::root; + /* See frame.h. */ unsigned int @@ -2006,6 +2009,9 @@ reinit_frame_cache (void) select_frame (NULL); frame_stash_invalidate (); + for (frame_info_ptr &iter : frame_info_ptr::root) + iter.invalidate (); + frame_debug_printf ("generation=%d", frame_cache_generation); } diff --git a/gdb/frame.h b/gdb/frame.h index 75bb3bd2aa0..9ad2599331f 100644 --- a/gdb/frame.h +++ b/gdb/frame.h @@ -20,6 +20,8 @@ #if !defined (FRAME_H) #define FRAME_H 1 +#include "frame-info.h" + /* The following is the intended naming schema for frame functions. It isn't 100% consistent, but it is approaching that. Frame naming schema: diff --git a/gdbsupport/intrusive_list.h b/gdbsupport/intrusive_list.h index 6812266159a..48b2123582f 100644 --- a/gdbsupport/intrusive_list.h +++ b/gdbsupport/intrusive_list.h @@ -391,13 +391,13 @@ class intrusive_list void pop_front () { gdb_assert (!this->empty ()); - erase_element (*m_front); + erase (*m_front); } void pop_back () { gdb_assert (!this->empty ()); - erase_element (*m_back); + erase (*m_back); } private: @@ -451,7 +451,8 @@ class intrusive_list m_back = &elem; } - void erase_element (T &elem) +public: + void erase (T &elem) { intrusive_list_node *elem_node = as_node (&elem); @@ -486,7 +487,6 @@ class intrusive_list elem_node->prev = INTRUSIVE_LIST_UNLINKED_VALUE; } -public: /* Remove the element pointed by I from the list. The element pointed by I is not destroyed. */ iterator erase (const_iterator i) @@ -494,7 +494,7 @@ class intrusive_list iterator ret = i; ++ret; - erase_element (*i); + erase (*i); return ret; } -- 2.31.1