Patchwork [1/2] darwin: Silence syscall deprecated declaration warning

login
register
mail settings
Submitter Simon Marchi
Date July 4, 2018, 4:30 a.m.
Message ID <20180704043033.29212-1-simon.marchi@polymtl.ca>
Download mbox | patch
Permalink /patch/28221/
State New
Headers show

Comments

Simon Marchi - July 4, 2018, 4:30 a.m.
This patch silences this warning:

/Users/simark/src/binutils-gdb/gdb/darwin-nat.c:839:10: error: 'syscall' is deprecated: first deprecated in macOS 10.12 - syscall(2) is unsupported; please switch to a supported interface. For SYS_kdebug_trace use kdebug_signpost(). [-Werror,-Wdeprecated-declarations]
          res = syscall (SYS___pthread_kill, thread->gdb_port, nsignal);
                ^
/usr/include/unistd.h:745:6: note: 'syscall' has been explicitly marked deprecated here
int      syscall(int, ...);
         ^

I guess it would be good to find a non-deprecated alternative for
sending that signal to a specific thread, but I have not idea what we
could use instead (not sure if plain kill would do the trick).

gdb/ChangeLog:

	* darwin-nat.c (darwin_resume_thread): Silence syscall
	deprecated declaration warning.
---
 gdb/darwin-nat.c      | 3 +++
 include/diagnostics.h | 6 ++++++
 2 files changed, 9 insertions(+)
Pedro Alves - July 4, 2018, 10:30 a.m.
On 07/04/2018 05:30 AM, Simon Marchi wrote:
> This patch silences this warning:
> 
> /Users/simark/src/binutils-gdb/gdb/darwin-nat.c:839:10: error: 'syscall' is deprecated: first deprecated in macOS 10.12 - syscall(2) is unsupported; please switch to a supported interface. For SYS_kdebug_trace use kdebug_signpost(). [-Werror,-Wdeprecated-declarations]
>           res = syscall (SYS___pthread_kill, thread->gdb_port, nsignal);
>                 ^
> /usr/include/unistd.h:745:6: note: 'syscall' has been explicitly marked deprecated here
> int      syscall(int, ...);
>          ^
> 
> I guess it would be good to find a non-deprecated alternative for
> sending that signal to a specific thread, but I have not idea what we
> could use instead (not sure if plain kill would do the trick).

Plain kill probably would not, as that it not directed to a specific
thread -- any thread could dequeue it.

My immediate question when reading this was "why not use the
pthread_kill C function instead of calling the syscall directly?"

I then followed the rabbit down the hole, starting here:

 https://stackoverflow.com/questions/44990839/pthread-kill-to-a-gcd-managed-thread

which lead to pthread_kill's sources, here:

  https://github.com/apple/darwin-libpthread/blob/768e71947f16da454ea05c4a98f77a0f14358f21/src/pthread.c#L1412

So it's very likely we use the syscall directly instead of pthread_kill because
we want to be able to send a signal to all kinds of threads, including
worker threads underlying GCD.  I wish there was a comment to the effect here.

I also peeked at debugserver's sources in lldb, and it seems to me that
lldb doesn't send a signal to the thread unless it was stopped on a mach
exception?  

Actually, I wonder if that syscall in gdb is really ever reached...  We
shouldn't be calling darwin_resume_thread on non-resumed threads, AFAICT,
so why do we handle that?  See comment underlined below -- when isn't
the process stopped when we get there?

static void
darwin_resume_thread (struct inferior *inf, darwin_thread_t *thread,
		      int step, int nsignal)
{
..
  switch (thread->msg_state)
    {
    case DARWIN_MESSAGE:
      if (thread->event.ex_type == EXC_SOFTWARE
	  && thread->event.ex_data[0] == EXC_SOFT_SIGNAL)
	{
...
	}
      else if (nsignal)
	{
	  /* Note: ptrace is allowed only if the process is stopped.
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	     Directly send the signal to the thread.  */
	  res = syscall (SYS___pthread_kill, thread->gdb_port, nsignal);
          ...
	}

Thanks,
Pedro Alves
Simon Marchi - July 4, 2018, 4:39 p.m.
On 2018-07-04 06:30, Pedro Alves wrote:
> Plain kill probably would not, as that it not directed to a specific
> thread -- any thread could dequeue it.
> 
> My immediate question when reading this was "why not use the
> pthread_kill C function instead of calling the syscall directly?"
> 
> I then followed the rabbit down the hole, starting here:
> 
> 
> https://stackoverflow.com/questions/44990839/pthread-kill-to-a-gcd-managed-thread
> 
> which lead to pthread_kill's sources, here:
> 
> 
> https://github.com/apple/darwin-libpthread/blob/768e71947f16da454ea05c4a98f77a0f14358f21/src/pthread.c#L1412
> 
> So it's very likely we use the syscall directly instead of pthread_kill 
> because
> we want to be able to send a signal to all kinds of threads, including
> worker threads underlying GCD.  I wish there was a comment to the 
> effect here.

Indeed, I saw that too.  I can only assume that this was Tristan's 
intention when writing the code.  I can add the comment if we're 
confident enough that it's the case.

> I also peeked at debugserver's sources in lldb, and it seems to me that
> lldb doesn't send a signal to the thread unless it was stopped on a 
> mach
> exception?
> 
> Actually, I wonder if that syscall in gdb is really ever reached...  We
> shouldn't be calling darwin_resume_thread on non-resumed threads, 
> AFAICT,
> so why do we handle that?  See comment underlined below -- when isn't
> the process stopped when we get there?
> 
> static void
> darwin_resume_thread (struct inferior *inf, darwin_thread_t *thread,
> 		      int step, int nsignal)
> {
> ..
>   switch (thread->msg_state)
>     {
>     case DARWIN_MESSAGE:
>       if (thread->event.ex_type == EXC_SOFTWARE
> 	  && thread->event.ex_data[0] == EXC_SOFT_SIGNAL)
> 	{
> ...
> 	}
>       else if (nsignal)
> 	{
> 	  /* Note: ptrace is allowed only if the process is stopped.
>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 	     Directly send the signal to the thread.  */
> 	  res = syscall (SYS___pthread_kill, thread->gdb_port, nsignal);
>           ...
> 	}

Just guessing here.  It looks like when the thread generates mach 
exception (breakpoint, unix signal, bad instruction, etc), it "sends" a 
message (in Darwin jargon) and is stuck waiting for a reply.  GDB needs 
to reply to it to unblock the thread (the call to darwin_send_reply).  
In this case, the thread might be stopped from the point of view of the 
user and GDB, but not from the point of view of the OS.  So that's why, 
if we want to inject a signal, we have to use __pthread_kill.  I don't 
know why the (ex_type == EXC_SOFTWARE && ex_data[0] == EXC_SOFT_SIGNAL) 
is treated differently though.

In any case, changing this would require a deeper analysis and knowledge 
of the Darwin kernel and some time, and I don't have either :).  Would 
you still be fine with silencing the warning for now to get it out of 
the way?

Simon
Pedro Alves - July 5, 2018, 4:12 p.m.
On 07/04/2018 05:39 PM, Simon Marchi wrote:
> On 2018-07-04 06:30, Pedro Alves wrote:
>> Plain kill probably would not, as that it not directed to a specific
>> thread -- any thread could dequeue it.
>>
>> My immediate question when reading this was "why not use the
>> pthread_kill C function instead of calling the syscall directly?"
>>
>> I then followed the rabbit down the hole, starting here:
>>
>>
>> https://stackoverflow.com/questions/44990839/pthread-kill-to-a-gcd-managed-thread
>>
>> which lead to pthread_kill's sources, here:
>>
>>
>> https://github.com/apple/darwin-libpthread/blob/768e71947f16da454ea05c4a98f77a0f14358f21/src/pthread.c#L1412
>>
>> So it's very likely we use the syscall directly instead of pthread_kill because
>> we want to be able to send a signal to all kinds of threads, including
>> worker threads underlying GCD.  I wish there was a comment to the effect here.
> 
> Indeed, I saw that too.  I can only assume that this was Tristan's intention when writing the code.  I can add the comment if we're confident enough that it's the case.

I'm reasonably confident.

> 
>> I also peeked at debugserver's sources in lldb, and it seems to me that
>> lldb doesn't send a signal to the thread unless it was stopped on a mach
>> exception?
>>
>> Actually, I wonder if that syscall in gdb is really ever reached...  We
>> shouldn't be calling darwin_resume_thread on non-resumed threads, AFAICT,
>> so why do we handle that?  See comment underlined below -- when isn't
>> the process stopped when we get there?
>>
>> static void
>> darwin_resume_thread (struct inferior *inf, darwin_thread_t *thread,
>>               int step, int nsignal)
>> {
>> ..
>>   switch (thread->msg_state)
>>     {
>>     case DARWIN_MESSAGE:
>>       if (thread->event.ex_type == EXC_SOFTWARE
>>       && thread->event.ex_data[0] == EXC_SOFT_SIGNAL)
>>     {
>> ...
>>     }
>>       else if (nsignal)
>>     {
>>       /* Note: ptrace is allowed only if the process is stopped.
>>              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>          Directly send the signal to the thread.  */
>>       res = syscall (SYS___pthread_kill, thread->gdb_port, nsignal);
>>           ...
>>     }
> 
> Just guessing here.  It looks like when the thread generates mach exception (breakpoint, unix signal, bad instruction, etc), it "sends" a message (in Darwin jargon) and is stuck waiting for a reply.  GDB needs to reply to it to unblock the thread (the call to darwin_send_reply).  In this case, the thread might be stopped from the point of view of the user and GDB, but not from the point of view of the OS.  So that's why, if we want to inject a signal, we have to use __pthread_kill.  I don't know why the (ex_type == EXC_SOFTWARE && ex_data[0] == EXC_SOFT_SIGNAL) is treated differently though.

If a thread was suspended by GDB, with task_suspend, I'd assume
that the thread's msg_state would not be DARWIN_MESSAGE, so we wouldn't
reach here.  But just guessing too.

> 
> In any case, changing this would require a deeper analysis and knowledge of the Darwin kernel and some time, and I don't have either :).  Would you still be fine with silencing the warning for now to get it out of the way?
Yes, of course.  I'd suggest wrapping this in a function though
(darwin_pthread_kill or some such), so that the silencing
would be contained there out of view.  We can also mention why we
need the function in its intro comment too.

Thanks,
Pedro Alves

Patch

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 8104de53e7f8..95b89aaae302 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -836,7 +836,10 @@  darwin_resume_thread (struct inferior *inf, darwin_thread_t *thread,
 	{
 	  /* Note: ptrace is allowed only if the process is stopped.
 	     Directly send the signal to the thread.  */
+	  DIAGNOSTIC_PUSH;
+	  DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS;
 	  res = syscall (SYS___pthread_kill, thread->gdb_port, nsignal);
+	  DIAGNOSTIC_POP;
 	  inferior_debug (4, _("darwin_resume_thread: kill 0x%x %d: %d\n"),
 			  thread->gdb_port, nsignal, res);
 	  thread->signaled = 1;
diff --git a/include/diagnostics.h b/include/diagnostics.h
index 4a674106dc01..34fc01b85bd4 100644
--- a/include/diagnostics.h
+++ b/include/diagnostics.h
@@ -35,6 +35,8 @@ 
 #if defined (__clang__) /* clang */
 
 # define DIAGNOSTIC_IGNORE_SELF_MOVE DIAGNOSTIC_IGNORE ("-Wself-move")
+# define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS \
+  DIAGNOSTIC_IGNORE ("-Wdeprecated-declarations")
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER \
   DIAGNOSTIC_IGNORE ("-Wdeprecated-register")
 # define DIAGNOSTIC_IGNORE_UNUSED_FUNCTION \
@@ -56,6 +58,10 @@ 
 # define DIAGNOSTIC_IGNORE_SELF_MOVE
 #endif
 
+#ifndef DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
+# define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
+#endif
+
 #ifndef DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
 # define DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER
 #endif