From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id BC8E1385516E for ; Wed, 14 Dec 2022 03:41:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BC8E1385516E Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 2BE3etfT031187 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 13 Dec 2022 22:40:59 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 2BE3etfT031187 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1670989260; bh=JZxtvc53eJFc2R1zVDoEOIBBfnwfck1uT+suiSHQb28=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=oZLcS+RG9q58RnpHbMhIBTjDs8YnsjS7n9IhCr/DIn/jwZY+y0m7J4F8KmtOELJL0 G2/oQ+dGIBtt8sZVTxC0PyNC8y/3Alaaj435cPRLYgPfloEpjspKdiPRwn1MA2RfUA de61IBRxntna5pOdns6JmzgKS7t3Ccr3Z9YIsmwU= Received: from simark.localdomain (unknown [217.28.27.60]) (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 ESMTPSA id 69F7B1E12C; Tue, 13 Dec 2022 22:34:44 -0500 (EST) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH v2 06/13] gdb: move frame_info_ptr to frame.{c,h} Date: Tue, 13 Dec 2022 22:34:34 -0500 Message-Id: <20221214033441.499512-7-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.38.1 In-Reply-To: <20221214033441.499512-1-simon.marchi@polymtl.ca> References: <20221214033441.499512-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 14 Dec 2022 03:40:55 +0000 X-Spam-Status: No, score=-3189.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Change-Id: Ic5949759e6262ea0da6123858702d48fe5673fea --- gdb/Makefile.in | 1 - gdb/c-lang.h | 1 + gdb/dwarf2/call-site.h | 2 +- gdb/frame-info.c | 65 ------------- gdb/frame-info.h | 211 ----------------------------------------- gdb/frame.c | 43 ++++++++- gdb/frame.h | 188 +++++++++++++++++++++++++++++++++++- gdb/gdbtypes.h | 2 - gdb/symtab.h | 1 + 9 files changed, 230 insertions(+), 284 deletions(-) delete mode 100644 gdb/frame-info.c delete mode 100644 gdb/frame-info.h diff --git a/gdb/Makefile.in b/gdb/Makefile.in index fb4d42c7baad..0f5df2ccb7b0 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -1088,7 +1088,6 @@ COMMON_SFILES = \ findvar.c \ frame.c \ frame-base.c \ - frame-info.c \ frame-unwind.c \ gcore.c \ gdb-demangle.c \ diff --git a/gdb/c-lang.h b/gdb/c-lang.h index c2db16e0bf07..1dbcfeb8f102 100644 --- a/gdb/c-lang.h +++ b/gdb/c-lang.h @@ -25,6 +25,7 @@ struct ui_file; struct language_arch_info; struct type_print_options; struct parser_state; +struct compile_instance; #include "compile/compile.h" #include "value.h" diff --git a/gdb/dwarf2/call-site.h b/gdb/dwarf2/call-site.h index f5bee967594e..c8a1e8b2971d 100644 --- a/gdb/dwarf2/call-site.h +++ b/gdb/dwarf2/call-site.h @@ -23,8 +23,8 @@ #define CALL_SITE_H #include "dwarf2/types.h" +#include "../frame.h" #include "gdbsupport/function-view.h" -#include "frame-info.h" struct dwarf2_locexpr_baton; struct dwarf2_per_cu_data; diff --git a/gdb/frame-info.c b/gdb/frame-info.c deleted file mode 100644 index 40a872ea152d..000000000000 --- a/gdb/frame-info.c +++ /dev/null @@ -1,65 +0,0 @@ -/* 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 . */ - -#include "defs.h" - -#include "frame-info.h" -#include "frame.h" - -/* See frame-info-ptr.h. */ - -intrusive_list frame_info_ptr::frame_list; - -/* See frame-info-ptr.h. */ - -void -frame_info_ptr::prepare_reinflate () -{ - m_cached_level = frame_relative_level (*this); - - if (m_cached_level != 0) - m_cached_id = get_frame_id (*this); -} - -/* See frame-info-ptr.h. */ - -void -frame_info_ptr::reinflate () -{ - /* Ensure we have a valid frame level (sentinel frame or above), indicating - prepare_reinflate was called. */ - gdb_assert (m_cached_level >= -1); - - if (m_ptr != nullptr) - { - /* The frame_info wasn't invalidated, no need to reinflate. */ - return; - } - - /* Frame #0 needs special handling, see comment in select_frame. */ - if (m_cached_level == 0) - m_ptr = get_current_frame ().get (); - else - { - gdb_assert (frame_id_p (m_cached_id)); - m_ptr = frame_find_by_id (m_cached_id).get (); - } - - gdb_assert (m_ptr != nullptr); -} diff --git a/gdb/frame-info.h b/gdb/frame-info.h deleted file mode 100644 index 893b6632363d..000000000000 --- a/gdb/frame-info.h +++ /dev/null @@ -1,211 +0,0 @@ -/* 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" -#include "frame-id.h" - -struct frame_info; - -/* 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), - m_cached_id (other.m_cached_id), - m_cached_level (other.m_cached_level) - { - frame_list.push_back (*this); - } - - frame_info_ptr (frame_info_ptr &&other) - : m_ptr (other.m_ptr), - m_cached_id (other.m_cached_id), - m_cached_level (other.m_cached_level) - { - other.m_ptr = nullptr; - other.m_cached_id = null_frame_id; - other.m_cached_level = invalid_level; - frame_list.push_back (*this); - } - - ~frame_info_ptr () - { - /* If this node has static storage, it may be deleted after - frame_list. Attempting to erase ourselves would then trigger - internal errors, so make sure we are still linked first. */ - if (is_linked ()) - frame_list.erase (frame_list.iterator_to (*this)); - } - - frame_info_ptr &operator= (const frame_info_ptr &other) - { - m_ptr = other.m_ptr; - m_cached_id = other.m_cached_id; - m_cached_level = other.m_cached_level; - return *this; - } - - frame_info_ptr &operator= (std::nullptr_t) - { - m_ptr = nullptr; - m_cached_id = null_frame_id; - m_cached_level = invalid_level; - return *this; - } - - frame_info_ptr &operator= (frame_info_ptr &&other) - { - m_ptr = other.m_ptr; - m_cached_id = other.m_cached_id; - m_cached_level = other.m_cached_level; - other.m_ptr = nullptr; - other.m_cached_id = null_frame_id; - other.m_cached_level = invalid_level; - 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; - } - - /* Cache the frame_id that the pointer will use to reinflate. */ - void prepare_reinflate (); - - /* Use the cached frame_id to reinflate the pointer. */ - void reinflate (); - -private: - /* We sometimes need to construct frame_info_ptr objects around the - sentinel_frame, which has level -1. Therefore, make the invalid frame - level value -2. */ - static constexpr int invalid_level = -2; - - /* The underlying pointer. */ - frame_info *m_ptr = nullptr; - - /* The frame_id of the underlying pointer. */ - frame_id m_cached_id = null_frame_id; - - /* The frame level of the underlying pointer. */ - int m_cached_level = invalid_level; - - /* 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 6a6d968b9a97..ba06f9b0b3e4 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -19,7 +19,6 @@ #include "defs.h" #include "frame.h" -#include "frame-info.h" #include "target.h" #include "value.h" #include "inferior.h" /* for inferior_ptid */ @@ -3161,6 +3160,48 @@ maintenance_print_frame_id (const char *args, int from_tty) get_frame_id (frame).to_string ().c_str ()); } +/* See frame-info-ptr.h. */ + +intrusive_list frame_info_ptr::frame_list; + +/* See frame-info-ptr.h. */ + +void +frame_info_ptr::prepare_reinflate () +{ + m_cached_level = frame_relative_level (*this); + + if (m_cached_level != 0) + m_cached_id = get_frame_id (*this); +} + +/* See frame-info-ptr.h. */ + +void +frame_info_ptr::reinflate () +{ + /* Ensure we have a valid frame level (sentinel frame or above), indicating + prepare_reinflate was called. */ + gdb_assert (m_cached_level >= -1); + + if (m_ptr != nullptr) + { + /* The frame_info wasn't invalidated, no need to reinflate. */ + return; + } + + /* Frame #0 needs special handling, see comment in select_frame. */ + if (m_cached_level == 0) + m_ptr = get_current_frame ().get (); + else + { + gdb_assert (frame_id_p (m_cached_id)); + m_ptr = frame_find_by_id (m_cached_id).get (); + } + + gdb_assert (m_ptr != nullptr); +} + void _initialize_frame (); void _initialize_frame () diff --git a/gdb/frame.h b/gdb/frame.h index 5c292cd2b952..ebbd9e08868e 100644 --- a/gdb/frame.h +++ b/gdb/frame.h @@ -20,8 +20,6 @@ #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: @@ -72,7 +70,9 @@ */ #include "cli/cli-option.h" +#include "frame-id.h" #include "gdbsupport/common-debug.h" +#include "gdbsupport/intrusive_list.h" struct symtab_and_line; struct frame_unwind; @@ -85,7 +85,6 @@ struct frame_print_options; /* The frame object. */ -class frame_info_ptr; /* Save and restore the currently selected frame. */ @@ -194,6 +193,189 @@ enum frame_type SENTINEL_FRAME }; +/* 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), + m_cached_id (other.m_cached_id), + m_cached_level (other.m_cached_level) + { + frame_list.push_back (*this); + } + + frame_info_ptr (frame_info_ptr &&other) + : m_ptr (other.m_ptr), + m_cached_id (other.m_cached_id), + m_cached_level (other.m_cached_level) + { + other.m_ptr = nullptr; + other.m_cached_id = null_frame_id; + other.m_cached_level = invalid_level; + frame_list.push_back (*this); + } + + ~frame_info_ptr () + { + /* If this node has static storage, it may be deleted after + frame_list. Attempting to erase ourselves would then trigger + internal errors, so make sure we are still linked first. */ + if (is_linked ()) + frame_list.erase (frame_list.iterator_to (*this)); + } + + frame_info_ptr &operator= (const frame_info_ptr &other) + { + m_ptr = other.m_ptr; + m_cached_id = other.m_cached_id; + m_cached_level = other.m_cached_level; + return *this; + } + + frame_info_ptr &operator= (std::nullptr_t) + { + m_ptr = nullptr; + m_cached_id = null_frame_id; + m_cached_level = invalid_level; + return *this; + } + + frame_info_ptr &operator= (frame_info_ptr &&other) + { + m_ptr = other.m_ptr; + m_cached_id = other.m_cached_id; + m_cached_level = other.m_cached_level; + other.m_ptr = nullptr; + other.m_cached_id = null_frame_id; + other.m_cached_level = invalid_level; + 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; + } + + /* Cache the frame_id that the pointer will use to reinflate. */ + void prepare_reinflate (); + + /* Use the cached frame_id to reinflate the pointer. */ + void reinflate (); + +private: + /* We sometimes need to construct frame_info_ptr objects around the + sentinel_frame, which has level -1. Therefore, make the invalid frame + level value -2. */ + static constexpr int invalid_level = -2; + + /* The underlying pointer. */ + frame_info *m_ptr = nullptr; + + /* The frame_id of the underlying pointer. */ + frame_id m_cached_id = null_frame_id; + + /* The frame level of the underlying pointer. */ + int m_cached_level = invalid_level; + + /* 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; +} + /* For every stopped thread, GDB tracks two frames: current and selected. Current frame is the inner most frame of the selected thread. Selected frame is the one being examined by the GDB diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index d5b966dc73f1..27499853d060 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -56,7 +56,6 @@ #include "dwarf2.h" #include "gdbsupport/gdb_obstack.h" #include "gmp-utils.h" -#include "frame-info.h" /* Forward declarations for prototypes. */ struct field; @@ -1734,7 +1733,6 @@ struct func_type struct type *self_type; }; - /* The type-specific info for TYPE_CODE_FIXED_POINT types. */ struct fixed_point_type_info diff --git a/gdb/symtab.h b/gdb/symtab.h index 4f3e84bbbe93..7b8704f83087 100644 --- a/gdb/symtab.h +++ b/gdb/symtab.h @@ -37,6 +37,7 @@ #include "completer.h" #include "gdb-demangle.h" #include "split-name.h" +#include "frame.h" /* Opaque declarations. */ struct ui_file; -- 2.38.1