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

[Hui Zhu <teawater@gmail.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
>