Re: [patch][rfc] Allow GDB to search for the right libthread_db.so.1

[Hui Zhu <teawater at gmail dot com>]

In function thread_db_load, I suggest try_thread_db_load with
libthread_db_search_path first.
Cause I think if user set a directory, he must want it be loaded.


Thanks,
Hui

On Thu, Apr 23, 2009 at 16:05, Hui Zhu <teawater@gmail.com> wrote:
> On Thu, Apr 23, 2009 at 15:01, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
>> On Wed, Apr 22, 2009 at 11:21 PM, Hui Zhu <teawater@gmail.com> wrote:
>>
>>> About your patch, I think let user choice load which libthread_db is
>>> very cool idea.
>>> Why not let they set which file they want to load directly?
>>>
>>> For example:
>>> set libthread-db /xxx_dir/libxxx
>>
>> As I stated at the start of this thread, we have a mixture of
>> executables: some are linked statically, some dynamically, and
>> against several (incompatible WRT libthread_db) glibc versions.
>>
>> I'd like GDB to work "automagically" for all such executables,
>> without the end user having to understand and specify exactly
>> which libthread_db must be loaded for each one [1].
>>
>> The patch allows me to achieve that (all I need to do is provide
>> appropriate local definition of LIBTHREAD_DB_SEARCH_PATH).
>>
>> [1] Understanding this requires the user to understand how
>> GDB uses libthread_db and the relationship between libpthread and
>> libthread_db. Also, the mapping is complicated by the fact that
>> 64-bit GDB is sometimes used to debug 32-bit inferiors.
>>
>
> I read your patch again, And I think let user set file name is not
> conflict with your patch.
> In thread_db_load_search:
> + ?while (*search_path)
> + ? ?{
> + ? ? ?const char *end = strchr (search_path, ':');
> + ? ? ?if (end)
> + ? ? ? {
> + ? ? ? ? size_t len = end - search_path;
> + ? ? ? ? ?if (len + 1 + strlen (LIBTHREAD_DB_SO) + 1 > sizeof (path))
> + ? ? ? ? ? ?{
> + ? ? ? ? ? ? ?char *cp = xmalloc (len + 1);
> + ? ? ? ? ? ? ?memcpy (cp, search_path, len);
> + ? ? ? ? ? ? ?cp[len] = '\0';
> + ? ? ? ? ? ? ?warning (_("libthread_db_search_path component too long,"
> + ? ? ? ? ? ? ? ? ? ? ? ? " ignored: %s."), cp);
> + ? ? ? ? ? ? ?xfree (cp);
> + ? ? ? ? ? ? ?search_path += len + 1;
> + ? ? ? ? ? ? ?continue;
> + ? ? ? ? ? ?}
> + ? ? ? ? memcpy (path, search_path, len);
> + ? ? ? ? path[len] = '\0';
> + ? ? ? ? search_path += len + 1;
> + ? ? ? }
> + ? ? ?else
> + ? ? ? {
> + ? ? ? ? ?size_t len = strlen (search_path);
> +
> + ? ? ? ? ?if (len + 1 + strlen (LIBTHREAD_DB_SO) + 1 > sizeof (path))
> + ? ? ? ? ? ?{
> + ? ? ? ? ? ? ?warning (_("libthread_db_search_path component too long,"
> + ? ? ? ? ? ? ? ? ? ? ? ? " ignored: %s."), search_path);
> + ? ? ? ? ? ? ?break;
> + ? ? ? ? ? ?}
> + ? ? ? ? memcpy (path, search_path, len + 1);
> + ? ? ? ? search_path += len;
> + ? ? ? }
>
> When you get a path, you can check if this is a directory.
>
> If this is a directory, do following job.
>
> + ? ? ?strcat (path, "/");
> + ? ? ?strcat (path, LIBTHREAD_DB_SO);
> + ? ? ?if (try_thread_db_load (path))
> + ? ? ? {
> + ? ? ? ? rc = 1;
> + ? ? ? ? break;
> + ? ? ? }
>
> If this is a normal file, try_thread_db_load (path) without strcat
> LIBTHREAD_DB_SO.
>
> What do you think about it?
>
>
> And in thread_db_load:
> + ?msym = lookup_minimal_symbol ("nptl_version", NULL, NULL);
> + ?if (!msym)
> + ? ?msym = lookup_minimal_symbol ("__linuxthreads_version", NULL, NULL);
> +
> + ?/* Some really old libpthread versions do not have either of the above. ?*/
> + ?if (!msym)
> + ? ?msym = lookup_minimal_symbol ("__pthread_threads_events", NULL, NULL);
> +
> + ?if (!msym)
> + ? ?/* No threads yet */
> + ? ?return 0;
>
> You really don't want gdb try it with libthread_db?
> If in the future, this code doesn't cover everything. ?And I think let
> gdb try will not affect anything. ?:)
>
> Thanks,
> Hui
>