Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Fred Fish <fnf@specifix.com>
To: GDB Patches <gdb-patches@sources.redhat.com>
Subject: [RFC] Patch for QUIT macro support
Date: Fri, 13 Oct 2006 12:43:00 -0000	[thread overview]
Message-ID: <200610130543.25806.fnf@specifix.com> (raw)

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 ();
 }
 


             reply	other threads:[~2006-10-13 12:43 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-13 12:43 Fred Fish [this message]
2006-10-13 13:22 ` Fred Fish
2006-10-13 13:30   ` Daniel Jacobowitz
2006-10-13 13:49     ` Fred Fish
2007-01-21 17:44       ` Daniel Jacobowitz
2007-01-21 19:23         ` Fred Fish
     [not found]       ` <200610151800.k9FI0fDS016688@elgar.sibelius.xs4all.nl>
2007-01-22 14:41         ` Fred Fish
2007-02-08 14:34           ` Daniel Jacobowitz
2007-02-08 20:49             ` Nick Roberts
2007-02-08 21:17               ` Fred Fish
2007-02-08 21:51                 ` Nick Roberts
2006-10-13 14:58     ` Fred Fish
2007-02-08 14:34       ` Daniel Jacobowitz
2006-10-16  0:08 Nick Roberts

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200610130543.25806.fnf@specifix.com \
    --to=fnf@specifix.com \
    --cc=gdb-patches@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox