Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@codesourcery.com>
To: gdb-patches@sourceware.org
Cc: Vladimir Prus <vladimir@codesourcery.com>
Subject: Re: [MI non-stop 03/11, RFA] Discard cleanup when deferring displaced step
Date: Fri, 11 Jul 2008 13:11:00 -0000	[thread overview]
Message-ID: <200807111410.58951.pedro@codesourcery.com> (raw)
In-Reply-To: <200806282039.52280.vladimir@codesourcery.com>

A Saturday 28 June 2008 17:39:52, Vladimir Prus wrote:
> My MI non-stop tests (coming later in the series) revealed a bug in
> displaced stepping -- we fail to discard cleanup on an early exit path,
> which caused a call to normal_stop at very unfortunate time -- when a
> thread was running.
>

Ooops, my bad.  Thanks for catching this!

I think it would make sense to tag the thread as running at that point.
In the point of view of the frontend, it is running, but internally,
it is known to not be executing, until its turn in the displaced stepping
request queue arrives.  That is, the frontend should not allow inspecting
this thread as if it was running already, and interrupting it should make
it stop as soon as it tries to execute, not skip over it like ends up
happening currently.

To be clear, the user told the thread to resume, it just didn't resume
immediatelly due to an internal implementation detail.  (is_running (ptid)
&& !is_executing (ptid)) should be true at that early return.  Not a
serious issue to run to fix it (may need care if starting the
displaced step on it ends up failing), but,

Anyone disagrees in principle?

> OK?
>
> - Volodya
>
> 	* infrun.c (resume): Discard cleanups on early exit path.
> ---
>  gdb/infrun.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index cf23c18..e803d1b 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -964,10 +964,13 @@ a command like `return' or `jump' to continue
> execution.")); && sig == TARGET_SIGNAL_0)
>      {
>        if (!displaced_step_prepare (inferior_ptid))
> -	/* Got placed in displaced stepping queue.  Will be resumed
> -	   later when all the currently queued displaced stepping
> -	   requests finish.  */
> -	return;
> +	{
> +	  /* Got placed in displaced stepping queue.  Will be resumed
> +	     later when all the currently queued displaced stepping
> +	     requests finish.  */
> +	  discard_cleanups (old_cleanups);
> +	  return;
> +	}
>      }
>
>    if (step && gdbarch_software_single_step_p (gdbarch))



-- 
Pedro Alves


  parent reply	other threads:[~2008-07-11 13:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-28 16:45 Vladimir Prus
2008-07-11 12:34 ` Daniel Jacobowitz
2008-07-11 13:11 ` Pedro Alves [this message]
2008-07-11 13:21   ` Vladimir Prus
2008-07-11 13:35     ` Daniel Jacobowitz
2008-07-13  5:55       ` Vladimir Prus
2008-07-11 13:35     ` Pedro Alves

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=200807111410.58951.pedro@codesourcery.com \
    --to=pedro@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    --cc=vladimir@codesourcery.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