[PATCH] Implement nbsd_nat_target::thread_name

[PATCH] Implement nbsd_nat_target::thread_name

[Kamil Rytarowski]

Return the name assigned to a thread by an application.  Return
the string in a static buffer.

gdb/ChangeLog:

	* nbsd-nat.h (nbsd_nat_target::thread_name): Add.
        (struct thread_info): Add forward declaration.
        * nbsd-nat.c: Include "gdbthread.h".
        (nbsd_nat_target::thread_name): Add.
---
 gdb/ChangeLog  |  7 +++++++
 gdb/nbsd-nat.c | 39 +++++++++++++++++++++++++++++++++++++++
 gdb/nbsd-nat.h |  4 ++++
 3 files changed, 50 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5400a4e3484..4f6de3767d7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -53,6 +53,13 @@
 	* nat/gdb_ptrace.h: Don't declare ptrace if HAVE_DECL_PTRACE is
 	not defined.

+2020-03-20  Kamil Rytarowski  <n54@gmx.com>
+
+	* nbsd-nat.h (nbsd_nat_target::thread_name): Add.
+	(struct thread_info): Add forward declaration.
+	* nbsd-nat.c: Include "gdbthread.h".
+	(nbsd_nat_target::thread_name): Add.
+
 2020-03-20  Kamil Rytarowski  <n54@gmx.com>

 	* amd64-bsd-nat.c (gdb_ptrace): Change return type from `int' to
diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
index 326bbe3aec3..59c311e840c 100644
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -20,6 +20,7 @@
 #include "defs.h"

 #include "nbsd-nat.h"
+#include "gdbthread.h"

 #include <sys/types.h>
 #include <sys/ptrace.h>
@@ -39,3 +40,41 @@ nbsd_nat_target::pid_to_exec_file (int pid)
     return NULL;
   return buf;
 }
+
+/* Return the name assigned to a thread by an application.  Returns
+   the string in a static buffer.  */
+
+const char *
+nbsd_nat_target::thread_name (struct thread_info *thr)
+{
+  ptid_t ptid = thr->ptid;
+  pid_t pid = ptid.pid ();
+  int lwp = ptid.lwp ();
+  int mib[5] = {CTL_KERN, KERN_LWP, pid, sizeof (struct kinfo_lwp), 0};
+  size_t size;
+
+  if (sysctl (mib, 5, NULL, &size, NULL, 0) == -1 || size == 0)
+    perror_with_name (("sysctl"));
+
+  mib[4] = size / sizeof (size_t);
+
+  gdb::unique_xmalloc_ptr<struct kinfo_lwp> kl
+    ((struct kinfo_lwp *) xcalloc (size, 1));
+  if (kl == NULL)
+    perror_with_name (("calloc"));
+
+  if (sysctl (mib, 5, kl.get (), &size, NULL, 0) == -1 || size == 0)
+    perror_with_name (("sysctl"));
+
+  static char buf[KI_LNAMELEN] = {};
+  for (size_t i = 0; i < size / sizeof (struct kinfo_lwp); i++)
+    {
+      if (kl.get ()[i].l_lid == lwp)
+	{
+	  xsnprintf (buf, sizeof buf, "%s", kl.get ()[i].l_name);
+	  break;
+	}
+    }
+
+  return buf;
+}
diff --git a/gdb/nbsd-nat.h b/gdb/nbsd-nat.h
index a752fbe572d..3909bdf67a2 100644
--- a/gdb/nbsd-nat.h
+++ b/gdb/nbsd-nat.h
@@ -22,11 +22,15 @@

 #include "inf-ptrace.h"

+struct thread_info;
+
 /* A prototype NetBSD target.  */

 struct nbsd_nat_target : public inf_ptrace_target
 {
   char *pid_to_exec_file (int pid) override;
+
+  const char *thread_name (struct thread_info *thr) override;
 };

 #endif /* nbsd-nat.h */
--
2.25.0

Re: [PATCH] Implement nbsd_nat_target::thread_name

[Tom Tromey]

>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:

Kamil> Return the name assigned to a thread by an application.  Return
Kamil> the string in a static buffer.

Thanks for the patch.

Kamil> +  if (sysctl (mib, 5, NULL, &size, NULL, 0) == -1 || size == 0)
Kamil> +    perror_with_name (("sysctl"));

I suspect it's better to return NULL there than to throw an exception.

Kamil> +  if (kl == NULL)
Kamil> +    perror_with_name (("calloc"));
Kamil> +
Kamil> +  if (sysctl (mib, 5, kl.get (), &size, NULL, 0) == -1 || size == 0)
Kamil> +    perror_with_name (("sysctl"));

Likewise in these cases.

thanks,
Tom

Re: [PATCH] Implement nbsd_nat_target::thread_name

[Kamil Rytarowski]

On 01.04.2020 21:28, Tom Tromey wrote:
>>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:
>
> Kamil> Return the name assigned to a thread by an application.  Return
> Kamil> the string in a static buffer.
>
> Thanks for the patch.
>
> Kamil> +  if (sysctl (mib, 5, NULL, &size, NULL, 0) == -1 || size == 0)
> Kamil> +    perror_with_name (("sysctl"));
>
> I suspect it's better to return NULL there than to throw an exception.
>
> Kamil> +  if (kl == NULL)
> Kamil> +    perror_with_name (("calloc"));
> Kamil> +
> Kamil> +  if (sysctl (mib, 5, kl.get (), &size, NULL, 0) == -1 || size == 0)
> Kamil> +    perror_with_name (("sysctl"));
>
> Likewise in these cases.
>
> thanks,
> Tom
>

Please check a newer patch:

[PATCH v2] Implement basic threading support in the NetBSD target

https://sourceware.org/pipermail/gdb-patches/2020-March/167018.html

Does this comment still apply to return NULL for thread_name()
thread_alive() and update_thread_list()?

In practice the kernel will almost never return error when used
properly, unless there is specified a wrong process id or similar. It's
generally safe to abort on error here.

Re: [PATCH] Implement nbsd_nat_target::thread_name

[Tom Tromey]

>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:

Kamil> Please check a newer patch:

Kamil> [PATCH v2] Implement basic threading support in the NetBSD target

Thanks, I sent a follow-up.

Kamil> Does this comment still apply to return NULL for thread_name()
Kamil> thread_alive() and update_thread_list()?

Kamil> In practice the kernel will almost never return error when used
Kamil> properly, unless there is specified a wrong process id or similar. It's
Kamil> generally safe to abort on error here.

It's fine by me then.

Tom