diff options
author | Pedro Alves <pedro@palves.net> | 2020-07-23 16:29:28 +0100 |
---|---|---|
committer | Pedro Alves <pedro@palves.net> | 2020-07-23 16:29:28 +0100 |
commit | e7bc9db8f447e056f4faa11702230239b4075c2c (patch) | |
tree | 0ee125f1e067bcff441fb6f289ff20fcd02cb24d /gdb/frame.c | |
parent | 90fcc46681979a7759d16e738a98c9e666e6e88f (diff) | |
download | gdb-e7bc9db8f447e056f4faa11702230239b4075c2c.zip gdb-e7bc9db8f447e056f4faa11702230239b4075c2c.tar.gz gdb-e7bc9db8f447e056f4faa11702230239b4075c2c.tar.bz2 |
Don't touch frame_info objects if frame cache was reinitialized
This fixes yet another bug exposed by ASAN + multi-target.exp
Running an Asan-enabled GDB against gdb.multi/multi-target.exp exposed
yet another latent GDB bug. See here for the full log:
https://sourceware.org/pipermail/gdb-patches/2020-July/170761.html
As Simon described, the problem is:
- We create a new frame_info object in restore_selected_frame (by
calling find_relative_frame)
- The frame is allocated on the frame_cache_obstack
- In frame_unwind_try_unwinder, we try to find an unwinder for that
frame
- While trying unwinders, memory read fails because the remote target
closes, because of "monitor exit"
- That calls reinit_frame_cache (as shown above), which resets
frame_cache_obstack
- When handling the exception in frame_unwind_try_unwinder, we try to
set some things on the frame_info object (like *this_cache, which
in fact tries to write into frame_info::prologue_cache), but the
frame_info object is no more, it went away with the obstack.
Fix this by maintaining a frame cache generation counter. Then in
exception handling code paths, don't touch frame objects if the
generation is not the same as it was on entry.
This commit generalizes the gdb.server/server-kill.exp testcase and
reuses it to test the scenario in question. The new tests fail
without the GDB fix.
gdb/ChangeLog:
* frame-unwind.c (frame_unwind_try_unwinder): On exception, don't
touch THIS_CACHE/THIS_FRAME if the frame cache was cleared
meanwhile.
* frame.c (frame_cache_generation, get_frame_cache_generation):
New.
(reinit_frame_cache): Increment FRAME_CACHE_GENERATION.
(get_prev_frame_if_no_cycle): On exception, don't touch
PREV_FRAME/THIS_FRAME if the frame cache was cleared meanwhile.
* frame.h (get_frame_cache_generation): Declare.
gdb/testsuite/ChangeLog:
* gdb.server/server-kill.exp (prepare): New, factored out from the
top level.
(kill_server): New.
(test_tstatus, test_unwind_nosyms, test_unwind_syms): New.
(top level) : Call test_tstatus, test_unwind_nosyms, test_unwind_syms.
Diffstat (limited to 'gdb/frame.c')
-rw-r--r-- | gdb/frame.c | 22 |
1 files changed, 20 insertions, 2 deletions
diff --git a/gdb/frame.c b/gdb/frame.c index ac1016b..a3599e8 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -53,6 +53,17 @@ static struct frame_info *sentinel_frame; +/* Number of calls to reinit_frame_cache. */ +static unsigned int frame_cache_generation = 0; + +/* See frame.h. */ + +unsigned int +get_frame_cache_generation () +{ + return frame_cache_generation; +} + /* The values behind the global "set backtrace ..." settings. */ set_backtrace_options user_set_backtrace_options; @@ -1843,6 +1854,8 @@ reinit_frame_cache (void) { struct frame_info *fi; + ++frame_cache_generation; + /* Tear down all frame caches. */ for (fi = sentinel_frame; fi != NULL; fi = fi->prev) { @@ -1922,6 +1935,8 @@ get_prev_frame_if_no_cycle (struct frame_info *this_frame) if (prev_frame->level == 0) return prev_frame; + unsigned int entry_generation = get_frame_cache_generation (); + try { compute_frame_id (prev_frame); @@ -1944,8 +1959,11 @@ get_prev_frame_if_no_cycle (struct frame_info *this_frame) } catch (const gdb_exception &ex) { - prev_frame->next = NULL; - this_frame->prev = NULL; + if (get_frame_cache_generation () == entry_generation) + { + prev_frame->next = NULL; + this_frame->prev = NULL; + } throw; } |