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

[Hui Zhu <teawater at gmail dot com>]

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