public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@palves.net>
To: Kevin Buettner <kevinb@redhat.com>, gdb-patches@sourceware.org
Cc: simark@simark.ca, tdevries@suse.de
Subject: Re: [PATCH v4 4/8] Python QUIT processing updates
Date: Mon, 30 Jan 2023 19:01:16 +0000	[thread overview]
Message-ID: <67d02aee-281d-5436-a969-90a3072867fe@palves.net> (raw)
In-Reply-To: <20230112015630.32999-5-kevinb@redhat.com>

On 2023-01-12 1:56 a.m., Kevin Buettner wrote:
> See the previous patches in this series for the motivation behind
> these changes.
> 
> This commit contains updates to Python's QUIT handling.  Ideally, we'd
> like to throw gdb_exception_forced_quit through the extension
> language; I made an attempt to do this for gdb_exception_quit in an
> earlier version of this patch, but Pedro pointed out that it is
> (almost certainly) not safe to do so.
> 
> Still, we definitely don't want to swallow the exception representing
> a SIGTERM for GDB, nor do we want to force modules written in the
> extension language to have to explicitly handle this case.  Since the
> idea is for GDB to cleanup and quit for this exception, we'll simply
> call quit_force() just as if the gdb_exception_forced_quit propagation
> had managed to make it back to the top level.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=26761

I guess this is OK as long as we don't hook up Python at a very low level
where running quit_force() would be dangerous.

Alternatively, couldn't we force some Python exception/error which is not
catch-able from Python code?  Set some error flag in the Python interpreter
or something, so that it aborts the Python code as soon as we return to the
Python interpreter.  We'd use the set_quit_flag() trick here where you're calling
quit_force (and instead of calling quit_force), so that as soon as we get out of
the Python interpreter on the other end, we'd re-raise gdb_exception_forced_quit.

Pedro Alves

> ---
>  gdb/python/py-finishbreakpoint.c | 5 +++++
>  gdb/python/py-gdb-readline.c     | 4 ++++
>  gdb/python/py-symbol.c           | 5 +++++
>  gdb/python/py-utils.c            | 3 +++
>  gdb/python/py-value.c            | 5 +++++
>  5 files changed, 22 insertions(+)
> 
> diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
> index d4d129110e9..1a224b35779 100644
> --- a/gdb/python/py-finishbreakpoint.c
> +++ b/gdb/python/py-finishbreakpoint.c
> @@ -20,6 +20,7 @@
>  
>  
>  #include "defs.h"
> +#include "top.h"		/* For quit_force().  */
>  #include "python-internal.h"
>  #include "breakpoint.h"
>  #include "frame.h"
> @@ -271,6 +272,10 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>  	    }
>  	}
>      }
> +  catch (const gdb_exception_forced_quit &except)
> +    {
> +      quit_force (NULL, 0);
> +    }
>    catch (const gdb_exception &except)
>      {
>        /* Just swallow.  Either the return type or the function value
> diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
> index ea0f78c9ad8..b9294ad9afc 100644
> --- a/gdb/python/py-gdb-readline.c
> +++ b/gdb/python/py-gdb-readline.c
> @@ -46,6 +46,10 @@ gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
>        p = command_line_input (buffer, prompt, "python");
>      }
>    /* Handle errors by raising Python exceptions.  */
> +  catch (const gdb_exception_forced_quit &e)
> +    {
> +      quit_force (NULL, 0);
> +    }
>    catch (const gdb_exception &except)
>      {
>        /* Detect user interrupt (Ctrl-C).  */
> diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
> index b8777966c47..899b0787582 100644
> --- a/gdb/python/py-symbol.c
> +++ b/gdb/python/py-symbol.c
> @@ -18,6 +18,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "defs.h"
> +#include "top.h"		/* For force_quit ().  */
>  #include "block.h"
>  #include "frame.h"
>  #include "symtab.h"
> @@ -515,6 +516,10 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
>  	= get_selected_frame (_("No frame selected."));
>        block = get_frame_block (selected_frame, NULL);
>      }
> +  catch (const gdb_exception_forced_quit &e)
> +    {
> +      quit_force (NULL, 0);
> +    }
>    catch (const gdb_exception &except)
>      {
>        /* Nothing.  */
> diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c
> index 624b90a827f..d5b07a80d82 100644
> --- a/gdb/python/py-utils.c
> +++ b/gdb/python/py-utils.c
> @@ -18,6 +18,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "defs.h"
> +#include "top.h"		/* For quit_force ().  */
>  #include "charset.h"
>  #include "value.h"
>  #include "python-internal.h"
> @@ -219,6 +220,8 @@ gdbpy_convert_exception (const struct gdb_exception &exception)
>  
>    if (exception.reason == RETURN_QUIT)
>      exc_class = PyExc_KeyboardInterrupt;
> +  else if (exception.reason == RETURN_FORCED_QUIT)
> +    quit_force (NULL, 0);
>    else if (exception.error == MEMORY_ERROR)
>      exc_class = gdbpy_gdb_memory_error;
>    else
> diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
> index dcc92e51b60..9473468035b 100644
> --- a/gdb/python/py-value.c
> +++ b/gdb/python/py-value.c
> @@ -18,6 +18,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "defs.h"
> +#include "top.h"		/* For quit_force ().  */
>  #include "charset.h"
>  #include "value.h"
>  #include "language.h"
> @@ -371,6 +372,10 @@ valpy_get_address (PyObject *self, void *closure)
>  	  res_val = value_addr (val_obj->value);
>  	  val_obj->address = value_to_value_object (res_val);
>  	}
> +      catch (const gdb_exception_forced_quit &except)
> +	{
> +	  quit_force (NULL, 0);
> +	}
>        catch (const gdb_exception &except)
>  	{
>  	  val_obj->address = Py_None;
> 


  reply	other threads:[~2023-01-30 19:01 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-12  1:56 [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Kevin Buettner
2023-01-12  1:56 ` [PATCH v4 1/8] Introduce gdb_exception_forced_quit Kevin Buettner
2023-01-30 18:56   ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 2/8] Handle gdb SIGTERM by throwing / catching gdb_exception_force_quit Kevin Buettner
2023-01-30 18:57   ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 3/8] Catch gdb_exception_error instead of gdb_exception (in many places) Kevin Buettner
2023-01-30 19:00   ` Pedro Alves
2023-02-16 10:52     ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 4/8] Python QUIT processing updates Kevin Buettner
2023-01-30 19:01   ` Pedro Alves [this message]
2023-02-20  2:52     ` Kevin Buettner
2023-02-23 12:50       ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 5/8] Guile " Kevin Buettner
2023-01-12  1:56 ` [PATCH v4 6/8] Call quit_force for gdb_exception_forced_quit in safe_execute_command Kevin Buettner
2023-01-30 19:01   ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 7/8] QUIT processing w/ explicit throw for gdb_exception_forced_quit Kevin Buettner
2023-01-30 19:02   ` Pedro Alves
2023-01-12  1:56 ` [PATCH v4 8/8] Forced quit cases handled by resetting sync_quit_force_run Kevin Buettner
2023-01-30 19:02   ` Pedro Alves
2023-01-12 12:37 ` [PATCH v4 0/8] Fix gdb.base/gdb-sigterm.exp failure/error Tom de Vries
2023-01-27 22:03   ` Kevin Buettner

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=67d02aee-281d-5436-a969-90a3072867fe@palves.net \
    --to=pedro@palves.net \
    --cc=gdb-patches@sourceware.org \
    --cc=kevinb@redhat.com \
    --cc=simark@simark.ca \
    --cc=tdevries@suse.de \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).