Re: [RFA] Add support for $sdir and $pdir to libthread-db-search-path

Doug Evans
2011-05-13T22:38:49Z

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

On Thu, May 12, 2011 at 1:12 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> This patch adds $sdir to the patch referenced here:
> Doug> http://sourceware.org/ml/gdb-patches/2011-04/msg00553.html
>
> Doug> and puts $sdir ahead of $pdir.
> Doug> If/when gdb gets a security model I think $pdir should be moved
> Doug> ahead of $sdir.
>
> Doug> It also simplifies some of the code to parse libthread-db-search-path.
>
> Doug> Ok to check in?
>
> It looks ok to me.
>
> Well, actually, I think the pre-existing code has some problems, but
> they aren't introduced by your patch.  I will clean them up separately.

Here is what I checked in.

2011-05-13 Doug Evans <dje@google.com>

Support $pdir and $sdir in libthread-db-search-path.
* NEWS: Mention $sdir,$pdir.
* gdb_thread_db.h (LIBTHREAD_DB_SEARCH_PATH): Add $sdir:$pdir.
* linux-thread-db.c (try_thread_db_load_from_pdir): New
function.
(try_thread_db_load_from_sdir): New function.
(try_thread_db_load_from_dir): New function.
(thread_db_load_search): Handle $pdir, $sdir. Remove trying
of
system directories if search of libthread-db-search-path
fails,
that is now done via $sdir.
(has_libpthread): New function.
(thread_db_load): Remove search for libthread_db in directory
of
libpthread, that is now done via $pdir.

gdbserver/
* thread-db.c (try_thread_db_load_from_sdir): New function.
(try_thread_db_load_from_dir): New function.
(thread_db_load_search): Handle $sdir, ignore $pdir.
Remove trying of system directories if search of
libthread-db-search-path fails, that is now done via $sdir.

doc/
* gdb.texinfo (Threads): Document $sdir,$pdir.
(Server): Document $pdir exception.

[-- Attachment #2: gdb-110513-pdir-3.patch.txt --]
[-- Type: text/plain, Size: 15608 bytes --]

2011-05-13 Doug Evans <dje@google.com>

Support $pdir and $sdir in libthread-db-search-path.
* NEWS: Mention $sdir,$pdir.
* gdb_thread_db.h (LIBTHREAD_DB_SEARCH_PATH): Add $sdir:$pdir.
* linux-thread-db.c (try_thread_db_load_from_pdir): New function.
(try_thread_db_load_from_sdir): New function.
(try_thread_db_load_from_dir): New function.
(thread_db_load_search): Handle $pdir, $sdir. Remove trying of
system directories if search of libthread-db-search-path fails,
that is now done via $sdir.
(has_libpthread): New function.
(thread_db_load): Remove search for libthread_db in directory of
libpthread, that is now done via $pdir.

gdbserver/
* thread-db.c (try_thread_db_load_from_sdir): New function.
(try_thread_db_load_from_dir): New function.
(thread_db_load_search): Handle $sdir, ignore $pdir.
Remove trying of system directories if search of
libthread-db-search-path fails, that is now done via $sdir.

doc/
* gdb.texinfo (Threads): Document $sdir,$pdir.
(Server): Document $pdir exception.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.440
diff -u -p -r1.440 NEWS
--- NEWS 13 May 2011 22:11:46 -0000 1.440
+++ NEWS 13 May 2011 22:28:30 -0000
@@ -3,6 +3,20 @@

*** Changes since GDB 7.3

+* libthread-db-search-path now supports two special values: $sdir and $pdir.
+ $sdir specifies the default system locations of shared libraries.
+ $pdir specifies the directory where the libpthread used by the application
+ lives.
+
+ GDB no longer looks in $sdir and $pdir after it has searched the directories
+ mentioned in libthread-db-search-path. If you want to search those
+ directories, they must be specified in libthread-db-search-path.
+ The default value of libthread-db-search-path on GNU/Linux and Solaris
+ systems is now "$sdir:$pdir".
+
+ $pdir is not supported by gdbserver, it is currently ignored.
+ $sdir is supported by gdbserver.
+
* New configure option --with-iconv-bin.
When using the internationalization support like the one in the GNU C
library, GDB will invoke the "iconv" program to get a list of supported
Index: gdb_thread_db.h
===================================================================
RCS file: /cvs/src/src/gdb/gdb_thread_db.h,v
retrieving revision 1.16
diff -u -p -r1.16 gdb_thread_db.h
--- gdb_thread_db.h 7 Jan 2011 19:36:17 -0000 1.16
+++ gdb_thread_db.h 13 May 2011 22:28:30 -0000
@@ -6,7 +6,10 @@
#endif

#ifndef LIBTHREAD_DB_SEARCH_PATH
-#define LIBTHREAD_DB_SEARCH_PATH ""
+/* $sdir appears before $pdir for some minimal security protection:
+ we trust the system libthread_db.so a bit more than some random
+ libthread_db associated with whatever libpthread the app is using. */
+#define LIBTHREAD_DB_SEARCH_PATH "$sdir:$pdir"
#endif

#else
Index: linux-thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-thread-db.c,v
retrieving revision 1.84
diff -u -p -r1.84 linux-thread-db.c
--- linux-thread-db.c 10 May 2011 18:45:22 -0000 1.84
+++ linux-thread-db.c 13 May 2011 22:28:30 -0000
@@ -812,73 +812,163 @@ try_thread_db_load (const char *library)
return 0;
}

+/* Handle $pdir in libthread-db-search-path.
+ Look for libthread_db in the directory of libpthread.
+ The result is true for success. */
+
+static int
+try_thread_db_load_from_pdir (void)
+{
+ struct objfile *obj;
+
+ ALL_OBJFILES (obj)
+ if (libpthread_name_p (obj->name))
+ {
+ char path[PATH_MAX], *cp;
+
+ gdb_assert (strlen (obj->name) < sizeof (path));
+ strcpy (path, obj->name);
+ cp = strrchr (path, '/');
+
+ if (cp == NULL)
+ {
+ warning (_("Expected absolute pathname for libpthread in the"
+ " inferior, but got %s."), path);
+ }
+ else if (cp + 1 + strlen (LIBTHREAD_DB_SO) + 1 > path + sizeof (path))
+ {
+ warning (_("Unexpected: path to libpthread in the inferior is"
+ " too long: %s"), path);
+ }
+ else
+ {
+ strcpy (cp + 1, LIBTHREAD_DB_SO);
+ if (try_thread_db_load (path))
+ return 1;
+ }
+ return 0;
+ }
+
+ return 0;
+}
+
+/* Handle $sdir in libthread-db-search-path.
+ Look for libthread_db in the system dirs, or wherever a plain
+ dlopen(file_without_path) will look.
+ The result is true for success. */
+
+static int
+try_thread_db_load_from_sdir (void)
+{
+ return try_thread_db_load (LIBTHREAD_DB_SO);
+}
+
+/* Try to load libthread_db from directory DIR of length DIR_LEN.
+ The result is true for success. */
+
+static int
+try_thread_db_load_from_dir (const char *dir, size_t dir_len)
+{
+ char path[PATH_MAX];
+
+ if (dir_len + 1 + strlen (LIBTHREAD_DB_SO) + 1 > sizeof (path))
+ {
+ char *cp = xmalloc (dir_len + 1);
+
+ memcpy (cp, dir, dir_len);
+ cp[dir_len] = '\0';
+ warning (_("libthread-db-search-path component too long,"
+ " ignored: %s."), cp);
+ xfree (cp);
+ return 0;
+ }
+
+ memcpy (path, dir, dir_len);
+ path[dir_len] = '/';
+ strcpy (path + dir_len + 1, LIBTHREAD_DB_SO);
+ return try_thread_db_load (path);
+}
+
/* Search libthread_db_search_path for libthread_db which "agrees"
- to work on current inferior. */
+ to work on current inferior.
+ The result is true for success. */

static int
thread_db_load_search (void)
{
- char path[PATH_MAX];
const char *search_path = libthread_db_search_path;
int rc = 0;

while (*search_path)
{
const char *end = strchr (search_path, ':');
+ const char *this_dir = search_path;
+ size_t this_dir_len;

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;
+ this_dir_len = end - search_path;
+ search_path += this_dir_len + 1;
}
else
{
- size_t len = strlen (search_path);
+ this_dir_len = strlen (this_dir);
+ search_path += this_dir_len;
+ }

- 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;
- }
- strcat (path, "/");
- strcat (path, LIBTHREAD_DB_SO);
- if (try_thread_db_load (path))
+ if (this_dir_len == sizeof ("$pdir") - 1
+ && strncmp (this_dir, "$pdir", this_dir_len) == 0)
{
- rc = 1;
- break;
+ if (try_thread_db_load_from_pdir ())
+ {
+ rc = 1;
+ break;
+ }
+ }
+ else if (this_dir_len == sizeof ("$sdir") - 1
+ && strncmp (this_dir, "$sdir", this_dir_len) == 0)
+ {
+ if (try_thread_db_load_from_sdir ())
+ {
+ rc = 1;
+ break;
+ }
+ }
+ else
+ {
+ if (try_thread_db_load_from_dir (this_dir, this_dir_len))
+ {
+ rc = 1;
+ break;
+ }
}
}
- if (rc == 0)
- rc = try_thread_db_load (LIBTHREAD_DB_SO);
+
+ if (libthread_db_debug)
+ printf_unfiltered (_("thread_db_load_search returning %d\n"), rc);
return rc;
}

+/* Return non-zero if the inferior has a libpthread. */
+
+static int
+has_libpthread (void)
+{
+ struct objfile *obj;
+
+ ALL_OBJFILES (obj)
+ if (libpthread_name_p (obj->name))
+ return 1;
+
+ return 0;
+}
+
/* Attempt to load and initialize libthread_db.
Return 1 on success. */

static int
thread_db_load (void)
{
- struct objfile *obj;
struct thread_db_info *info;

info = get_thread_db_info (GET_PID (inferior_ptid));
@@ -898,39 +988,15 @@ thread_db_load (void)
if (thread_db_load_search ())
return 1;

- /* None of the libthread_db's on our search path, not the system default
- ones worked. If the executable is dynamically linked against
- libpthread, try loading libthread_db from the same directory. */
-
- ALL_OBJFILES (obj)
- if (libpthread_name_p (obj->name))
- {
- char path[PATH_MAX], *cp;
-
- gdb_assert (strlen (obj->name) < sizeof (path));
- strcpy (path, obj->name);
- cp = strrchr (path, '/');
-
- if (cp == NULL)
- {
- warning (_("Expected absolute pathname for libpthread in the"
- " inferior, but got %s."), path);
- }
- else if (cp + 1 + strlen (LIBTHREAD_DB_SO) + 1 > path + sizeof (path))
- {
- warning (_("Unexpected: path to libpthread in the inferior is"
- " too long: %s"), path);
- }
- else
- {
- strcpy (cp + 1, LIBTHREAD_DB_SO);
- if (try_thread_db_load (path))
- return 1;
- }
- warning (_("Unable to find libthread_db matching inferior's thread"
- " library, thread debugging will not be available."));
- return 0;
+ /* We couldn't find a libthread_db.
+ If the inferior has a libpthread warn the user. */
+ if (has_libpthread ())
+ {
+ warning (_("Unable to find libthread_db matching inferior's thread"
+ " library, thread debugging will not be available."));
+ return 0;
}
+
/* Either this executable isn't using libpthread at all, or it is
statically linked. Since we can't easily distinguish these two cases,
no warning is issued. */
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.837
diff -u -p -r1.837 gdb.texinfo
--- doc/gdb.texinfo 13 May 2011 22:11:46 -0000 1.837
+++ doc/gdb.texinfo 13 May 2011 22:28:30 -0000
@@ -2858,16 +2858,22 @@ watchpoints in programs with multiple th
If this variable is set, @var{path} is a colon-separated list of
directories @value{GDBN} will use to search for @code{libthread_db}.
If you omit @var{path}, @samp{libthread-db-search-path} will be reset to
-its default value (an empty list on @sc{gnu}/Linux and Solaris systems).
+its default value (@code{$sdir:$pdir} on @sc{gnu}/Linux and Solaris systems).
Internally, the default value comes from the @code{LIBTHREAD_DB_SEARCH_PATH}
macro.

On @sc{gnu}/Linux and Solaris systems, @value{GDBN} uses a ``helper''
@code{libthread_db} library to obtain information about threads in the
inferior process. @value{GDBN} will use @samp{libthread-db-search-path}
-to find @code{libthread_db}. If that fails, @value{GDBN} will continue
-with default system shared library directories, and finally the directory
-from which @code{libpthread} was loaded in the inferior process.
+to find @code{libthread_db}.
+
+A special entry @samp{$sdir} for @samp{libthread-db-search-path}
+refers to the default system directories that are
+normally searched for loading shared libraries.
+
+A special entry @samp{$pdir} for @samp{libthread-db-search-path}
+refers to the directory from which @code{libpthread}
+was loaded in the inferior process.

For any @code{libthread_db} library @value{GDBN} finds in above directories,
@value{GDBN} attempts to initialize it with the current inferior process.
@@ -16382,6 +16388,9 @@ directories to search for @code{libthrea
libthread-db-search-path}). If you omit @var{path},
@samp{libthread-db-search-path} will be reset to its default value.

+The special entry @samp{$pdir} for @samp{libthread-db-search-path} is
+not supported in @code{gdbserver}.
+
@item monitor exit
Tell gdbserver to exit immediately. This command should be followed by
@code{disconnect} to close the debugging session. @code{gdbserver} will
Index: gdbserver/thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/thread-db.c,v
retrieving revision 1.40
diff -u -p -r1.40 thread-db.c
--- gdbserver/thread-db.c 10 May 2011 16:53:23 -0000 1.40
+++ gdbserver/thread-db.c 13 May 2011 22:28:30 -0000
@@ -698,10 +698,50 @@ try_thread_db_load (const char *library)
return 0;
}

+/* Handle $sdir in libthread-db-search-path.
+ Look for libthread_db in the system dirs, or wherever a plain
+ dlopen(file_without_path) will look.
+ The result is true for success. */
+
static int
-thread_db_load_search (void)
+try_thread_db_load_from_sdir (void)
+{
+ return try_thread_db_load (LIBTHREAD_DB_SO);
+}
+
+/* Try to load libthread_db from directory DIR of length DIR_LEN.
+ The result is true for success. */
+
+static int
+try_thread_db_load_from_dir (const char *dir, size_t dir_len)
{
char path[PATH_MAX];
+
+ if (dir_len + 1 + strlen (LIBTHREAD_DB_SO) + 1 > sizeof (path))
+ {
+ char *cp = xmalloc (dir_len + 1);
+
+ memcpy (cp, dir, dir_len);
+ cp[dir_len] = '\0';
+ warning (_("libthread-db-search-path component too long,"
+ " ignored: %s."), cp);
+ free (cp);
+ return 0;
+ }
+
+ memcpy (path, dir, dir_len);
+ path[dir_len] = '/';
+ strcpy (path + dir_len + 1, LIBTHREAD_DB_SO);
+ return try_thread_db_load (path);
+}
+
+/* Search libthread_db_search_path for libthread_db which "agrees"
+ to work on current inferior.
+ The result is true for success. */
+
+static int
+thread_db_load_search (void)
+{
const char *search_path;
int rc = 0;

@@ -712,49 +752,45 @@ thread_db_load_search (void)
while (*search_path)
{
const char *end = strchr (search_path, ':');
+ const char *this_dir = search_path;
+ size_t this_dir_len;
+
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);
- free (cp);
- search_path += len + 1;
- continue;
- }
- memcpy (path, search_path, len);
- path[len] = '\0';
- search_path += len + 1;
+ this_dir_len = end - search_path;
+ search_path += this_dir_len + 1;
}
else
{
- size_t len = strlen (search_path);
+ this_dir_len = strlen (this_dir);
+ search_path += this_dir_len;
+ }

- if (len + 1 + strlen (LIBTHREAD_DB_SO) + 1 > sizeof (path))
+ if (this_dir_len == sizeof ("$pdir") - 1
+ && strncmp (this_dir, "$pdir", this_dir_len) == 0)
+ {
+ /* We don't maintain a list of loaded libraries so we don't know
+ where libpthread lives. We *could* fetch the info, but we don't
+ do that yet. Ignore it. */
+ }
+ else if (this_dir_len == sizeof ("$sdir") - 1
+ && strncmp (this_dir, "$sdir", this_dir_len) == 0)
+ {
+ if (try_thread_db_load_from_sdir ())
{
- warning ("libthread_db_search_path component too long,"
- " ignored: %s.", search_path);
+ rc = 1;
break;
}
- memcpy (path, search_path, len + 1);
- search_path += len;
}
- strcat (path, "/");
- strcat (path, LIBTHREAD_DB_SO);
- if (debug_threads)
- fprintf (stderr, "thread_db_load_search trying %s\n", path);
- if (try_thread_db_load (path))
+ else
{
- rc = 1;
- break;
+ if (try_thread_db_load_from_dir (this_dir, this_dir_len))
+ {
+ rc = 1;
+ break;
+ }
}
}
- if (rc == 0)
- rc = try_thread_db_load (LIBTHREAD_DB_SO);

if (debug_threads)
fprintf (stderr, "thread_db_load_search returning %d\n", rc);