From ba380b3e5162e89c4c81a73f4fb9fcbbbbe75e24 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Mon, 25 Jul 2022 14:06:34 -0300 Subject: [PATCH] Introduce frame_info_ptr smart pointer class 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 Approved-by: Tom Tomey --- 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 8df5feb4825..44cb52910f9 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.30.2