Message ID | 20180704043033.29212-1-simon.marchi@polymtl.ca |
---|---|
State | New, archived |
Headers |
Received: (qmail 31504 invoked by alias); 4 Jul 2018 04:31:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 31263 invoked by uid 89); 4 Jul 2018 04:31:16 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL, TIME_LIMIT_EXCEEDED autolearn=unavailable version=3.3.2 spammy=Hx-languages-length:2357 X-HELO: barracuda.ebox.ca Received: from barracuda.ebox.ca (HELO barracuda.ebox.ca) (96.127.255.19) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 04 Jul 2018 04:30:54 +0000 Received: from smtp.ebox.ca (smtp.electronicbox.net [96.127.255.82]) by barracuda.ebox.ca with ESMTP id GtrAUIZqlP3bAuFY (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 04 Jul 2018 00:30:35 -0400 (EDT) Received: from simark.lan (unknown [192.222.164.54]) by smtp.ebox.ca (Postfix) with ESMTP id BB216441D64; Wed, 4 Jul 2018 00:30:35 -0400 (EDT) From: Simon Marchi <simon.marchi@polymtl.ca> To: gdb-patches@sourceware.org Cc: tom@tromey.com, Simon Marchi <simon.marchi@polymtl.ca> Subject: [PATCH 1/2] darwin: Silence syscall deprecated declaration warning Date: Wed, 4 Jul 2018 00:30:32 -0400 Message-Id: <20180704043033.29212-1-simon.marchi@polymtl.ca> X-IsSubscribed: yes |
Commit Message
Simon Marchi
July 4, 2018, 4:30 a.m. UTC
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(+)
Comments
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
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
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
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