From 980548fd880338d2cdf4ce641ca39632dc040426 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 14 Feb 2018 18:59:00 +0000 Subject: [PATCH] Fix GDB crash after Quit thrown from unwinder sniffer I ran into a GDB crash in gdb.base/bp-cmds-continue-ctrl-c.exp in my multi-target branch, which turns out exposed a bug that exists in master too. That testcase has a breakpoint with a "continue" command associated. Then the breakpoint is constantly being hit. At the same time, the testcase is continualy interrupting the program with Ctrl-C, and re-resuming it, in a loop. Running that testcase manually under Valgrind, after a few sequences of 'Ctrl-C' + 'continue', I got: Breakpoint 1, Quit (gdb) ==21270== Invalid read of size 8 ==21270== at 0x4D8185: pyuw_this_id(frame_info*, void**, frame_id*) (py-unwind.c:461) ==21270== by 0x6D426A: compute_frame_id(frame_info*) (frame.c:505) ==21270== by 0x6D43B7: get_frame_id(frame_info*) (frame.c:537) ==21270== by 0x84F3B8: scoped_restore_current_thread::scoped_restore_current_thread() (thread.c:1678) ==21270== by 0x718E3D: fetch_inferior_event(void*) (infrun.c:4076) ==21270== by 0x7067C9: inferior_event_handler(inferior_event_type, void*) (inf-loop.c:43) ==21270== by 0x45BEF9: handle_target_event(int, void*) (linux-nat.c:4419) ==21270== by 0x6C4255: handle_file_event(file_handler*, int) (event-loop.c:733) ==21270== by 0x6C47F8: gdb_wait_for_event(int) (event-loop.c:859) ==21270== by 0x6C3666: gdb_do_one_event() (event-loop.c:322) ==21270== by 0x6C3712: start_event_loop() (event-loop.c:371) ==21270== by 0x746801: captured_command_loop() (main.c:329) ==21270== Address 0x0 is not stack'd, malloc'd or (recently) free'd ==21270== ==21270== ==21270== Process terminating with default action of signal 11 (SIGSEGV): dumping core ==21270== Access not within mapped region at address 0x0 ==21270== at 0x4D8185: pyuw_this_id(frame_info*, void**, frame_id*) (py-unwind.c:461) ==21270== by 0x6D426A: compute_frame_id(frame_info*) (frame.c:505) ==21270== by 0x6D43B7: get_frame_id(frame_info*) (frame.c:537) ==21270== by 0x84F3B8: scoped_restore_current_thread::scoped_restore_current_thread() (thread.c:1678) ==21270== by 0x718E3D: fetch_inferior_event(void*) (infrun.c:4076) ==21270== by 0x7067C9: inferior_event_handler(inferior_event_type, void*) (inf-loop.c:43) ==21270== by 0x45BEF9: handle_target_event(int, void*) (linux-nat.c:4419) ==21270== by 0x6C4255: handle_file_event(file_handler*, int) (event-loop.c:733) ==21270== by 0x6C47F8: gdb_wait_for_event(int) (event-loop.c:859) ==21270== by 0x6C3666: gdb_do_one_event() (event-loop.c:322) ==21270== by 0x6C3712: start_event_loop() (event-loop.c:371) ==21270== by 0x746801: captured_command_loop() (main.c:329) ==21270== If you believe this happened as a result of a stack ==21270== overflow in your program's main thread (unlikely but ==21270== possible), you can try to increase the size of the ==21270== main thread stack using the --main-stacksize= flag. ==21270== The main thread stack size used in this run was 8388608. ==21270== Above, when we get to compute_frame_id, fi->unwind is non-NULL, meaning, we found an unwinder, in this case the Python unwinder, but somehow, fi->prologue_cache is left NULL. pyuw_this_id then crashes because it assumes fi->prologue_cache is non-NULL: static void pyuw_this_id (struct frame_info *this_frame, void **cache_ptr, struct frame_id *this_id) { *this_id = ((cached_frame_info *) *cache_ptr)->frame_id; ^^^^^^^^^^ '*cache_ptr' here is 'fi->prologue_cache'. There's a quit() call in pyuw_sniffer that I believe is the one that sometimes triggers the crash above. The crash can be reproduced easily with this hack to force a quit out of the python unwinder: --- a/gdb/python/py-unwind.c +++ b/gdb/python/py-unwind.c @@ -497,6 +497,8 @@ pyuw_sniffer (const struct frame_unwind *self, struct frame_info *this_frame, struct gdbarch *gdbarch = (struct gdbarch *) (self->unwind_data); cached_frame_info *cached_frame; + quit (); + gdbpy_enter enter_py (gdbarch, current_language); TRACE_PY_UNWIND (3, "%s (SP=%s, PC=%s)\n", __FUNCTION__, After that quit is thrown, any subsequent operation that involves unwinding results in GDB crashing with SIGSEGV like above. The problem is that this commit: commit 30a9c02feff56bd58a276c2a7262f364baa558ac CommitDate: Sun Oct 8 23:16:42 2017 -0600 Subject: Remove cleanup from frame_prepare_for_sniffer missed that we need to call frame_cleanup_after_sniffer before rethrowing the exception too. Without the fix, the "bt" added to gdb.base/bp-cmds-continue-ctrl-c.exp in this commit makes GDB crash: Running src/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.exp ... ERROR: Process no longer exists gdb/ChangeLog: 2018-02-14 Pedro Alves * frame-unwind.c (frame_unwind_try_unwinder): Always call frame_cleanup_after_sniffer on exception. gdb/testsuite/ChangeLog: 2018-02-14 Pedro Alves * gdb.base/bp-cmds-continue-ctrl-c.exp (do_test): Test "bt" after getting a "Quit". --- gdb/ChangeLog | 5 +++++ gdb/frame-unwind.c | 3 ++- gdb/testsuite/ChangeLog | 5 +++++ gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.exp | 13 +++++++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 7b4c5b4..947bed4 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2018-02-14 Pedro Alves + + * frame-unwind.c (frame_unwind_try_unwinder): Always call + frame_cleanup_after_sniffer on exception. + 2018-02-14 Tom Tromey * solist.h (struct target_so_ops) : Make pathname diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c index 66a28ae..e6e6353 100644 --- a/gdb/frame-unwind.c +++ b/gdb/frame-unwind.c @@ -110,13 +110,14 @@ frame_unwind_try_unwinder (struct frame_info *this_frame, void **this_cache, /* Catch all exceptions, caused by either interrupt or error. Reset *THIS_CACHE. */ *this_cache = NULL; + frame_cleanup_after_sniffer (this_frame); + if (ex.error == NOT_AVAILABLE_ERROR) { /* This usually means that not even the PC is available, thus most unwinders aren't able to determine if they're the best fit. Keep trying. Fallback prologue unwinders should always accept the frame. */ - frame_cleanup_after_sniffer (this_frame); return 0; } throw_exception (ex); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 1a4337c..8bcb50e 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2018-02-14 Pedro Alves + + * gdb.base/bp-cmds-continue-ctrl-c.exp (do_test): Test "bt" after + getting a "Quit". + 2018-02-09 Markus Metzger * lib/gdb.exp (skip_btrace_pt_tests): Update expected error message. diff --git a/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.exp b/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.exp index 8c2fa77..43600fc 100644 --- a/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.exp +++ b/gdb/testsuite/gdb.base/bp-cmds-continue-ctrl-c.exp @@ -89,6 +89,19 @@ proc do_test {} { } -re "Quit\r\n$gdb_prompt $" { send_log "$internal_pass (Quit)\n" + + # Check that if we managed to quit somewhere deep in + # the unwinders, we can still unwind again. + set ok 0 + gdb_test_multiple "bt" "$internal_pass (bt)" { + -re "#0.*$gdb_prompt $" { + send_log "$internal_pass (bt)\n" + set ok 1 + } + } + if {!$ok} { + return + } } -re "Quit\r\n\r\nCommand aborted.\r\n$gdb_prompt $" { send_log "$internal_pass (Command aborted)\n" -- 2.7.4