Re: [patch] Allow gdbserver to dynamically lookup libthread_db.so.1

[Paul Pluzhnikov <ppluzhnikov at google dot com>]

On Sun, Oct 4, 2009 at 1:32 PM, Pedro Alves <pedro@codesourcery.com> wrote:

> I take it you only care for extended-remote?  How is the user
> supposed to tweak the new setting with plain remote?

Note that default search path is initialized from LIBTHREAD_DB_SEARCH_PATH.

The user is expected to set this to appropriate system-specific default if
the standard loader search path is inappropriate.

>> Index: gdbserver/acinclude.m4
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/gdbserver/acinclude.m4,v
>> retrieving revision 1.7
>> diff -u -p -u -r1.7 acinclude.m4
>> --- gdbserver/acinclude.m4            5 Jun 2008 22:36:57 -0000              1.7
>> +++ gdbserver/acinclude.m4            2 Oct 2009 23:49:31 -0000
>> @@ -22,7 +22,7 @@ AC_DEFUN([SRV_CHECK_THREAD_DB],
>>       void ps_get_thread_area() {}
>>       void ps_getpid() {}],
>>      [td_ta_new();],
>> -   [srv_cv_thread_db="-lthread_db"],
>> +   [srv_cv_thread_db="-ldl"],
>>      [srv_cv_thread_db=no
>>
>>    if test "$prefix" = "/usr" || test "$prefix" = "NONE"; then
>> @@ -42,28 +42,9 @@ AC_DEFUN([SRV_CHECK_THREAD_DB],
>>       void ps_get_thread_area() {}
>>       void ps_getpid() {}],
>>      [td_ta_new();],
>> -   [srv_cv_thread_db="$thread_db"],
>> +   [srv_cv_thread_db="-ldl"],
>>      [srv_cv_thread_db=no])
>>      ])
>>    LIBS="$old_LIBS"
>
> This doesn't make sense.

Why not?

If we can find '-lthread_db', '/lib/libthread_db.so.1' or
'$prefix/lib/libthread_db.so.1', then we switch on using libthread_db and
use '-ldl' to dynamically load it.

Perhaps a better fix is to skip this check altogether, and always use
dlopen on Linux?

>> +   /* Handle of the libthread_db from dlopen.   */
>> +   void *handle;
...
>
> Although gdbserver doesn't have a target stack concept, let's try to keep the
> layers a bit separate.   Could you please make this a new (private) structure
> in thread-db.c, and then have a new pointer here, say
> process_info_private->thread_db into such an object?

Done. Several existing members of struct process_info_private,
e.g. thread_agent also are thread_db-private, so I moved them as well.

I believe I've addressed all your other comments.

Tested on Linux/x86_64 (with "--target_board native-gdbserver"), no
regressions.

Thanks,
-- 
Paul Pluzhnikov

doc/
2009-10-06  Paul Pluzhnikov  <ppluzhnikov@google.com>

       * gdb.texinfo (Server): Document libthread-db-search-path.

gdbserver/
2009-10-06  Paul Pluzhnikov  <ppluzhnikov@google.com>


       * acinclude.m4: Link with libdl.
       * configure.ac: Don't check for tls_get_address.
       * linux-low.h (struct process_info_private): Move members to struct
       thread_db.
       (thread_db_free): New prototype.
       * linux-low.c (linux_remove_process): Adjust.
       (linux_wait_for_event_1, linux_look_up_symbols): Likewise.
       * server.c (handle_query): Move code ...
       (handle_monitor_command): ... here.
       * server.h (handle_monitor_command): New prototype.
       * target.h (struct target_ops): New member.
       * thread-db.c (struct thread_db): New.
       (libthread_db_search_path): New variable.
       (thread_db_create_event, thread_db_enable_reporting)
       (find_one_thread, maybe_attach_thread, find_new_threads_callback)
       (thread_db_find_new_threads, (thread_db_get_tls_address): Adjust.
       (try_thread_db_load_1, dladdr_to_soname): New functions.
       (try_thread_db_load, thread_db_load_search): New functions.
       (thread_db_init): Search for libthread_db.
       (thread_db_free): New function.
       (thread_db_handle_monitor_command): Likewise.

Attachment: gdb-gdbserver-threaddb-search-20091006.txt
Description: Text document