This is the mail archive of the gdb-patches@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]

[RFC] Patch for QUIT macro support


If you use gdb to debug a large executable, turn off pagination, and
use "rbreak ." or just "rbreak" with no argument, you can't interrupt
the rbreak command until it is done, which can take a very long time
depending upon the circumstances.  You can reproduce this somewhat by
using gdb to debug itself.

To test a possible fix for this, I made the following change:

--- symtab.c	27 Sep 2006 23:02:50 -0000	1.1.1.5.6.1
+++ symtab.c	13 Oct 2006 04:48:15 -0000
@@ -3306,6 +3306,7 @@ rbreak_command (char *regexp, int from_t
 
   for (p = ss; p != NULL; p = p->next)
     {
+      QUIT;	/* Allow user to ^C in case ss list is huge (I.E. "rbreak .") */
       if (p->msymbol == NULL)
 	{
 	  char *string = alloca (strlen (p->symtab->filename)

but to my surprise, this had no affect.  Further inveswtigation lead
me to the conclusion that the QUIT macro is no longer useful because
of changes in when the quit_flag is set.  Without getting to deep into
the details, when a user types ^C now, the function that catches the
signal arranges for a handler to be called, but that handler isn't
actually called until the event loop checks to see if there are any
handlers in the ready to run state.  And that doesn't happen during
the running of internal gdb commands like rbreak_command.

The patch attached below is pretty minimal and does fix this problem,
but it has the following issues:

(1) It exposes a previously internal entry point of the event handler
to other parts of gdb.

(2) It potential allows signal handlers unrelated to SIGINT to be
run at every point in the gdb code where QUIT is used.  Perhaps what
is needed is a way to restrict invoke_async_signal_handler to only
test for one particular handler being ready.

(3) If in fact QUIT is broken, and perhaps has been for a while,
it's possible that it's no longer safe in some places where it's
been used and reenabling it without more extensive testing could
cause other problems.

-Fred

+2006-10-12  Fred Fish  <fnf@specifix.com>
+
+	* defs.h (QUIT): Call invoke_async_signal_handler() if
+	quit_flag is set.
+	* event-loop.c (invoke_async_signal_handler): Remove static
+	qualifier so it can be called from other parts of gdb.
+	* event-top.c (handle_sigint): Set quit_flag so we can check
+	at convenient points if we want to abandon some long running
+	processing, via use of the "QUIT" macro.
+	(async_request_quit): Remove useless setting of quit_flag,
+	done too late to be helpful.
+	* symtab.c (rbreak_command): Call QUIT to test for ^C before
+	setting each breakpoint.

Index: defs.h
===================================================================
RCS file: /cvsroots/specifix/gnusense/gdb/gdb/defs.h,v
retrieving revision 1.1.1.5
diff -u -p -r1.1.1.5 defs.h
--- defs.h	22 Dec 2005 01:56:10 -0000	1.1.1.5
+++ defs.h	13 Oct 2006 04:45:03 -0000
@@ -174,8 +174,8 @@ extern void quit (void);
 #define QUIT_FIXME "ignoring redefinition of QUIT"
 #else
 #define QUIT { \
-  if (quit_flag) quit (); \
-  if (deprecated_interactive_hook) deprecated_interactive_hook (); \
+  extern void invoke_async_signal_handler (void); \
+  if (quit_flag) invoke_async_signal_handler (); \
 }
 #endif
 
Index: event-loop.c
===================================================================
RCS file: /cvsroots/specifix/gnusense/gdb/gdb/event-loop.c,v
retrieving revision 1.1.1.2
diff -u -p -r1.1.1.2 event-loop.c
--- event-loop.c	22 Dec 2005 01:56:31 -0000	1.1.1.2
+++ event-loop.c	13 Oct 2006 04:46:26 -0000
@@ -214,7 +214,7 @@ sighandler_list;
 static int async_handler_ready = 0;
 
 static void create_file_handler (int fd, int mask, handler_func * proc, gdb_client_data client_data);
-static void invoke_async_signal_handler (void);
+void invoke_async_signal_handler (void);
 static void handle_file_event (int event_file_desc);
 static int gdb_wait_for_event (void);
 static int check_async_ready (void);
@@ -982,7 +982,7 @@ mark_async_signal_handler (async_signal_
 }
 
 /* Call all the handlers that are ready. */
-static void
+void
 invoke_async_signal_handler (void)
 {
   async_signal_handler *async_handler_ptr;
Index: event-top.c
===================================================================
RCS file: /cvsroots/specifix/gnusense/gdb/gdb/event-top.c,v
retrieving revision 1.1.1.4.6.1
diff -u -p -r1.1.1.4.6.1 event-top.c
--- event-top.c	27 Sep 2006 23:02:49 -0000	1.1.1.4.6.1
+++ event-top.c	13 Oct 2006 04:47:23 -0000
@@ -969,6 +969,14 @@ handle_sigint (int sig)
 {
   signal (sig, handle_sigint);
 
+  /* This used to be in async_request_quit() but for the normal case
+     when immediate_quit isn't set, mark_async_signal_handler_wrapper
+     arranges to the signal handlers to eventually be checked, and
+     only at that point would async_request_quit get called, which is
+     too late for most things checking quit_flag with the QUIT
+     macro. */
+  quit_flag = 1;
+
   /* If immediate_quit is set, we go ahead and process the SIGINT right
      away, even if we usually would defer this to the event loop. The
      assumption here is that it is safe to process ^C immediately if
@@ -988,7 +996,6 @@ handle_sigint (int sig)
 void
 async_request_quit (gdb_client_data arg)
 {
-  quit_flag = 1;
   quit ();
 }
 


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