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]

Re: [PATCH 2/4]: Handle SIGINT under Python by raising KeyboardInterrupt


Hi,

This looks like it might work, I'll give it a go and get back to you. The only issue I see right now is if immediate_quit is set, it could leave Python in a bad state (among other issues). Perhaps make immediate_quit an argument of check_quit_flag?

Yit
July 30, 2012

On Jul 30, 2012, at 10:06 AM, Tom Tromey wrote:

>>>>>> "Yit" == Khoo Yit Phang <khooyp@cs.umd.edu> writes:
> 
> Yit> This turns out the be way more involved than it seems. In particular,
> Yit> convert_value_from_python (and it's callers) have nested TRY_CATCHs
> Yit> and interleaves calls to Python and calls to GDB. I haven't gone
> Yit> through every TRY_CATCH yet, but using these macros won't be that
> Yit> simple. I'm inclined to just wrap long-running calls for now.
> 
> How about this patch instead?
> 
> This one works by partially replacing gdb's mechanism with Python's,
> which seems to use the same general idea: set a flag and check it later.
> 
> This may result in Python doing a few extra calls to PyErr_CheckSignals,
> but I don't think that is a big deal.
> 
> I haven't run it through the test suite yet.  I will.
> 
> I'd appreciate it if you could give it a try.
> 
> Tom
> 
> commit 49f1b4e40a0cb0a856ce9a685204be41f63f6571
> Author: Tom Tromey <tromey@redhat.com>
> Date:   Mon Jul 30 07:54:37 2012 -0600
> 
>    use python signal-checking code if available
> 
>    	* defs.h (quit_flag): Conditionally declare.
>    	(clear_quit_flag, check_quit_flag, set_quit_flag): New
>    	macros.
>    	(QUIT): Use them.
>    	* event-top.c (command_handler): Use clear_quit_flag.
>    	(handle_sigint): Use set_quit_flag.
>    	(async_request_quit): Use check_quit_flag.
>    	* exceptions.c (throw_exception): Use clear_quit_flag.
>    	* main.c (captured_main): Use clear_quit_flag.
>    	* python/python.c (clear_quit_flag, set_quit_flag)
>    	(check_quit_flag): New functions.
>    	* remote-sim.c (gdb_os_poll_quit): Use check_quit_flag,
>    	clear_quit_flag.
>    	* remote.c (remote_wait_as): Use check_quit_flag,
>    	clear_quit_flag.
>    	* symfile.c (load_progress): Use check_quit_flag.
>    	* top.c (command_loop): Use clear_quit_flag.
>    	* utils.c (quit_flag): Conditionally define.
> 
> diff --git a/gdb/defs.h b/gdb/defs.h
> index 7be99a2..4ee46da 100644
> --- a/gdb/defs.h
> +++ b/gdb/defs.h
> @@ -171,7 +171,36 @@ extern char *python_libdir;
> /* Search path for separate debug files.  */
> extern char *debug_file_directory;
> 
> +/* GDB has two methods for handling SIGINT.  When immediate_quit is
> +   nonzero, a SIGINT results in an immediate longjmp out of the signal
> +   handler.  Otherwise, SIGINT simply sets a flag; code that might
> +   take a long time, and which ought to be interruptible, checks this
> +   flag using the QUIT macro.
> +   
> +   If GDB is built with Python support, it uses Python's low-level
> +   interface to implement the flag.  This approach makes it possible
> +   for Python and GDB SIGINT handling to coexist seamlessly.
> +
> +   If GDB is built without Python, it instead uses its traditional
> +   variables.
> +   
> +   The distinction between these two cases is hidden behind a few
> +   macros, defined here.  */
> +
> +#ifndef HAVE_PYTHON
> extern int quit_flag;
> +/* Clear the quit flag.  */
> +#define clear_quit_flag() do { quit_flag = 0; } while (0)
> +/* Evaluate to non-zero if the quit flag is set, zero otherwise.  */
> +#define check_quit_flag() (quit_flag + 0)
> +/* Set the quit flag.  */
> +#define set_quit_flag() do { quit_flag = 1; } while (0)
> +#else
> +extern void clear_quit_flag (void);
> +extern int check_quit_flag (void);
> +extern void set_quit_flag (void);
> +#endif /* HAVE_PYTHON */
> +
> extern int immediate_quit;
> 
> extern void quit (void);
> @@ -184,7 +213,7 @@ extern void quit (void);
>    needed.  */
> 
> #define QUIT { \
> -  if (quit_flag) quit (); \
> +  if (check_quit_flag ()) quit (); \
>   if (deprecated_interactive_hook) deprecated_interactive_hook (); \
> }
> 
> diff --git a/gdb/event-top.c b/gdb/event-top.c
> index 52e7852..204546d 100644
> --- a/gdb/event-top.c
> +++ b/gdb/event-top.c
> @@ -415,7 +415,7 @@ command_handler (char *command)
>   int stdin_is_tty = ISATTY (stdin);
>   struct cleanup *stat_chain;
> 
> -  quit_flag = 0;
> +  clear_quit_flag ();
>   if (instream == stdin && stdin_is_tty)
>     reinitialize_more_filter ();
> 
> @@ -799,7 +799,7 @@ handle_sigint (int sig)
>      set quit_flag to 1 here.  Then if QUIT is called before we get to
>      the event loop, we will unwind as expected.  */
> 
> -  quit_flag = 1;
> +  set_quit_flag ();
> 
>   /* 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
> @@ -831,7 +831,7 @@ async_request_quit (gdb_client_data arg)
>      is no reason to call quit again here, unless immediate_quit is
>      set.  */
> 
> -  if (quit_flag || immediate_quit)
> +  if (check_quit_flag () || immediate_quit)
>     quit ();
> }
> 
> diff --git a/gdb/exceptions.c b/gdb/exceptions.c
> index 7db9df9..b7cf9a2 100644
> --- a/gdb/exceptions.c
> +++ b/gdb/exceptions.c
> @@ -221,7 +221,7 @@ exceptions_state_mc_action_iter_1 (void)
> void
> throw_exception (struct gdb_exception exception)
> {
> -  quit_flag = 0;
> +  clear_quit_flag ();
>   immediate_quit = 0;
> 
>   do_cleanups (all_cleanups ());
> diff --git a/gdb/main.c b/gdb/main.c
> index d075694..326b101 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -331,7 +331,7 @@ captured_main (void *data)
>   dirarg = (char **) xmalloc (dirsize * sizeof (*dirarg));
>   ndir = 0;
> 
> -  quit_flag = 0;
> +  clear_quit_flag ();
>   saved_command_line = (char *) xmalloc (saved_command_line_size);
>   saved_command_line[0] = '\0';
>   instream = stdin;
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index c66efe4..4a2b518 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -151,6 +151,31 @@ ensure_python_env (struct gdbarch *gdbarch,
>   return make_cleanup (restore_python_env, env);
> }
> 
> +/* Clear the quit flag.  */
> +
> +void
> +clear_quit_flag (void)
> +{
> +  /* This clears the flag as a side effect.  */
> +  PyOS_InterruptOccurred ();
> +}
> +
> +/* Set the quit flag.  */
> +
> +void
> +set_quit_flag (void)
> +{
> +  PyErr_SetInterrupt ();
> +}
> +
> +/* Return true if the quit flag has been set, false otherwise.  */
> +
> +int
> +check_quit_flag (void)
> +{
> +  return PyOS_InterruptOccurred ();
> +}
> +
> /* A wrapper around PyRun_SimpleFile.  FILE is the Python script to run
>    named FILENAME.
> 
> diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
> index f5927f2..d0ba080 100644
> --- a/gdb/remote-sim.c
> +++ b/gdb/remote-sim.c
> @@ -950,9 +950,9 @@ gdb_os_poll_quit (host_callback *p)
>   if (deprecated_ui_loop_hook != NULL)
>     deprecated_ui_loop_hook (0);
> 
> -  if (quit_flag)		/* gdb's idea of quit */
> +  if (check_quit_flag ())	/* gdb's idea of quit */
>     {
> -      quit_flag = 0;		/* we've stolen it */
> +      clear_quit_flag ();	/* we've stolen it */
>       return 1;
>     }
>   else if (immediate_quit)
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 6780212..e12f630 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -5714,9 +5714,9 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
> 	  ofunc = signal (SIGINT, remote_interrupt);
> 	  /* If the user hit C-c before this packet, or between packets,
> 	     pretend that it was hit right here.  */
> -	  if (quit_flag)
> +	  if (check_quit_flag ())
> 	    {
> -	      quit_flag = 0;
> +	      clear_quit_flag ();
> 	      remote_interrupt (SIGINT);
> 	    }
> 	}
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 95ed480..264ed56 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -1986,7 +1986,7 @@ load_progress (ULONGEST bytes, void *untyped_arg)
>   args->buffer += bytes;
>   totals->write_count += 1;
>   args->section_sent += bytes;
> -  if (quit_flag
> +  if (check_quit_flag ()
>       || (deprecated_ui_load_progress_hook != NULL
> 	  && deprecated_ui_load_progress_hook (args->section_name,
> 					       args->section_sent)))
> diff --git a/gdb/top.c b/gdb/top.c
> index 061ad48..10baa32 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -567,7 +567,7 @@ command_loop (void)
>       if (window_hook && instream == stdin)
> 	(*window_hook) (instream, get_prompt ());
> 
> -      quit_flag = 0;
> +      clear_quit_flag ();
>       if (instream == stdin && stdin_is_tty)
> 	reinitialize_more_filter ();
>       old_chain = make_cleanup (null_cleanup, 0);
> diff --git a/gdb/utils.c b/gdb/utils.c
> index c69c3e0..036e39e 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -114,9 +114,11 @@ static int debug_timestamp = 0;
> 
> int job_control;
> 
> +#ifndef HAVE_PYTHON
> /* Nonzero means a quit has been requested.  */
> 
> int quit_flag;
> +#endif /* HAVE_PYTHON */
> 
> /* Nonzero means quit immediately if Control-C is typed now, rather
>    than waiting until QUIT is executed.  Be careful in setting this;


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