diff options
author | Bruno Larsen <blarsen@redhat.com> | 2022-07-25 14:06:37 -0300 |
---|---|---|
committer | Bruno Larsen <blarsen@redhat.com> | 2022-10-10 11:57:10 +0200 |
commit | c29a6445a981cee5e8eebe3617ef5c049fd3409f (patch) | |
tree | bdaf7bc8b1e30feb35995b1be6d1e059c680e7fd /gdb/frame-info.h | |
parent | bd2b40ac129b167f1a709589dee9c009a04a6e21 (diff) | |
download | fsf-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.h | 31 |
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 |