[PATCH] [PR gdb/23077] Fix error 'Couldn't get registers: Device busy' on FreeBSD

[PATCH] [PR gdb/23077] Fix error 'Couldn't get registers: Device busy' on FreeBSD

[Rajendra SY]

[-- Attachment #1: Type: text/plain, Size: 2683 bytes --]

<patch attached>

Problem:
GDB  "attach pid" command shows error message "Couldn't get registers:
Device busy" because of which below gdb tests fail.

Cause:
On FreeBSD ptrace() syscall requires LWP id to fetch info from the
inferior process.
The current implementation is every command is executed completely and
then it wait's for asynchronous events.

When attach command is executed it calls inf_ptrace_attach() function
which is currently doing just the ptrace(PT_ATTACH), it does not wait
for the process to stop to fetch LWP info.
The fbsd_wait() function is the one which fetches LWP info which is
being called after the command completes.

Command execution code flow -
gdb_do_one_event() => .. => handle_file_event() => .. =>
execute_command() => check_frame_language_change()

The async event handling code flow where LWP info gets updated -
gdb_one_event() => check_async_event_handlers() => ..
->do_target_wait() => .. => fbsd_wait()

The check, if the frame language changed via
check_frame_language_change() function is trying to read registers
using ptrace(pid) which fails because LWP id is not fetched.

The purposed patch tries to avoid this situation by avoiding calling
check_frame_language_change() function just for the "attach" command.

Tests:
FAIL: gdb.base/attach.exp: attach1, after setting file
FAIL: gdb.base/attach.exp: attach2, with no file
FAIL: gdb.base/attach.exp: load file manually, after attach2 (re-read)
(got interactive prompt)
FAIL: gdb.base/attach.exp: attach when process' a.out not in cwd

FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=off: re-attach
to inferior

These above currently failing tests will pass with this fix.

gdb/ChangeLog:
2018-04-20  Rajendra SY  <rajendra.sy@gmail.com>

        PR gdb/23077
        * gdb/top.c (execute_command):

[-- Attachment #2: gdb-skip-check_frame_language_change.diff --]
[-- Type: application/octet-stream, Size: 900 bytes --]

diff --git a/gdb/top.c b/gdb/top.c
index 8903a92983..15f99a6a0e 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -543,6 +543,7 @@ execute_command (const char *p, int from_tty)
   struct cmd_list_element *c;
   const char *line;
   const char *cmd_start = p;
+  int is_attach_cmd = 0;
 
   cleanup_if_error = make_bpstat_clear_actions_cleanup ();
   scoped_value_mark cleanup = prepare_execute_command ();
@@ -557,7 +558,7 @@ execute_command (const char *p, int from_tty)
       discard_cleanups (cleanup_if_error);
       return;
     }
-
+  if (strncmp (p, "attach", 6) == 0) is_attach_cmd = 1;
   target_log_command (p);
 
   while (*p == ' ' || *p == '\t')
@@ -641,8 +642,9 @@ execute_command (const char *p, int from_tty)
 		  repeat_arguments);
 	}
     }
-
-  check_frame_language_change ();
+  
+  if (!is_attach_cmd)
+    check_frame_language_change ();
 
   discard_cleanups (cleanup_if_error);
 }

Re: [PATCH] [PR gdb/23077] Fix error 'Couldn't get registers: Device busy' on FreeBSD

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]

On 04/20/2018 10:46 AM, Rajendra SY wrote:
> <patch attached>
> 
> Problem:
> GDB  "attach pid" command shows error message "Couldn't get registers:
> Device busy" because of which below gdb tests fail.
> 
> Cause:
> On FreeBSD ptrace() syscall requires LWP id to fetch info from the
> inferior process.
> The current implementation is every command is executed completely and
> then it wait's for asynchronous events.
> 
> When attach command is executed it calls inf_ptrace_attach() function
> which is currently doing just the ptrace(PT_ATTACH), it does not wait
> for the process to stop to fetch LWP info.
> The fbsd_wait() function is the one which fetches LWP info which is
> being called after the command completes.
> 
> Command execution code flow -
> gdb_do_one_event() => .. => handle_file_event() => .. =>
> execute_command() => check_frame_language_change()
> 
> The async event handling code flow where LWP info gets updated -
> gdb_one_event() => check_async_event_handlers() => ..
> ->do_target_wait() => .. => fbsd_wait()
> 
> The check, if the frame language changed via
> check_frame_language_change() function is trying to read registers
> using ptrace(pid) which fails because LWP id is not fetched.
> 
> The purposed patch tries to avoid this situation by avoiding calling
> check_frame_language_change() function just for the "attach" command.
> 
Thanks for the patch, but we need to find a better fix.

It seems to me that the problem is that we fail to mark
the thread as executing between the initial attach, and the
subsequent target_wait.  Can you give the attached patch a try?
check_frame_language_change() then becomes a nop until
the thread is marked not-executing again, after target_wait
is called.

We don't see the problem on Linux because there, the target_attach
implementation waits for the thread to stop before returning.
Still, that's supposedly hidden from the core, since the Linux
target, like most targets, is a '!to_attach_no_wait' target.

Pedro Alves

[-- Attachment #2: 0001-Mark-just-attached-to-thread-as-executing.patch --]
[-- Type: text/x-patch, Size: 1394 bytes --]

From d1a0ee089d0d59af19fd65267e0e1d2f5f7821c0 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Fri, 20 Apr 2018 12:22:05 +0100
Subject: [PATCH] Mark just-attached-to thread as executing

---
 gdb/inf-ptrace.c | 3 ++-
 gdb/remote.c     | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index e20388658fe..68d557138a0 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -235,7 +235,8 @@ inf_ptrace_attach (struct target_ops *ops, const char *args, int from_tty)
 
   /* Always add a main thread.  If some target extends the ptrace
      target, it should decorate the ptid later with more info.  */
-  add_thread_silent (inferior_ptid);
+  thread_info *tp = add_thread_silent (inferior_ptid);
+  set_executing (tp->ptid, true);
 
   unpusher.release ();
 }
diff --git a/gdb/remote.c b/gdb/remote.c
index f54a38833b3..dc1c62f214c 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5316,7 +5316,8 @@ extended_remote_attach (struct target_ops *target, const char *args,
       inferior_ptid = remote_current_thread (inferior_ptid);
 
       /* Add the main thread to the thread list.  */
-      add_thread_silent (inferior_ptid);
+      thread_info *tp = add_thread_silent (inferior_ptid);
+      set_executing (tp->ptid, true);
     }
 
   /* Next, if the target can specify a description, read it.  We do
-- 
2.14.3

Re: [PATCH] [PR gdb/23077] Fix error 'Couldn't get registers: Device busy' on FreeBSD

[Rajendra SY]

Pedro,

I tested your patch it works on FreeBSD.
No error message after attach.


Thanks
Rajendra

On Fri, Apr 20, 2018 at 7:01 PM, Pedro Alves <palves@redhat.com> wrote:
> On 04/20/2018 10:46 AM, Rajendra SY wrote:
>> <patch attached>
>>
>> Problem:
>> GDB  "attach pid" command shows error message "Couldn't get registers:
>> Device busy" because of which below gdb tests fail.
>>
>> Cause:
>> On FreeBSD ptrace() syscall requires LWP id to fetch info from the
>> inferior process.
>> The current implementation is every command is executed completely and
>> then it wait's for asynchronous events.
>>
>> When attach command is executed it calls inf_ptrace_attach() function
>> which is currently doing just the ptrace(PT_ATTACH), it does not wait
>> for the process to stop to fetch LWP info.
>> The fbsd_wait() function is the one which fetches LWP info which is
>> being called after the command completes.
>>
>> Command execution code flow -
>> gdb_do_one_event() => .. => handle_file_event() => .. =>
>> execute_command() => check_frame_language_change()
>>
>> The async event handling code flow where LWP info gets updated -
>> gdb_one_event() => check_async_event_handlers() => ..
>> ->do_target_wait() => .. => fbsd_wait()
>>
>> The check, if the frame language changed via
>> check_frame_language_change() function is trying to read registers
>> using ptrace(pid) which fails because LWP id is not fetched.
>>
>> The purposed patch tries to avoid this situation by avoiding calling
>> check_frame_language_change() function just for the "attach" command.
>>
> Thanks for the patch, but we need to find a better fix.
>
> It seems to me that the problem is that we fail to mark
> the thread as executing between the initial attach, and the
> subsequent target_wait.  Can you give the attached patch a try?
> check_frame_language_change() then becomes a nop until
> the thread is marked not-executing again, after target_wait
> is called.
>
> We don't see the problem on Linux because there, the target_attach
> implementation waits for the thread to stop before returning.
> Still, that's supposedly hidden from the core, since the Linux
> target, like most targets, is a '!to_attach_no_wait' target.
>
> Pedro Alves

[pushed v2] FreeBSD: Fix 'Couldn't get registers: Device busy' error (PR gdb/23077)

[Pedro Alves]

On 04/20/2018 04:19 PM, Rajendra SY wrote:
> I tested your patch it works on FreeBSD.
> No error message after attach.

Great, I've pushed it in now, as below.

Thanks,
Pedro Alves

From 00aecdcf6224e44fedf31a07340e9da11500fcfb Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Sat, 21 Apr 2018 18:19:30 +0100
Subject: [PATCH] FreeBSD: Fix 'Couldn't get registers: Device busy' error (PR
 gdb/23077)

As Rajendra SY reported at
<https://sourceware.org/ml/gdb-patches/2018-04/msg00399.html>, several
attach-related tests are failing on FreeBSD.  The "attach" command
errors with "Couldn't get registers: Device busy".

When the "attach" command is executed, it calls target_attach ->
inf_ptrace_attach, which just does the ptrace(PT_ATTACH), it does not
wait for the child to stop with SIGSTOP.  Afterwards, the command is
complete and we go back to the event loop.  The event loop wakes up
and we end up in target_wait -> fbsd_wait, and handle the SIGSTOP
stop.

At the end of execute_command, though, before going back to the event
loop, we check if the frame language changed via
check_frame_language_change().  That reads the current PC, which is
what leads to the registers read that fails.

The problem is that we fail to mark the attached-to thread as
executing between the initial attach, and the subsequent target_wait.
Until we see the thread stop with SIGSTOP, we shouldn't try to read
registers off of it.  I guess there may a timing issue here - if
you're "lucky", the thread may stop before gdb reads its registers,
masking the problem.

With that fixed, check_frame_language_change() becomes a nop until the
thread is marked not-executing again, after target_wait is called and
we go through handle_inferior_event -> normal_stop.

We haven't seen the problem on Linux because there, the target_attach
implementation waits for the thread to stop before returning.  Still,
that's supposedly hidden from the core, since the Linux target, like
most targets, is a '!to_attach_no_wait' target.

This fixes:
 FAIL: gdb.base/attach.exp: attach1, after setting file
 FAIL: gdb.base/attach.exp: attach2, with no file
 FAIL: gdb.base/attach.exp: load file manually, after attach2 (re-read) (got interactive prompt)
 FAIL: gdb.base/attach.exp: attach when process' a.out not in cwd

 FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=on: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=off: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=on: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=off: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=on: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=off: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=on: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=off: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=on: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=off: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=on: re-attach to inferior
 FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=off: re-attach to inferior

gdb/ChangeLog:
2018-04-21  Pedro Alves  <palves@redhat.com>
	    Rajendra SY  <rajendra.sy@gmail.com>

	* inf-ptrace.c (inf_ptrace_attach): Mark the thread as executing.
	* remote.c (extended_remote_attach): In all-stop mode, mark the
	thread as executing.
---
 gdb/ChangeLog    | 7 +++++++
 gdb/inf-ptrace.c | 5 ++++-
 gdb/remote.c     | 5 ++++-
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0db5c46f6cd..70c461a4b3a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2018-04-21  Pedro Alves  <palves@redhat.com>
+	    Rajendra SY  <rajendra.sy@gmail.com>
+
+	* inf-ptrace.c (inf_ptrace_attach): Mark the thread as executing.
+	* remote.c (extended_remote_attach): In all-stop mode, mark the
+	thread as executing.
+
 2018-04-19  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* thread.c (thread_apply_all_command): Fix comment.
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index e20388658fe..942494bbdac 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -235,7 +235,10 @@ inf_ptrace_attach (struct target_ops *ops, const char *args, int from_tty)
 
   /* Always add a main thread.  If some target extends the ptrace
      target, it should decorate the ptid later with more info.  */
-  add_thread_silent (inferior_ptid);
+  thread_info *thr = add_thread_silent (inferior_ptid);
+  /* Don't consider the thread stopped until we've processed its
+     initial SIGSTOP stop.  */
+  set_executing (thr->ptid, true);
 
   unpusher.release ();
 }
diff --git a/gdb/remote.c b/gdb/remote.c
index f54a38833b3..49013848d55 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5316,7 +5316,10 @@ extended_remote_attach (struct target_ops *target, const char *args,
       inferior_ptid = remote_current_thread (inferior_ptid);
 
       /* Add the main thread to the thread list.  */
-      add_thread_silent (inferior_ptid);
+      thread_info *thr = add_thread_silent (inferior_ptid);
+      /* Don't consider the thread stopped until we've processed the
+	 saved stop reply.  */
+      set_executing (thr->ptid, true);
     }
 
   /* Next, if the target can specify a description, read it.  We do
-- 
2.14.3

Re: [PATCH] [PR gdb/23077] Fix error 'Couldn't get registers: Device busy' on FreeBSD

[Pedro Alves]

Hi!

FYI, your emails are missing the actual patches:

 https://sourceware.org/ml/gdb-patches/2018-04/msg00396.html
 https://sourceware.org/ml/gdb-patches/2018-04/msg00397.html

Thanks,
Pedro Alves

[PATCH] [PR gdb/23077] Fix error 'Couldn't get registers: Device busy' on FreeBSD

[Rajendra SY]

Problem:
GDB  "attach pid" command shows error message "Couldn't get registers:
Device busy" because of which below gdb tests fail.

Cause:
On FreeBSD ptrace() syscall requires LWP id to fetch info from the
inferior process.
The current implementation is every command is executed completely and
then it wait's for asynchronous events.

When attach command is executed it calls inf_ptrace_attach() function
which is currently doing just the ptrace(PT_ATTACH), it does not wait
for the process to stop to fetch LWP info.
The fbsd_wait() function is the one which fetches LWP info which is
being called after the command completes.

Command execution code flow -
gdb_do_one_event() => .. => handle_file_event() => .. =>
execute_command() => check_frame_language_change()

The async event handling code flow where LWP info gets updated -
gdb_one_event() => check_async_event_handlers() => ..
->do_target_wait() => .. => fbsd_wait()

The check, if the frame language changed via
check_frame_language_change() function is trying to read registers
using ptrace(pid) which fails because LWP id is not fetched.

The purposed patch tries to avoid this situation by avoiding calling
check_frame_language_change() function just for the "attach" command.

Tests:
FAIL: gdb.base/attach.exp: attach1, after setting file
FAIL: gdb.base/attach.exp: attach2, with no file
FAIL: gdb.base/attach.exp: load file manually, after attach2 (re-read)
(got interactive prompt)
FAIL: gdb.base/attach.exp: attach when process' a.out not in cwd

FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=gdb dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=call dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=on ds=agent dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=gdb dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=call dd=off: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=on: re-attach to inferior
FAIL: gdb.base/dprintf-detach.exp: bai=off ds=agent dd=off: re-attach
to inferior

These above currently failing tests will pass with this fix.

gdb/ChangeLog:
2018-04-20  Rajendra SY  <rajendra.sy@gmail.com>

        PR gdb/23077
        * gdb/top.c (execute_command):