aboutsummaryrefslogtreecommitdiff
path: root/gdb/frame-info.h
diff options
context:
space:
mode:
authorBruno Larsen <blarsen@redhat.com>2022-07-25 14:06:37 -0300
committerBruno Larsen <blarsen@redhat.com>2022-10-10 11:57:10 +0200
commitc29a6445a981cee5e8eebe3617ef5c049fd3409f (patch)
treebdaf7bc8b1e30feb35995b1be6d1e059c680e7fd /gdb/frame-info.h
parentbd2b40ac129b167f1a709589dee9c009a04a6e21 (diff)
downloadfsf-binutils-gdb-c29a6445a981cee5e8eebe3617ef5c049fd3409f.zip
fsf-binutils-gdb-c29a6445a981cee5e8eebe3617ef5c049fd3409f.tar.gz
fsf-binutils-gdb-c29a6445a981cee5e8eebe3617ef5c049fd3409f.tar.bz2
gdb/frame: Add reinflation method for frame_info_ptr
Currently, despite having a smart pointer for frame_infos, GDB may attempt to use an invalidated frame_info_ptr, which would cause internal errors to happen. One such example has been documented as PR python/28856, that happened when printing frame arguments calls an inferior function. To avoid failures, the smart wrapper was changed to also cache the frame id, so the pointer can be reinflated later. For this to work, the frame-id stuff had to be moved to their own .h file, which is included by frame-info.h. Frame_id caching is done explicitly using the prepare_reinflate method. Caching is done manually so that only the pointers that need to be saved will be, and reinflating has to be done manually using the reinflate method because the get method and the -> operator must not change the internals of the class. Finally, attempting to reinflate when the pointer is being invalidated causes the following assertion errors: check_ptrace_stopped_lwp_gone: assertion `lp->stopped` failed. get_frame_pc: Assertion `frame->next != NULL` failed. As for performance concerns, my personal testing with `time make chec-perf GDB_PERFTEST_MODE=run` showed an actual reduction of around 10% of time running. This commit also adds a testcase that exercises the python/28856 bug with 7 different triggers, run, continue, step, backtrace, finish, up and down. Some of them can seem to be testing the same thing twice, but since this test relies on stale pointers, there is always a chance that GDB got lucky when testing, so better to test extra. Regression tested on x86_64, using both gcc and clang. Approved-by: Tom Tomey <tom@tromey.com>
Diffstat (limited to 'gdb/frame-info.h')
-rw-r--r--gdb/frame-info.h31
1 files changed, 29 insertions, 2 deletions
diff --git a/gdb/frame-info.h b/gdb/frame-info.h
index 195aa5c..665f4bd 100644
--- a/gdb/frame-info.h
+++ b/gdb/frame-info.h
@@ -21,10 +21,15 @@
#define GDB_FRAME_INFO_H
#include "gdbsupport/intrusive_list.h"
+#include "frame-id.h"
struct frame_info;
+/* Forward declarations of functions, needed for the frame_info_ptr
+ to work correctly. */
extern void reinit_frame_cache ();
+extern struct frame_id get_frame_id (frame_info_ptr);
+extern frame_info_ptr frame_find_by_id (struct frame_id id);
/* A wrapper for "frame_info *". frame_info objects are invalidated
whenever reinit_frame_cache is called. This class arranges to
@@ -57,13 +62,13 @@ public:
}
frame_info_ptr (const frame_info_ptr &other)
- : m_ptr (other.m_ptr)
+ : m_ptr (other.m_ptr), m_cached_id (other.m_cached_id)
{
frame_list.push_back (*this);
}
frame_info_ptr (frame_info_ptr &&other)
- : m_ptr (other.m_ptr)
+ : m_ptr (other.m_ptr), m_cached_id (other.m_cached_id)
{
other.m_ptr = nullptr;
frame_list.push_back (*this);
@@ -77,19 +82,23 @@ public:
frame_info_ptr &operator= (const frame_info_ptr &other)
{
m_ptr = other.m_ptr;
+ m_cached_id = other.m_cached_id;
return *this;
}
frame_info_ptr &operator= (std::nullptr_t)
{
m_ptr = nullptr;
+ m_cached_id = null_frame_id;
return *this;
}
frame_info_ptr &operator= (frame_info_ptr &&other)
{
m_ptr = other.m_ptr;
+ m_cached_id = other.m_cached_id;
other.m_ptr = nullptr;
+ other.m_cached_id = null_frame_id;
return *this;
}
@@ -125,11 +134,29 @@ public:
m_ptr = nullptr;
}
+ /* Cache the frame_id that the pointer will use to reinflate. */
+ void prepare_reinflate ()
+ {
+ m_cached_id = get_frame_id (*this);
+ }
+
+ /* Use the cached frame_id to reinflate the pointer. */
+ void reinflate ()
+ {
+ gdb_assert (m_cached_id != null_frame_id);
+
+ if (m_ptr == nullptr)
+ m_ptr = frame_find_by_id (m_cached_id).get ();
+ gdb_assert (m_ptr != nullptr);
+ }
+
private:
/* The underlying pointer. */
frame_info *m_ptr = nullptr;
+ /* The frame_id of the underlying pointer. */
+ frame_id m_cached_id = null_frame_id;
/* All frame_info_ptr objects are kept on an intrusive list.
This keeps their construction and destruction costs