* Re: [PATCH 1/4] libthread_db initialization changes related to upcoming glibc-2.34
@ 2021-06-11 16:35 Carlos O'Donell
0 siblings, 0 replies; 4+ messages in thread
From: Carlos O'Donell @ 2021-06-11 16:35 UTC (permalink / raw)
To: kevinb, gdb-patches
This looks good to me.
I've tested on x86_64 Fedora Rawhide.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Tested-by: Carlos O'Donell <carlos@redhat.com>
> This commit makes some adjustments to accomodate the upcoming
> glibc-2.34 release. Beginning with glibc-2.34, functionality formerly
> contained in libpthread has been moved to libc. For the time being,
> libpthread.so still exists in the file system, but it won't show up in
> ldd output and therefore won't be able to trigger initialization of
> libthread_db related code. E.g...
>
> Fedora 34 / glibc-2.33.9000:
>
> [kev@f34-2 gdb]$ ldd testsuite/outputs/gdb.threads/tls/tls
> linux-vdso.so.1 (0x00007ffcf94fa000)
> libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007ff0ba9af000)
> libm.so.6 => /lib64/libm.so.6 (0x00007ff0ba8d4000)
> libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007ff0ba8b9000)
> libc.so.6 => /lib64/libc.so.6 (0x00007ff0ba6c6000)
> /lib64/ld-linux-x86-64.so.2 (0x00007ff0babf0000)
>
> Fedora 34 / glibc-2.33:
>
> [kev@f34-1 gdb]$ ldd testsuite/outputs/gdb.threads/tls/tls
> linux-vdso.so.1 (0x00007fff32dc0000)
> libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f815f6de000)
> libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007f815f4bf000)
> libm.so.6 => /lib64/libm.so.6 (0x00007f815f37b000)
> libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f815f360000)
> libc.so.6 => /lib64/libc.so.6 (0x00007f815f191000)
> /lib64/ld-linux-x86-64.so.2 (0x00007f815f721000)
>
> Note that libpthread is missing from the ldd output for the
> glibc-2.33.9000 machine.
>
> This means that (unless we happen to think of some entirely different
> mechanism), we'll now need to potentially match "libc" in addition to
> "libpthread" as libraries which might be thread libraries. This
> accounts for the change made in solib.c. Note that the new code
> attempts to match "/libc." via strstr(). That trailing dot (".")
> avoids inadvertently matching libraries such as libcrypt (and
> all the other many libraries which begin with "libc").
This is correct and the solution looks good.
You do need to trigger libthread_db loading for libc.so.6 now.
> To avoid attempts to load libthread_db when encountering older
> versions of libc, we now attempt to find "pthread_create" (which is a
> symbol that we'd expect to be in any pthread library) in the
> associated objfile. This accounts for the changes in
> linux-thread-db.c.
That is a good heuristic.
> I think that other small adjustments will need to be made elsewhere
> too. I've been working through regressions on my glibc-2.33.9000
> machine; I've fixed some fairly "obvious" changes in the testsuite
> (which are in other commits). For the rest, it's not yet clear to me
> whether the handful of remaining failures represent a problem in glibc
> or gdb. I'm still investigating, however, I'll note that these are
> problems that I only see on my glibc-2.33.9000 machine.
>
> gdb/ChangeLog:
>
> * solib.c (libpthread_name_p): Match "libc" in addition
> to "libpthread".
> * linux-thread-db.c (libpthread_objfile_p): New function.
> (libpthread_name_p): Adjust preexisting callers to use
> libpthread_objfile_p().
> ---
> gdb/linux-thread-db.c | 24 +++++++++++++++++++++---
> gdb/solib.c | 10 ++++++++--
> 2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
> index d1e8c22ac96..5b4e5a8654f 100644
> --- a/gdb/linux-thread-db.c
> +++ b/gdb/linux-thread-db.c
> @@ -799,6 +799,24 @@ check_thread_db (struct thread_db_info *info, bool log_progress)
> return test_passed;
> }
>
> +/* Predicate which tests whether objfile OBJ refers to the library
> + containing pthread related symbols. Historically, this library has
> + been named in such a way that looking for "libpthread" in the name
> + was sufficient to identify it. As of glibc-2.34, the C library
> + (libc) contains the thread library symbols. Therefore we check
> + that the name matches a possible thread library, but we also check
> + that it contains at least one of the symbols (pthread_create) that
> + we'd expect to find in the thread library. */
> +
> +static bool
> +libpthread_objfile_p (objfile *obj)
> +{
> + return (libpthread_name_p (objfile_name (obj))
> + && lookup_minimal_symbol ("pthread_create",
> + NULL,
> + obj).minsym != NULL);
> +}
OK. If the loaded file matches the name list then do a global symbol
search to see if pthread_create is present and if it is then we will
indirectly trigger an attempt to load libthread_db.
> +
> /* Attempt to initialize dlopen()ed libthread_db, described by INFO.
> Return true on success.
> Failure could happen if libthread_db does not have symbols we expect,
> @@ -1072,7 +1090,7 @@ try_thread_db_load_from_pdir (const char *subdir)
> return false;
>
> for (objfile *obj : current_program_space->objfiles ())
> - if (libpthread_name_p (objfile_name (obj)))
> + if (libpthread_objfile_p (obj))
OK.
> {
> if (try_thread_db_load_from_pdir_1 (obj, subdir))
> return true;
> @@ -1181,7 +1199,7 @@ static bool
> has_libpthread (void)
> {
> for (objfile *obj : current_program_space->objfiles ())
> - if (libpthread_name_p (objfile_name (obj)))
> + if (libpthread_objfile_p (obj))
OK.
> return true;
>
> return false;
> @@ -1286,7 +1304,7 @@ thread_db_new_objfile (struct objfile *objfile)
> of the list of shared libraries to load, and in an app of several
> thousand shared libraries, this can otherwise be painful. */
> && ((objfile->flags & OBJF_MAINLINE) != 0
> - || libpthread_name_p (objfile_name (objfile))))
> + || libpthread_objfile_p (objfile)))
OK.
> check_for_thread_db ();
> }
>
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 2df52118143..88b201ff6a0 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -900,12 +900,18 @@ Do you need \"set solib-search-path\" or \"set sysroot\"?"),
>
> Uses a fairly simplistic heuristic approach where we check
> the file name against "/libpthread". This can lead to false
> - positives, but this should be good enough in practice. */
> + positives, but this should be good enough in practice.
> +
> + In glibc-2.34 (and later), functions formerly residing
> + in libpthread have been moved to libc, so "/libc." needs
> + to be checked too. (Matching the "." will avoid matching
> + libraries such as libcrypt.) */
>
> bool
> libpthread_name_p (const char *name)
> {
> - return (strstr (name, "/libpthread") != NULL);
> + return (strstr (name, "/libpthread") != NULL
> + || strstr (name, "/libc.") != NULL );
OK. So you look for both libpthread.* and libc.*.
> }
>
> /* Return non-zero if SO is the libpthread shared library. */
> --
> 2.31.1
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 0/4] libthread_db initialization changes related to upcoming glibc-2.34
@ 2021-06-10 17:26 Kevin Buettner
2021-06-10 17:26 ` [PATCH 1/4] " Kevin Buettner
0 siblings, 1 reply; 4+ messages in thread
From: Kevin Buettner @ 2021-06-10 17:26 UTC (permalink / raw)
To: gdb-patches
This patch series contains changes necessary for GDB to be able to
debug threaded programs using glibc-2.34.
The actual change to GDB sources are contained in the first patch; the
remainder deal with several testing issues that I ran into.
Kevin Buettner (4):
libthread_db initialization changes related to upcoming glibc-2.34
testsuite/glib-2.34: Match/consume optional libthread_db related
output
print-symbol-loading.exp: Allow libc symbols to be already loaded
mi-sym-info.exp: Increase timeout for 114-symbol-info-functions
gdb/linux-thread-db.c | 24 +++++++-
gdb/solib.c | 10 +++-
.../gdb.base/execl-update-breakpoints.exp | 1 +
.../gdb.base/fork-print-inferior-events.exp | 3 +-
.../gdb.base/print-symbol-loading.exp | 15 ++++-
gdb/testsuite/gdb.mi/mi-sym-info.exp | 56 ++++++++++---------
6 files changed, 73 insertions(+), 36 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/4] libthread_db initialization changes related to upcoming glibc-2.34
2021-06-10 17:26 [PATCH 0/4] " Kevin Buettner
@ 2021-06-10 17:26 ` Kevin Buettner
2021-06-11 19:10 ` Simon Marchi
0 siblings, 1 reply; 4+ messages in thread
From: Kevin Buettner @ 2021-06-10 17:26 UTC (permalink / raw)
To: gdb-patches
This commit makes some adjustments to accomodate the upcoming
glibc-2.34 release. Beginning with glibc-2.34, functionality formerly
contained in libpthread has been moved to libc. For the time being,
libpthread.so still exists in the file system, but it won't show up in
ldd output and therefore won't be able to trigger initialization of
libthread_db related code. E.g...
Fedora 34 / glibc-2.33.9000:
[kev@f34-2 gdb]$ ldd testsuite/outputs/gdb.threads/tls/tls
linux-vdso.so.1 (0x00007ffcf94fa000)
libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007ff0ba9af000)
libm.so.6 => /lib64/libm.so.6 (0x00007ff0ba8d4000)
libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007ff0ba8b9000)
libc.so.6 => /lib64/libc.so.6 (0x00007ff0ba6c6000)
/lib64/ld-linux-x86-64.so.2 (0x00007ff0babf0000)
Fedora 34 / glibc-2.33:
[kev@f34-1 gdb]$ ldd testsuite/outputs/gdb.threads/tls/tls
linux-vdso.so.1 (0x00007fff32dc0000)
libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f815f6de000)
libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007f815f4bf000)
libm.so.6 => /lib64/libm.so.6 (0x00007f815f37b000)
libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f815f360000)
libc.so.6 => /lib64/libc.so.6 (0x00007f815f191000)
/lib64/ld-linux-x86-64.so.2 (0x00007f815f721000)
Note that libpthread is missing from the ldd output for the
glibc-2.33.9000 machine.
This means that (unless we happen to think of some entirely different
mechanism), we'll now need to potentially match "libc" in addition to
"libpthread" as libraries which might be thread libraries. This
accounts for the change made in solib.c. Note that the new code
attempts to match "/libc." via strstr(). That trailing dot (".")
avoids inadvertently matching libraries such as libcrypt (and
all the other many libraries which begin with "libc").
To avoid attempts to load libthread_db when encountering older
versions of libc, we now attempt to find "pthread_create" (which is a
symbol that we'd expect to be in any pthread library) in the
associated objfile. This accounts for the changes in
linux-thread-db.c.
I think that other small adjustments will need to be made elsewhere
too. I've been working through regressions on my glibc-2.33.9000
machine; I've fixed some fairly "obvious" changes in the testsuite
(which are in other commits). For the rest, it's not yet clear to me
whether the handful of remaining failures represent a problem in glibc
or gdb. I'm still investigating, however, I'll note that these are
problems that I only see on my glibc-2.33.9000 machine.
gdb/ChangeLog:
* solib.c (libpthread_name_p): Match "libc" in addition
to "libpthread".
* linux-thread-db.c (libpthread_objfile_p): New function.
(libpthread_name_p): Adjust preexisting callers to use
libpthread_objfile_p().
---
gdb/linux-thread-db.c | 24 +++++++++++++++++++++---
gdb/solib.c | 10 ++++++++--
2 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index d1e8c22ac96..5b4e5a8654f 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -799,6 +799,24 @@ check_thread_db (struct thread_db_info *info, bool log_progress)
return test_passed;
}
+/* Predicate which tests whether objfile OBJ refers to the library
+ containing pthread related symbols. Historically, this library has
+ been named in such a way that looking for "libpthread" in the name
+ was sufficient to identify it. As of glibc-2.34, the C library
+ (libc) contains the thread library symbols. Therefore we check
+ that the name matches a possible thread library, but we also check
+ that it contains at least one of the symbols (pthread_create) that
+ we'd expect to find in the thread library. */
+
+static bool
+libpthread_objfile_p (objfile *obj)
+{
+ return (libpthread_name_p (objfile_name (obj))
+ && lookup_minimal_symbol ("pthread_create",
+ NULL,
+ obj).minsym != NULL);
+}
+
/* Attempt to initialize dlopen()ed libthread_db, described by INFO.
Return true on success.
Failure could happen if libthread_db does not have symbols we expect,
@@ -1072,7 +1090,7 @@ try_thread_db_load_from_pdir (const char *subdir)
return false;
for (objfile *obj : current_program_space->objfiles ())
- if (libpthread_name_p (objfile_name (obj)))
+ if (libpthread_objfile_p (obj))
{
if (try_thread_db_load_from_pdir_1 (obj, subdir))
return true;
@@ -1181,7 +1199,7 @@ static bool
has_libpthread (void)
{
for (objfile *obj : current_program_space->objfiles ())
- if (libpthread_name_p (objfile_name (obj)))
+ if (libpthread_objfile_p (obj))
return true;
return false;
@@ -1286,7 +1304,7 @@ thread_db_new_objfile (struct objfile *objfile)
of the list of shared libraries to load, and in an app of several
thousand shared libraries, this can otherwise be painful. */
&& ((objfile->flags & OBJF_MAINLINE) != 0
- || libpthread_name_p (objfile_name (objfile))))
+ || libpthread_objfile_p (objfile)))
check_for_thread_db ();
}
diff --git a/gdb/solib.c b/gdb/solib.c
index 2df52118143..88b201ff6a0 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -900,12 +900,18 @@ Do you need \"set solib-search-path\" or \"set sysroot\"?"),
Uses a fairly simplistic heuristic approach where we check
the file name against "/libpthread". This can lead to false
- positives, but this should be good enough in practice. */
+ positives, but this should be good enough in practice.
+
+ In glibc-2.34 (and later), functions formerly residing
+ in libpthread have been moved to libc, so "/libc." needs
+ to be checked too. (Matching the "." will avoid matching
+ libraries such as libcrypt.) */
bool
libpthread_name_p (const char *name)
{
- return (strstr (name, "/libpthread") != NULL);
+ return (strstr (name, "/libpthread") != NULL
+ || strstr (name, "/libc.") != NULL );
}
/* Return non-zero if SO is the libpthread shared library. */
--
2.31.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4] libthread_db initialization changes related to upcoming glibc-2.34
2021-06-10 17:26 ` [PATCH 1/4] " Kevin Buettner
@ 2021-06-11 19:10 ` Simon Marchi
2021-06-11 21:24 ` Kevin Buettner
0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi @ 2021-06-11 19:10 UTC (permalink / raw)
To: Kevin Buettner, gdb-patches
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 2df52118143..88b201ff6a0 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -900,12 +900,18 @@ Do you need \"set solib-search-path\" or \"set sysroot\"?"),
>
> Uses a fairly simplistic heuristic approach where we check
> the file name against "/libpthread". This can lead to false
> - positives, but this should be good enough in practice. */
> + positives, but this should be good enough in practice.
> +
> + In glibc-2.34 (and later), functions formerly residing
> + in libpthread have been moved to libc, so "/libc." needs
> + to be checked too. (Matching the "." will avoid matching
> + libraries such as libcrypt.) */
I'd prefer that you use "As of glibc-2.34...", as you've used
elsewhere. Saying "In glibc-2.34 and later" assumes that it will be
like this forever, which we don't know.
Otherwise, LGTM.
Simon
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/4] libthread_db initialization changes related to upcoming glibc-2.34
2021-06-11 19:10 ` Simon Marchi
@ 2021-06-11 21:24 ` Kevin Buettner
0 siblings, 0 replies; 4+ messages in thread
From: Kevin Buettner @ 2021-06-11 21:24 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On Fri, 11 Jun 2021 15:10:54 -0400
Simon Marchi <simon.marchi@polymtl.ca> wrote:
> > diff --git a/gdb/solib.c b/gdb/solib.c
> > index 2df52118143..88b201ff6a0 100644
> > --- a/gdb/solib.c
> > +++ b/gdb/solib.c
> > @@ -900,12 +900,18 @@ Do you need \"set solib-search-path\" or \"set sysroot\"?"),
> >
> > Uses a fairly simplistic heuristic approach where we check
> > the file name against "/libpthread". This can lead to false
> > - positives, but this should be good enough in practice. */
> > + positives, but this should be good enough in practice.
> > +
> > + In glibc-2.34 (and later), functions formerly residing
> > + in libpthread have been moved to libc, so "/libc." needs
> > + to be checked too. (Matching the "." will avoid matching
> > + libraries such as libcrypt.) */
>
> I'd prefer that you use "As of glibc-2.34...", as you've used
> elsewhere. Saying "In glibc-2.34 and later" assumes that it will be
> like this forever, which we don't know.
>
> Otherwise, LGTM.
I've revised that comment as requested. That portion of the comment
now looks like this:
As of glibc-2.34, functions formerly residing in libpthread have
been moved to libc, so "/libc." needs to be checked too. (Matching
the "." will avoid matching libraries such as libcrypt.) */
Kevin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-11 21:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11 16:35 [PATCH 1/4] libthread_db initialization changes related to upcoming glibc-2.34 Carlos O'Donell
-- strict thread matches above, loose matches on Subject: below --
2021-06-10 17:26 [PATCH 0/4] " Kevin Buettner
2021-06-10 17:26 ` [PATCH 1/4] " Kevin Buettner
2021-06-11 19:10 ` Simon Marchi
2021-06-11 21:24 ` Kevin Buettner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).