Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] Fix "PC register is not available" issue
Date: Tue, 18 Mar 2014 16:16:00 -0000	[thread overview]
Message-ID: <20140318161608.GD4282@adacore.com> (raw)
In-Reply-To: <83txawa9wk.fsf@gnu.org>

Hi Eli,

On Mon, Mar 17, 2014 at 09:43:23PM +0200, Eli Zaretskii wrote:
> This problem was raised and mentioned several times over the last few
> years.  It was discussed here in the thread which started with this
> message:
> 
>   https://sourceware.org/ml/gdb-patches/2013-06/msg00237.html

Thanks for looking into this!

> So I came up with the patch below.  The idea is that setting
> th->suspended to -1 just signals to GDB that the thread isn't
> suspended, and shouldn't be resumed; otherwise, we simply ignore the
> failure to suspend the thread, although the warning is still printed.
> 
> I am running with this patch for almost a month, and the dreaded "PC
> register is not available" message didn't appear even once.  No more
> botched debugging sessions!  The test program in PR/14018 also runs
> indefinitely until interrupted, instead of stopping.
> 
> So I suggest to install this.  OK?  Should I also close the Bugzilla
> PR?
> 
> --- gdb/windows-nat.c~0	2014-02-06 04:21:29.000000000 +0200
> +++ gdb/windows-nat.c	2014-02-26 22:27:10.225625000 +0200
> @@ -313,9 +313,10 @@ thread_rec (DWORD id, int get_context)
>  		    warning (_("SuspendThread (tid=0x%x) failed."
>  			       " (winerr %u)"),
>  			     (unsigned) id, (unsigned) err);
> -		    return NULL;
> +		    th->suspended = -1;
>  		  }
> -		th->suspended = 1;
> +		else
> +		  th->suspended = 1;
>  	      }
>  	    else if (get_context < 0)
>  	      th->suspended = -1;

Generally speaking, it seems to make sense to me that we would
mark as unsuspended threads that we cannot suspend. But one question
that rises from doing that is: how does the rest of GDB handle
this thread? In particular, does the thread show up in "info thread"
and is the user able to switch to that thread? etc? Certainly, if
the current situation is to leave the user stranded, the suggested
approach cannot only be an improvement...

Another thought I had on your patch is that we might want to limit
the warning to situation where the return code is not a permission
denied.

It would also have been nice to double-check that the thread in
question, when the error shows up, is indeed on a system thread
unrelated to our program (Eg: the thread created when we try to
interrupt the program with ctrl-c or ctrl-break). Not sure if
there is a way to determine that, though.

I would have looked into that, but I don't have much time this week, and
might be equally busy the following 2 weeks, so I didn't want to delay
my answer any further. Overall, your patch looks very promising.

Sorry I can't be anymore support for the moment.
-- 
Joel


  reply	other threads:[~2014-03-18 16:16 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-17 19:43 Eli Zaretskii
2014-03-18 16:16 ` Joel Brobecker [this message]
2014-03-18 16:35   ` Eli Zaretskii
2014-03-18 16:54     ` Joel Brobecker
2014-03-18 17:13       ` Eli Zaretskii
2014-03-18 17:33         ` Pedro Alves
2014-03-19  3:41           ` Eli Zaretskii
2014-03-19 10:07             ` Pedro Alves
2014-03-19 16:24               ` Eli Zaretskii
2014-03-19 16:41                 ` Pedro Alves
2014-03-26 18:49       ` Eli Zaretskii
2014-03-27 12:56         ` Joel Brobecker
2014-03-27 17:41           ` Eli Zaretskii
2014-03-28 13:00             ` Joel Brobecker
2014-03-28 17:29               ` Eli Zaretskii
2014-03-28 14:50         ` Pedro Alves
2014-03-28 17:35           ` Eli Zaretskii
2014-03-28 17:49             ` Pedro Alves
2014-03-28 18:30               ` Eli Zaretskii
2014-03-31 15:31                 ` Eli Zaretskii
2014-04-05  9:06                   ` Eli Zaretskii
2014-04-07 16:58                     ` Joel Brobecker
2014-04-07 17:09                   ` Pedro Alves
2014-04-07 18:25                     ` Eli Zaretskii
2014-04-07 21:39                       ` Joel Brobecker
2014-04-08  2:44                         ` Eli Zaretskii
2014-04-08  4:23                           ` Joel Brobecker
2014-04-08 15:17                             ` Eli Zaretskii
2014-04-08 11:32                       ` Pedro Alves
2014-04-08 16:43                         ` Pedro Alves
2014-04-08 17:10                           ` Eli Zaretskii
2014-04-08 17:36                             ` Pedro Alves
2014-04-08 17:54                               ` Eli Zaretskii
2014-04-11 20:06                                 ` Joel Brobecker
2014-04-19  8:33                                   ` Eli Zaretskii
2014-04-21 15:43                                     ` Joel Brobecker
2014-04-21 15:59                                       ` Eli Zaretskii

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=20140318161608.GD4282@adacore.com \
    --to=brobecker@adacore.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    /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