From: Bruno Larsen <blarsen@redhat.com>
To: gdb-patches@sourceware.org
Cc: blarsen@redhat.com, Tom Tromey <tom@tromey.com>
Subject: [PATCH v4 2/5] Introduce frame_info_ptr smart pointer class
Date: Tue, 30 Aug 2022 12:08:34 +0200 [thread overview]
Message-ID: <20220830100837.926692-3-blarsen@redhat.com> (raw)
In-Reply-To: <20220830100837.926692-1-blarsen@redhat.com>
From: Tom Tromey <tom@tromey.com>
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 <blarsen@redhat.com>
---
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 <http://www.gnu.org/licenses/>. */
+
+#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<frame_info_ptr>
+{
+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_info_ptr> 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_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
next prev parent reply other threads:[~2022-08-30 10:08 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-30 10:08 [PATCH v4 0/5] Smart pointer wrapper for frame_info Bruno Larsen
2022-08-30 10:08 ` [PATCH v4 1/5] Remove frame_id_eq Bruno Larsen
2022-08-30 10:08 ` Bruno Larsen [this message]
2022-08-30 10:08 ` [PATCH v4 3/5] Change GDB to use frame_info_ptr Bruno Larsen
2022-08-30 10:08 ` [PATCH v4 4/5] Continue making GDB " Bruno Larsen
2022-08-30 10:08 ` [PATCH v4 5/5] gdb/frame: Add reinflation method for frame_info_ptr Bruno Larsen
2022-10-07 19:34 ` Tom Tromey
2022-10-10 7:54 ` Bruno Larsen
2022-10-11 7:58 ` Tom de Vries
2022-10-11 9:42 ` Bruno Larsen
2022-09-13 8:06 ` [Ping][PATCH v4 0/5] Smart pointer wrapper for frame_info Bruno Larsen
2022-09-21 15:39 ` [PINGv2][PATCH " Bruno Larsen
2022-09-29 7:01 ` [PINGv3][PATCH " Bruno Larsen
2022-10-05 9:58 ` [PINGv4][PATCH " Bruno Larsen
2022-10-07 19:35 ` Tom Tromey
2022-10-10 7:55 ` Bruno Larsen
2022-10-10 10:46 ` Tom de Vries
2022-10-10 12:04 ` Bruno Larsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220830100837.926692-3-blarsen@redhat.com \
--to=blarsen@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).