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/stack.c | |
parent | bd2b40ac129b167f1a709589dee9c009a04a6e21 (diff) | |
download | gdb-c29a6445a981cee5e8eebe3617ef5c049fd3409f.zip gdb-c29a6445a981cee5e8eebe3617ef5c049fd3409f.tar.gz 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/stack.c')
-rw-r--r-- | gdb/stack.c | 11 |
1 files changed, 11 insertions, 0 deletions
diff --git a/gdb/stack.c b/gdb/stack.c index abdd6f3..4867669 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -362,11 +362,13 @@ print_stack_frame (frame_info_ptr frame, int print_level, if (current_uiout->is_mi_like_p ()) print_what = LOC_AND_ADDRESS; + frame.prepare_reinflate (); try { print_frame_info (user_frame_print_options, frame, print_level, print_what, 1 /* print_args */, set_current_sal); + frame.reinflate (); if (set_current_sal) set_current_sal_from_frame (frame); } @@ -742,6 +744,11 @@ print_frame_args (const frame_print_options &fp_opts, = (print_names && fp_opts.print_frame_arguments != print_frame_arguments_none); + /* If one of the arguments has a pretty printer that calls a + function of the inferior to print it, the pointer must be + reinflatable. */ + frame.prepare_reinflate (); + /* Temporarily change the selected frame to the given FRAME. This allows routines that rely on the selected frame instead of being given a frame as parameter to use the correct frame. */ @@ -902,6 +909,7 @@ print_frame_args (const frame_print_options &fp_opts, } first = 0; + frame.reinflate (); } } @@ -1172,6 +1180,7 @@ print_frame_info (const frame_print_options &fp_opts, print_source_lines (sal.symtab, sal.line, sal.line + 1, 0); } + frame.reinflate (); /* If disassemble-next-line is set to on and there is line debug messages, output assembly codes for next line. */ @@ -2061,6 +2070,7 @@ backtrace_command_1 (const frame_print_options &fp_opts, for (fi = trailing; fi && count--; fi = get_prev_frame (fi)) { QUIT; + fi.prepare_reinflate (); /* Don't use print_stack_frame; if an error() occurs it probably means further attempts to backtrace would fail (on the other @@ -2085,6 +2095,7 @@ backtrace_command_1 (const frame_print_options &fp_opts, } /* Save the last frame to check for error conditions. */ + fi.reinflate (); trailing = fi; } |