From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 4DA24382F09E for ; Tue, 30 Aug 2022 10:08:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4DA24382F09E Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-591-OVUstgeeMZ-uLgwEMUeu0A-1; Tue, 30 Aug 2022 06:08:45 -0400 X-MC-Unique: OVUstgeeMZ-uLgwEMUeu0A-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id EAE22185A7B2; Tue, 30 Aug 2022 10:08:44 +0000 (UTC) Received: from fedora.redhat.com (unknown [10.43.2.105]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5F0449458A; Tue, 30 Aug 2022 10:08:44 +0000 (UTC) From: Bruno Larsen To: gdb-patches@sourceware.org Cc: blarsen@redhat.com, Tom Tromey Subject: [PATCH v4 2/5] Introduce frame_info_ptr smart pointer class Date: Tue, 30 Aug 2022 12:08:34 +0200 Message-Id: <20220830100837.926692-3-blarsen@redhat.com> In-Reply-To: <20220830100837.926692-1-blarsen@redhat.com> References: <20220830100837.926692-1-blarsen@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 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-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org 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: , X-List-Received-Date: Tue, 30 Aug 2022 10:09:00 -0000 From: Tom Tromey This adds frame_info_ptr, a smart pointer class. Every instance of the class is kept on an intrusive 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 | 179 +++++++++++++++++++++++++++++++++++++++++++++++ gdb/frame.c | 6 ++ gdb/frame.h | 2 + 3 files changed, 187 insertions(+) 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..195aa5ccf03 --- /dev/null +++ b/gdb/frame-info.h @@ -0,0 +1,179 @@ +/* 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) + { + frame_list.push_back (*this); + } + + /* Create a null frame_info_ptr. */ + frame_info_ptr () + { + frame_list.push_back (*this); + } + + frame_info_ptr (std::nullptr_t) + { + frame_list.push_back (*this); + } + + frame_info_ptr (const frame_info_ptr &other) + : m_ptr (other.m_ptr) + { + frame_list.push_back (*this); + } + + frame_info_ptr (frame_info_ptr &&other) + : m_ptr (other.m_ptr) + { + other.m_ptr = nullptr; + frame_list.push_back (*this); + } + + ~frame_info_ptr () + { + frame_list.erase (frame_list.iterator_to (*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 an intrusive list. + This keeps their construction and destruction costs + reasonably small. */ + static intrusive_list frame_list; + + /* 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..7c31da53729 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::frame_list; + /* 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::frame_list) + 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: -- 2.37.2