On Mon, Apr 20, 2009 at 6:17 AM, Daniel Jacobowitz <drow@false.org> wrote:

> I have just a few minor comments on this.  Overall, I am heartily
> in favor of the idea - I have carried a less elegant version of this
> feature in the Debian GDB packages for two years or so.

Could you tell a bit more about this? I thought out set up with
multiple incompatible (WRT libthread_db) libc versions was more an
exception than the rule.

> On Fri, Apr 17, 2009 at 12:13:08PM -0700, Paul Pluzhnikov wrote:
>> +#ifndef LIBTHREAD_DB_SO
>> +#define LIBTHREAD_DB_SO "libthread_db.so.1"
>> +#endif
>> +
>> +#ifndef LIBTHREAD_DB_SEARCH_PATH
>> +#define LIBTHREAD_DB_SEARCH_PATH ""
>> +#endif
>
> Do we intend to let anything override these?  Otherwise IMHO the
> ifndef just confuses the issue.

It might be reasonable to override these via CFLAGS. In particular,
we intend to override the LIBTHREAD_DB_SEARCH_PATH with an appropriate
local default. Of course we could just as easily patch the source.

> Space before paren :-)

Sorry about that. Fixed everywhere.

>> +  if (!msym)
>> +    msym = lookup_minimal_symbol ("__pthread_threads_events", NULL, NULL);
>
> Why is this symbol lookup necessary?  Is it accomplishing the same
> thing that searching for a shared library matching "libpthread[-.]"
> would?

The symbol lookup works for statically linked executables (which we
also have and would like to support). Searching for libpthread solib
doesn't.

On Sat, Apr 18, 2009 at 10:00 AM, Eli Zaretskii <eliz@gnu.org> wrote:

>> +Setting @code{libthread-db-search-path} is currently implemented
>> +only for @sc{gnu}/Linux targets.
>
> In general, mentioning specific platforms in the manual is a
> maintenance nightmare, because we must remember to make changes in the
> manual to track the support of the feature on other platforms.  It is
> much better to say that this feature is available "only on some
> platforms."

Done.

>  That's assuming that on other platforms, using the
> command will cause a suitable error message; does it?

Yes: since the add_setshow... is not done on other platforms, you'll
get the same error as if you tried to set unknown variable (which is
actually quite confusing!):

   ./gdb -q -nx /bin/date
  (no debugging symbols found)
  (gdb) set libthread-db-search-path
  No symbol table is loaded.  Use the "file" command.
  (gdb)

> Btw, what are the Linux-specific aspects that this feature depends on?
> Why can't we implement it on Solaris as well?

This could quite easily be implemented on Solaris as well, although I
expect that there is significantly smaller number of users who need
this on Solaris. OTOH, anyone using glibc on Solaris will likely want
it (if he debugs both glibc and native-libc binaries).


Attached is a revised patch that fixes all the comments so far.

Thanks,
-- 
Paul Pluzhnikov

2009-04-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb_thread_db.h (LIBTHREAD_DB_SEARCH_PATH): New define.
	(LIBTHREAD_DB_SO): Moved from linux-thread-db.c

	* linux-thread-db.c (libthread_db_search_path): New setting.
	(thread_db_handle): New variable (replaces using_thread_db).
	(try_thread_db_load_1): New function.
	(try_thread_db_load, thread_db_load_search): Likewise.
	(dladdr_to_soname): Likewise.
	(thread_db_load): Iterate over possibly multiple libthread_db's.
	(check_for_thread_db): Attempt to load new libthread_db.
	(thread_db_detach, thread_db_wait): Unload libthread_db.
	(thread_db_mourn_inferior): Likewise.
	(_initialize_thread_db): Add new libthread-db-search-path option.
	Defer loading of libthread_db to check_for_thread_db.

doc/ChangeLog

2009-04-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

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