This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] Fix GDB crash after Quit thrown from unwinder sniffer


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=980548fd880338d2cdf4ce641ca39632dc040426

commit 980548fd880338d2cdf4ce641ca39632dc040426
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Feb 14 18:59:00 2018 +0000

    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  <palves@redhat.com>
    
    	* frame-unwind.c (frame_unwind_try_unwinder): Always call
    	frame_cleanup_after_sniffer on exception.
    
    gdb/testsuite/ChangeLog:
    2018-02-14  Pedro Alves  <palves@redhat.com>
    
    	* gdb.base/bp-cmds-continue-ctrl-c.exp (do_test): Test "bt" after
    	getting a "Quit".

Diff:
---
 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  <palves@redhat.com>
+
+	* frame-unwind.c (frame_unwind_try_unwinder): Always call
+	frame_cleanup_after_sniffer on exception.
+
 2018-02-14  Tom Tromey  <tom@tromey.com>
 
 	* solist.h (struct target_so_ops) <bfd_open>: 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  <palves@redhat.com>
+
+	* gdb.base/bp-cmds-continue-ctrl-c.exp (do_test): Test "bt" after
+	getting a "Quit".
+
 2018-02-09  Markus Metzger  <markus.t.metzger@intel.com>
 
 	* 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"


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]