Re: [PATCH 04/10] Remove support for thread events without TRACE_EVENT_CLONE in GDBServer.

Pedro Alves
2015-11-04T18:19:27Z

On 10/29/2015 05:09 PM, Antoine Tremblay wrote:
> This patch removes support for thread events if TRACE_EVENT_CLONE is not
> supported in GDBServer.

(I had never seen it spelled without the leading P(RACE_EVENT_CLONE).
Is that a typo? (subject too).)

>
> Before, on systems that did not support PTRACE_EVENT_CLONE, both GDB and
> GDBServer coordinated with libthread_db.so to insert breakpoints at magic
> locations in libpthread.so, in order to break at thread creation and thread
> death.
>
> Simple software single stepping support was implemented to step over these
> breakpoints in case there was no hardware single stepping support. However,
> these simple software single stepping implementations were not fit for any other
> use as discussed in :
> https://sourceware.org/ml/gdb-patches/2015-04/msg01110.html
>
> These too simple implementations conflict with ongoing work to make proper
> implementations of software single stepping in GDBServer.
>
> The problem is that if some implementations are correct and others are not and
> only there for the thread magic breakpoint, we can't enable features based
> solely software single step support since some would be broken.
>
> To keep the incorrect implementations and allow the new proper ones at the same
> time we would need to implement fallback code and it quickly becomes ugly and
> confusing with multiple checks for legacy software single step or proper
> software single step.
>
> However, PTRACE_EVENT_CLONE was first introduced in Linux 2.5.46,
> released in November 2002.
>
> So I think it's reasonable to just remove support for kernels that don't support
> PTRACE_EVENT_CLONE, and sidestep the libthread_db breakpoints issues entirely.
>
> This thread on the mailling list discusses the issue :
> https://sourceware.org/ml/gdb/2015-10/msg00078.html
>
> No regressions, tested on ubuntu 14.04 ARMv7 and x86.
> With gdbserver-{native,extended} / { -marm -mthumb }
>
> gdb/gdbserver/ChangeLog:
>
> * linux-low.c (linux_look_up_symbols): Don't call
> linux_supports_traceclone.
> * linux-low.h (thread_db_init): Remove use_events argument.
> * thread-db.c (thread_db_use_event): Remove global variable.
> (struct thread_db) <td_thr_event_enable_ftype>: Remove field.
> (thread_db_create_event): Remove function.
> (thread_db_enable_reporting): Likewise.
> (find_one_thread): Don't check for thread_db_use_events.
> (attach_thread): Likewise.
> (thread_db_load_search): Remove td_thr_event_enable_p initialization.
> (try_thread_db_load_1): Don't check for thread_db_use_events.
> (thread_db_init): Remove use_events argument.
> (thread_db_init): Remove thread events handling.

LGTM.

Thanks,
Pedro Alves