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] When disabling target async, remove all target event sources from the event loop


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

commit b7d2e91626b0e587f3fd5023e79b5079da6baed5
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Feb 3 16:07:54 2015 +0100

    When disabling target async, remove all target event sources from the event loop
    
    The sigall-reverse.exp test occasionally fails with something like this:
    
     (gdb) PASS: gdb.reverse/sigall-reverse.exp: send signal TERM
     continue
     Continuing.
     The next instruction is syscall exit_group.  It will make the program exit.  Do you want to stop the program?([y] or n) FAIL: gdb.reverse/sigall-reverse.exp: continue to signal exit (timeout)
     FAIL: gdb.reverse/sigall-reverse.exp: reverse to handler of TERM (timeout)
     FAIL: gdb.reverse/sigall-reverse.exp: reverse to gen_TERM (timeout)
    
    This is another event-loop/async related problem exposed by the patch
    that made 'query' use gdb_readline_wrapper (588dcc3edbde19f9).
    
    The problem is that even though gdb_readline_wrapper disables
    target-async while the secondary prompt is in progress, the record
    target's async event source is left marked.  So when
    gdb_readline_wrapper nests an event loop to process input, it may
    happen that that event loop ends up processing a target event while
    GDB is not really ready for it.  Here's the relevant part of the
    backtrace showing the root issue in action:
    
    ...
     #14 0x000000000061cb48 in fetch_inferior_event (client_data=0x0) at src/gdb/infrun.c:4158
     #15 0x0000000000642917 in inferior_event_handler (event_type=INF_REG_EVENT, client_data=0x0) at src/gdb/inf-loop.c:57
     #16 0x000000000077ca5c in record_full_async_inferior_event_handler (data=0x0) at src/gdb/record-full.c:791
     #17 0x0000000000640fdf in invoke_async_event_handler (data=...) at src/gdb/event-loop.c:1067
     #18 0x000000000063fb01 in process_event () at src/gdb/event-loop.c:339
     #19 0x000000000063fb2a in gdb_do_one_event () at src/gdb/event-loop.c:360
     #20 0x000000000074d607 in gdb_readline_wrapper (prompt=0x3588f40 "The next instruction is syscall exit_group.  It will make the program exit.  Do you want to stop the program?([y] or n) ") at src/gdb/top.c:842
     #21 0x0000000000750bd9 in defaulted_query (ctlstr=0x8c6588 "The next instruction is syscall exit_group.  It will make the program exit.  Do you want to stop the program?", defchar=121 'y', args=0x7fff70524410) at src/gdb/utils.c:1279
     #22 0x0000000000750e4c in yquery (ctlstr=0x8c6588 "The next instruction is syscall exit_group.  It will make the program exit.  Do you want to stop the program?") at src/gdb/utils.c:1358
     #23 0x00000000004b020e in record_linux_system_call (syscall=gdb_sys_exit_group, regcache=0x3529450, tdep=0xd6c840 <amd64_linux_record_tdep>) at src/gdb/linux-record.c:1933
    
    With my all-stop-on-top-of-non-stop series, I'm also seeing
    gdb.server/ext-attach.exp fail occasionally due to the same issue.
    
    The first part of the fix is for target_async implementations to make
    sure to remove/unmark all target-related event sources from the event
    loop.
    
    Tested on x86_64 Fedora 20, native and gdbserver.
    
    gdb/
    2015-02-03  Pedro Alves  <palves@redhat.com>
    
    	* event-loop.c (clear_async_event_handler): New function.
    	* event-loop.h (clear_async_event_handler): New declaration.
    	* record-btrace.c (record_btrace_async): New function.
    	(init_record_btrace_ops): Install record_btrace_async.
    	* record-full.c (record_full_async): New function.
    	(record_full_resume): Don't mark the async event source here.
    	(init_record_full_ops): Install record_full_async.
    	(record_full_core_resume): Don't mark the async event source here.
    	(init_record_full_core_ops): Install record_full_async.
    	* remote.c (remote_async): Mark and clear the async stop reply
    	queue event-loop token as appropriate.

Diff:
---
 gdb/ChangeLog       | 14 ++++++++++++++
 gdb/event-loop.c    |  8 ++++++++
 gdb/event-loop.h    |  4 ++++
 gdb/record-btrace.c | 17 +++++++++++++++++
 gdb/record-full.c   | 32 ++++++++++++++++++++------------
 gdb/remote.c        | 10 +++++++++-
 6 files changed, 72 insertions(+), 13 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e7e60bb..9a36dc8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2015-02-03  Pedro Alves  <palves@redhat.com>
 
+	* event-loop.c (clear_async_event_handler): New function.
+	* event-loop.h (clear_async_event_handler): New declaration.
+	* record-btrace.c (record_btrace_async): New function.
+	(init_record_btrace_ops): Install record_btrace_async.
+	* record-full.c (record_full_async): New function.
+	(record_full_resume): Don't mark the async event source here.
+	(init_record_full_ops): Install record_full_async.
+	(record_full_core_resume): Don't mark the async event source here.
+	(init_record_full_core_ops): Install record_full_async.
+	* remote.c (remote_async): Mark and clear the async stop reply
+	queue event-loop token as appropriate.
+
+2015-02-03  Pedro Alves  <palves@redhat.com>
+
 	* linux-nat.c (linux_child_follow_fork, linux_nat_wait_1): Use
 	target_is_async_p instead of target_can_async.
 	(linux_nat_wait): Use target_is_async_p instead of
diff --git a/gdb/event-loop.c b/gdb/event-loop.c
index cfdd9a6..e4de572 100644
--- a/gdb/event-loop.c
+++ b/gdb/event-loop.c
@@ -1050,6 +1050,14 @@ mark_async_event_handler (async_event_handler *async_handler_ptr)
   async_handler_ptr->ready = 1;
 }
 
+/* See event-loop.h.  */
+
+void
+clear_async_event_handler (async_event_handler *async_handler_ptr)
+{
+  async_handler_ptr->ready = 0;
+}
+
 struct async_event_handler_data
 {
   async_event_handler_func* proc;
diff --git a/gdb/event-loop.h b/gdb/event-loop.h
index feb5ea0..b9338a2 100644
--- a/gdb/event-loop.h
+++ b/gdb/event-loop.h
@@ -131,3 +131,7 @@ extern void
 /* Call the handler from HANDLER the next time through the event
    loop.  */
 extern void mark_async_event_handler (struct async_event_handler *handler);
+
+/* Mark the handler (ASYNC_HANDLER_PTR) as NOT ready.  */
+
+extern void clear_async_event_handler (struct async_event_handler *handler);
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 3b7ef5c..7af549f 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -266,6 +266,22 @@ record_btrace_close (struct target_ops *self)
     btrace_teardown (tp);
 }
 
+/* The to_async method of target record-btrace.  */
+
+static void
+record_btrace_async (struct target_ops *ops,
+		     void (*callback) (enum inferior_event_type event_type,
+				       void *context),
+		     void *context)
+{
+  if (callback != NULL)
+    mark_async_event_handler (record_btrace_async_inferior_event_handler);
+  else
+    clear_async_event_handler (record_btrace_async_inferior_event_handler);
+
+  ops->beneath->to_async (ops->beneath, callback, context);
+}
+
 /* The to_info_record method of target record-btrace.  */
 
 static void
@@ -1940,6 +1956,7 @@ init_record_btrace_ops (void)
   ops->to_doc = "Collect control-flow trace and provide the execution history.";
   ops->to_open = record_btrace_open;
   ops->to_close = record_btrace_close;
+  ops->to_async = record_btrace_async;
   ops->to_detach = record_detach;
   ops->to_disconnect = record_disconnect;
   ops->to_mourn_inferior = record_mourn_inferior;
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 6db16d6..c660743 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -910,6 +910,22 @@ record_full_close (struct target_ops *self)
     delete_async_event_handler (&record_full_async_inferior_event_token);
 }
 
+/* "to_async" target method.  */
+
+static void
+record_full_async (struct target_ops *ops,
+		   void (*callback) (enum inferior_event_type event_type,
+				     void *context),
+		   void *context)
+{
+  if (callback != NULL)
+    mark_async_event_handler (record_full_async_inferior_event_token);
+  else
+    clear_async_event_handler (record_full_async_inferior_event_token);
+
+  ops->beneath->to_async (ops->beneath, callback, context);
+}
+
 static int record_full_resume_step = 0;
 
 /* True if we've been resumed, and so each record_full_wait call should
@@ -989,12 +1005,7 @@ record_full_resume (struct target_ops *ops, ptid_t ptid, int step,
   /* We are about to start executing the inferior (or simulate it),
      let's register it with the event loop.  */
   if (target_can_async_p ())
-    {
-      target_async (inferior_event_handler, 0);
-      /* Notify the event loop there's an event to wait for.  We do
-	 most of the work in record_full_wait.  */
-      mark_async_event_handler (record_full_async_inferior_event_token);
-    }
+    target_async (inferior_event_handler, 0);
 }
 
 static int record_full_get_sig = 0;
@@ -1902,6 +1913,7 @@ init_record_full_ops (void)
     "Log program while executing and replay execution from log.";
   record_full_ops.to_open = record_full_open;
   record_full_ops.to_close = record_full_close;
+  record_full_ops.to_async = record_full_async;
   record_full_ops.to_resume = record_full_resume;
   record_full_ops.to_wait = record_full_wait;
   record_full_ops.to_disconnect = record_disconnect;
@@ -1943,12 +1955,7 @@ record_full_core_resume (struct target_ops *ops, ptid_t ptid, int step,
   /* We are about to start executing the inferior (or simulate it),
      let's register it with the event loop.  */
   if (target_can_async_p ())
-    {
-      target_async (inferior_event_handler, 0);
-
-      /* Notify the event loop there's an event to wait for.  */
-      mark_async_event_handler (record_full_async_inferior_event_token);
-    }
+    target_async (inferior_event_handler, 0);
 }
 
 /* "to_kill" method for prec over corefile.  */
@@ -2141,6 +2148,7 @@ init_record_full_core_ops (void)
     "Log program while executing and replay execution from log.";
   record_full_core_ops.to_open = record_full_open;
   record_full_core_ops.to_close = record_full_close;
+  record_full_core_ops.to_async = record_full_async;
   record_full_core_ops.to_resume = record_full_core_resume;
   record_full_core_ops.to_wait = record_full_wait;
   record_full_core_ops.to_kill = record_full_core_kill;
diff --git a/gdb/remote.c b/gdb/remote.c
index 9be15cb..e971a29 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11691,9 +11691,17 @@ remote_async (struct target_ops *ops,
       serial_async (rs->remote_desc, remote_async_serial_handler, rs);
       rs->async_client_callback = callback;
       rs->async_client_context = context;
+
+      /* If there are pending events in the stop reply queue tell the
+	 event loop to process them.  */
+      if (!QUEUE_is_empty (stop_reply_p, stop_reply_queue))
+	mark_async_event_handler (remote_async_inferior_event_token);
     }
   else
-    serial_async (rs->remote_desc, NULL, NULL);
+    {
+      serial_async (rs->remote_desc, NULL, NULL);
+      clear_async_event_handler (remote_async_inferior_event_token);
+    }
 }
 
 static void


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