[patch] fix tkill_failed gcc warning in gdbserver

[patch] fix tkill_failed gcc warning in gdbserver

[Doug Evans]

Hi.

android doesn't have SYS_tkill but it does have __NR_tkill.

I will check this in tomorrow, pending objections.

This includes some minor tweaking to linux-nat.c:kill_lwp.
The code changes are technically better, and I like the consistency.

2009-12-21  Doug Evans  <dje@google.com>

	gdb/
	* linux-nat.c (kill_lwp): Minor cleanup, move definition of
	tkill_failed into ifdef HAVE_TKILL_SYSCALL.  Move setting of errno
	there too.  Delete unnecessary resetting of errno after syscall.
	Minor comment changes to match gdbserver/linux-low.c:kill_lwp.

	gdbserver/
	* configure.ac: Look for both __NR_tkill and SYS_tkill.
	Define TKILL_SYSCALL_NAME to the one we find, if any.
	* config.in: Regenerate.
	* configure: Regenerate.
	* linux-low.c (kill_lwp): Move definition of tkill_failed to
	ifdef TKILL_SYSCALL_NAME to avoid gcc warning.  Move setting of errno
	there too.  Delete unnecessary resetting of errno after syscall.
	Minor comment changes to match gdb/linux-nat.c:kill_lwp.

Index: linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.156
diff -u -p -r1.156 linux-nat.c
--- linux-nat.c	20 Nov 2009 19:48:45 -0000	1.156
+++ linux-nat.c	21 Dec 2009 19:43:51 -0000
@@ -2034,27 +2034,29 @@ linux_nat_resume (struct target_ops *ops
     target_async (inferior_event_handler, 0);
 }
 
-/* Issue kill to specified lwp.  */
-
-static int tkill_failed;
+/* Send a signal to an LWP.  */
 
 static int
 kill_lwp (int lwpid, int signo)
 {
-  errno = 0;
-
-/* Use tkill, if possible, in case we are using nptl threads.  If tkill
-   fails, then we are not using nptl threads and we should be using kill.  */
+  /* Use tkill, if possible, in case we are using nptl threads.  If tkill
+     fails, then we are not using nptl threads and we should be using kill.  */
 
 #ifdef HAVE_TKILL_SYSCALL
-  if (!tkill_failed)
-    {
-      int ret = syscall (__NR_tkill, lwpid, signo);
-      if (errno != ENOSYS)
-	return ret;
-      errno = 0;
-      tkill_failed = 1;
-    }
+  {
+    static int tkill_failed;
+
+    if (!tkill_failed)
+      {
+	int ret;
+
+	errno = 0;
+	ret = syscall (__NR_tkill, lwpid, signo);
+	if (errno != ENOSYS)
+	  return ret;
+	tkill_failed = 1;
+      }
+  }
 #endif
 
   return kill (lwpid, signo);
Index: gdbserver/configure.ac
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/configure.ac,v
retrieving revision 1.30
diff -u -p -r1.30 configure.ac
--- gdbserver/configure.ac	17 Nov 2009 17:58:15 -0000	1.30
+++ gdbserver/configure.ac	21 Dec 2009 19:43:51 -0000
@@ -1,5 +1,5 @@
 dnl Autoconf configure script for GDB server.
-dnl Copyright (C) 2000, 2002, 2003, 2004, 2005, 2006, 2007, 2008
+dnl Copyright (C) 2000, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009
 dnl Free Software Foundation, Inc.
 dnl
 dnl This file is part of GDB.
@@ -196,6 +205,34 @@ AS_HELP_STRING([--with-libthread-db=PATH
   srv_libs="$srv_libthread_db_path"
 ])
 
+dnl See if sys/syscall.h has __NR_tkill or SYS_tkill.
+AC_CACHE_CHECK([whether <sys/syscall.h> has __NR_tkill],
+               gdb_cv_sys_syscall_h_has_NR_tkill,
+  AC_TRY_COMPILE(
+    [#include <sys/syscall.h>],
+    [int i = __NR_tkill;],
+    gdb_cv_sys_syscall_h_has_NR_tkill=yes,
+    gdb_cv_sys_syscall_h_has_NR_tkill=no
+  )
+)
+if test "x$gdb_cv_sys_syscall_h_has_NR_tkill" != "xyes"; then
+  AC_CACHE_CHECK([whether <sys/syscall.h> has __SYS_tkill],
+                  gdb_cv_sys_syscall_h_has_SYS_tkill,
+    AC_TRY_COMPILE(
+      [#include <sys/syscall.h>],
+      [int i = SYS_tkill;],
+      gdb_cv_sys_syscall_h_has_SYS_tkill=yes,
+      gdb_cv_sys_syscall_h_has_SYS_tkill=no
+    )
+  )
+fi
+dnl See if we can issue tkill syscall.
+if test "x$gdb_cv_sys_syscall_h_has_NR_tkill" = "xyes"; then
+  AC_DEFINE(TKILL_SYSCALL_NAME, __NR_tkill, [Name of the tkill syscall.])
+elif test "x$gdb_cv_sys_syscall_h_has_SYS_tkill" = "xyes"; then
+  AC_DEFINE(TKILL_SYSCALL_NAME, SYS_tkill, [Name of the tkill syscall.])
+fi
+
 if test "$srv_xmlfiles" != ""; then
   srv_xmlbuiltin="xml-builtin.o"
   AC_DEFINE(USE_XML, 1, [Define if an XML target description is available.])
Index: gdbserver/linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.117
diff -u -p -r1.117 linux-low.c
--- gdbserver/linux-low.c	21 Dec 2009 17:54:03 -0000	1.117
+++ gdbserver/linux-low.c	21 Dec 2009 19:43:51 -0000
@@ -1572,25 +1572,29 @@ linux_wait (ptid_t ptid,
   return event_ptid;
 }
 
-/* Send a signal to an LWP.  For LinuxThreads, kill is enough; however, if
-   thread groups are in use, we need to use tkill.  */
+/* Send a signal to an LWP.  */
 
 static int
 kill_lwp (unsigned long lwpid, int signo)
 {
-  static int tkill_failed;
+  /* Use tkill, if possible, in case we are using nptl threads.  If tkill
+     fails, then we are not using nptl threads and we should be using kill.  */
 
-  errno = 0;
-
-#ifdef SYS_tkill
-  if (!tkill_failed)
-    {
-      int ret = syscall (SYS_tkill, lwpid, signo);
-      if (errno != ENOSYS)
-	return ret;
-      errno = 0;
-      tkill_failed = 1;
-    }
+#ifdef TKILL_SYSCALL_NAME
+  {
+    static int tkill_failed;
+
+    if (!tkill_failed)
+      {
+	int ret;
+
+	errno = 0;
+	ret = syscall (TKILL_SYSCALL_NAME, lwpid, signo);
+	if (errno != ENOSYS)
+	  return ret;
+	tkill_failed = 1;
+      }
+  }
 #endif
 
   return kill (lwpid, signo);

Re: [patch] fix tkill_failed gcc warning in gdbserver

[Daniel Jacobowitz]

On Mon, Dec 21, 2009 at 12:10:33PM -0800, Doug Evans wrote:
> Hi.
> 
> android doesn't have SYS_tkill but it does have __NR_tkill.

Does it have the other SYS_ constants?  It's a weird omission.
Anyway, might as well use __NR_tkill unconditionally.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] fix tkill_failed gcc warning in gdbserver

[Doug Evans]

On Mon, Dec 21, 2009 at 12:19 PM, Daniel Jacobowitz <drow@false.org> wrote:
> On Mon, Dec 21, 2009 at 12:10:33PM -0800, Doug Evans wrote:
>> Hi.
>>
>> android doesn't have SYS_tkill but it does have __NR_tkill.
>
> Does it have the other SYS_ constants?  It's a weird omission.
> Anyway, might as well use __NR_tkill unconditionally.

It doesn't have any SYS_*.  Sorry for the confusion.

I would like to have used __NR_tkill unconditionally but I couldn't be
sure removing SYS_tkill wouldn't break something.
If you say it's ok, I'll remove it.

Re: [patch] fix tkill_failed gcc warning in gdbserver

[Doug Evans]

On Mon, Dec 21, 2009 at 12:19 PM, Daniel Jacobowitz <drow@false.org> wrote:
> On Mon, Dec 21, 2009 at 12:10:33PM -0800, Doug Evans wrote:
>> Hi.
>>
>> android doesn't have SYS_tkill but it does have __NR_tkill.
>
> Does it have the other SYS_ constants?  It's a weird omission.
> Anyway, might as well use __NR_tkill unconditionally.

The next question is, does gdbserver need to check for
syscall.h/syscall() like gdb does?
Or can I just replace #ifdef SYS_tkill with #ifdef __NR_tkill.
gdbserver includes sys/syscall.h unconditionally, gdb/linux-nat.c  does not.
gdb also checks for __NR_tkill in a way that works if it were an enum.
Does one need to worry about that case?

Re: [patch] fix tkill_failed gcc warning in gdbserver

[Doug Evans]

On Mon, Dec 21, 2009 at 12:36 PM, Doug Evans <dje@google.com> wrote:
> On Mon, Dec 21, 2009 at 12:19 PM, Daniel Jacobowitz <drow@false.org> wrote:
>> On Mon, Dec 21, 2009 at 12:10:33PM -0800, Doug Evans wrote:
>>> Hi.
>>>
>>> android doesn't have SYS_tkill but it does have __NR_tkill.
>>
>> Does it have the other SYS_ constants?  It's a weird omission.
>> Anyway, might as well use __NR_tkill unconditionally.
>
> The next question is, does gdbserver need to check for
> syscall.h/syscall() like gdb does?
> Or can I just replace #ifdef SYS_tkill with #ifdef __NR_tkill.
> gdbserver includes sys/syscall.h unconditionally, gdb/linux-nat.c  does not.
> gdb also checks for __NR_tkill in a way that works if it were an enum.
> Does one need to worry about that case?
>

How about this?

2009-12-21  Doug Evans  <dje@google.com>

        gdb/
        * linux-nat.c (kill_lwp): Minor cleanup, move definition of
        tkill_failed into ifdef HAVE_TKILL_SYSCALL.  Move setting of errno
        there too.  Delete unnecessary resetting of errno after syscall.
	Minor comment changes to match gdbserver/linux-low.c:kill_lwp.

        gdbserver/
        * linux-low.c (kill_lwp): Use __NR_tkill instead of SYS_tkill.
        Move definition of tkill_failed to ifdef __NR_tkill to avoid gcc
        warning ifndef __NR_tkill.  Move setting of errno there too.
        Delete unnecessary resetting of errno after syscall.
        Minor comment changes to match gdb/linux-nat.c:kill_lwp.

Index: linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-nat.c,v
retrieving revision 1.156
diff -u -p -r1.156 linux-nat.c
--- linux-nat.c 20 Nov 2009 19:48:45 -0000      1.156
+++ linux-nat.c 21 Dec 2009 20:39:15 -0000
@@ -2034,27 +2034,29 @@ linux_nat_resume (struct target_ops *ops
     target_async (inferior_event_handler, 0);
 }

-/* Issue kill to specified lwp.  */
-
-static int tkill_failed;
+/* Send a signal to an LWP.  */

 static int
 kill_lwp (int lwpid, int signo)
 {
-  errno = 0;
-
-/* Use tkill, if possible, in case we are using nptl threads.  If tkill
-   fails, then we are not using nptl threads and we should be using kill.  */
+  /* Use tkill, if possible, in case we are using nptl threads.  If tkill
+     fails, then we are not using nptl threads and we should be using kill.  */

 #ifdef HAVE_TKILL_SYSCALL
-  if (!tkill_failed)
-    {
-      int ret = syscall (__NR_tkill, lwpid, signo);
-      if (errno != ENOSYS)
-       return ret;
-      errno = 0;
-      tkill_failed = 1;
-    }
+  {
+    static int tkill_failed;
+
+    if (!tkill_failed)
+      {
+       int ret;
+
+       errno = 0;
+       ret = syscall (__NR_tkill, lwpid, signo);
+       if (errno != ENOSYS)
+         return ret;
+       tkill_failed = 1;
+      }
+  }
 #endif

   return kill (lwpid, signo);
Index: gdbserver/linux-low.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/linux-low.c,v
retrieving revision 1.117
diff -u -p -r1.117 linux-low.c
--- gdbserver/linux-low.c       21 Dec 2009 17:54:03 -0000      1.117
+++ gdbserver/linux-low.c       21 Dec 2009 20:39:15 -0000
@@ -1572,25 +1584,29 @@ linux_wait (ptid_t ptid,

   return event_ptid;
 }

-/* Send a signal to an LWP.  For LinuxThreads, kill is enough; however, if
-   thread groups are in use, we need to use tkill.  */
+/* Send a signal to an LWP.  */

 static int
 kill_lwp (unsigned long lwpid, int signo)
 {
-  static int tkill_failed;
+  /* Use tkill, if possible, in case we are using nptl threads.  If tkill
+     fails, then we are not using nptl threads and we should be using kill.  */

-  errno = 0;
+#ifdef __NR_tkill
+  {
+    static int tkill_failed;

-#ifdef SYS_tkill
-  if (!tkill_failed)
-    {
-      int ret = syscall (SYS_tkill, lwpid, signo);
-      if (errno != ENOSYS)
-       return ret;
-      errno = 0;
-      tkill_failed = 1;
-    }
+    if (!tkill_failed)
+      {
+       int ret;
+
+       errno = 0;
+       ret = syscall (__NR_tkill, lwpid, signo);
+       if (errno != ENOSYS)
+         return ret;
+       tkill_failed = 1;
+      }
+  }
 #endif

   return kill (lwpid, signo);

Re: [patch] fix tkill_failed gcc warning in gdbserver

[Daniel Jacobowitz]

On Mon, Dec 21, 2009 at 12:48:51PM -0800, Doug Evans wrote:
> How about this?
> 
> 2009-12-21  Doug Evans  <dje@google.com>
> 
>         gdb/
>         * linux-nat.c (kill_lwp): Minor cleanup, move definition of
>         tkill_failed into ifdef HAVE_TKILL_SYSCALL.  Move setting of errno
>         there too.  Delete unnecessary resetting of errno after syscall.
> 	Minor comment changes to match gdbserver/linux-low.c:kill_lwp.
> 
>         gdbserver/
>         * linux-low.c (kill_lwp): Use __NR_tkill instead of SYS_tkill.
>         Move definition of tkill_failed to ifdef __NR_tkill to avoid gcc
>         warning ifndef __NR_tkill.  Move setting of errno there too.
>         Delete unnecessary resetting of errno after syscall.
>         Minor comment changes to match gdb/linux-nat.c:kill_lwp.

Fine with me.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] fix tkill_failed gcc warning in gdbserver

[Daniel Jacobowitz]

On Mon, Dec 21, 2009 at 12:36:13PM -0800, Doug Evans wrote:
> The next question is, does gdbserver need to check for
> syscall.h/syscall() like gdb does?
> Or can I just replace #ifdef SYS_tkill with #ifdef __NR_tkill.
> gdbserver includes sys/syscall.h unconditionally, gdb/linux-nat.c  does not.
> gdb also checks for __NR_tkill in a way that works if it were an enum.
> Does one need to worry about that case?

I don't know the answers to any of these.  But if you have Linux
kernel headers, you are going to have __NR_* constants, and I don't
know any Linux C library that doesn't have syscall().

If it breaks, we'll fix it :-)

-- 
Daniel Jacobowitz
CodeSourcery