| Message ID | 87sg3qnrz3.fsf@oldenburg.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | nptl_db: different libpthread/ld.so load orders (bug 27744) | expand |
On 2021-04-16 11:56 a.m., Florian Weimer wrote: > libthread_db is loaded once GDB encounters libpthread, and at this > point, ld.so may not have been loaded yet. Hi Florian, Can this state (libpthread loaded, ld.so not loaded) really happen during the normal lifetime of a process? My understanding is that this state happens when attaching only because GDB reads the shared libraries from the process in an undefined order, so libpthread may be discovered before ld.so. So we present to libthread_db a state that doesn't really make sense. Still, that quick fix probably helps in the attach case, since it avoids requiring symbols outside of libpthread, but I would just like to understand. Simon
* Simon Marchi: > On 2021-04-16 11:56 a.m., Florian Weimer wrote: >> libthread_db is loaded once GDB encounters libpthread, and at this >> point, ld.so may not have been loaded yet. > Can this state (libpthread loaded, ld.so not loaded) really happen > during the normal lifetime of a process? My understanding is that this > state happens when attaching only because GDB reads the shared libraries > from the process in an undefined order, so libpthread may be discovered > before ld.so. So we present to libthread_db a state that doesn't really > make sense. I think it depends on the order of the link map list, I think, and there ld.so ends up being sorted last. If libpthread gets merged into libc for glibc 2.34, I will try to make this work with old GDB by adding a fake libpthread.so.0 at the end of the link map list, after ld.so. But none of this code exists yet, so it's impossible to know if it actually will work as expected. Thanks, Florian
On 16/04/21 17:07, Simon Marchi wrote: > > Can this state (libpthread loaded, ld.so not loaded) really happen > during the normal lifetime of a process? My understanding is that this > state happens when attaching only because GDB reads the shared libraries > from the process in an undefined order, so libpthread may be discovered > before ld.so. So we present to libthread_db a state that doesn't really > make sense. I don't think it's random from GDB's perspective -- GDB reads the shared library list of out of the link map, so it should be reading them in link map order. IIRC, the order which libraries are loaded by GDB hasn't changed. The issue is that until recently (before glibc 1daccf403b1b), the stacks lists lived in libpthread (stack_used/__stack_user), so the fact that GDB loaded libthread_db.so before ld.so's symbols were loaded didn't make a difference. Now they were moved to ld.so, so libthread_db.so can't find them until GDB reads the ld.so symbols. Is this assessment correct? Pedro Alves
* Pedro Alves: > IIRC, the order which libraries are loaded by GDB hasn't changed. The > issue is that until recently (before glibc 1daccf403b1b), the stacks > lists lived in libpthread (stack_used/__stack_user), so the fact that > GDB loaded libthread_db.so before ld.so's symbols were loaded didn't > make a difference. Now they were moved to ld.so, so libthread_db.so > can't find them until GDB reads the ld.so symbols. Is this assessment > correct? Yes, I believe this is what happens. I assume that ldd shows objects in link map order, the it looks like this: $ ldd /usr/bin/python3 linux-vdso.so.1 (0x00007fff1f8d5000) libpython3.9.so.1.0 => /lib64/libpython3.9.so.1.0 (0x00007f3fd7782000) libc.so.6 => /lib64/libc.so.6 (0x00007f3fd75b7000) libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f3fd7595000) libdl.so.2 => /lib64/libdl.so.2 (0x00007f3fd758e000) libutil.so.1 => /lib64/libutil.so.1 (0x00007f3fd7589000) libm.so.6 => /lib64/libm.so.6 (0x00007f3fd7443000) /lib64/ld-linux-x86-64.so.2 (0x00007f3fd7af7000) Thanks, Florian
On 16/04/21 17:28, Florian Weimer wrote: > * Pedro Alves: > >> IIRC, the order which libraries are loaded by GDB hasn't changed. The >> issue is that until recently (before glibc 1daccf403b1b), the stacks >> lists lived in libpthread (stack_used/__stack_user), so the fact that >> GDB loaded libthread_db.so before ld.so's symbols were loaded didn't >> make a difference. Now they were moved to ld.so, so libthread_db.so >> can't find them until GDB reads the ld.so symbols. Is this assessment >> correct? > > Yes, I believe this is what happens. > OK, I believe what is confusing in your commit log was the reference to two different kinds of "loaded": "libthread_db is loaded once GDB encounters libpthread, and at this point, ld.so may not have been loaded yet. " The first loaded is about GDB dlopening libthread_db.so. The second loaded refers to reading symbols -- ld.so has been loaded by the inferior already at that point. It would be clearer as: "libthread_db is loaded once GDB encounters libpthread, and at this point, ld.so's symbols may not have been read by GDB yet. " If I understood that correctly, then the following sentence is also a bit confusing: "As a result, _rtld_global cannot be accessed by regular means from libthread_db." Because that sounds to me like you were perhaps talking about some magic means to reference globals, some magic relocations, or some other magic voodoo only understood by glibc experts. Pedro Alves > I assume that ldd shows objects in link map order, the it looks like > this: > > $ ldd /usr/bin/python3 > linux-vdso.so.1 (0x00007fff1f8d5000) > libpython3.9.so.1.0 => /lib64/libpython3.9.so.1.0 (0x00007f3fd7782000) > libc.so.6 => /lib64/libc.so.6 (0x00007f3fd75b7000) > libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f3fd7595000) > libdl.so.2 => /lib64/libdl.so.2 (0x00007f3fd758e000) > libutil.so.1 => /lib64/libutil.so.1 (0x00007f3fd7589000) > libm.so.6 => /lib64/libm.so.6 (0x00007f3fd7443000) > /lib64/ld-linux-x86-64.so.2 (0x00007f3fd7af7000) > > Thanks, > Florian >
* Pedro Alves: > On 16/04/21 17:28, Florian Weimer wrote: >> * Pedro Alves: >> >>> IIRC, the order which libraries are loaded by GDB hasn't changed. The >>> issue is that until recently (before glibc 1daccf403b1b), the stacks >>> lists lived in libpthread (stack_used/__stack_user), so the fact that >>> GDB loaded libthread_db.so before ld.so's symbols were loaded didn't >>> make a difference. Now they were moved to ld.so, so libthread_db.so >>> can't find them until GDB reads the ld.so symbols. Is this assessment >>> correct? >> >> Yes, I believe this is what happens. >> > > OK, I believe what is confusing in your commit log was the reference to > two different kinds of "loaded": > > "libthread_db is loaded once GDB encounters libpthread, and at this > point, ld.so may not have been loaded yet. " > > The first loaded is about GDB dlopening libthread_db.so. The second loaded > refers to reading symbols -- ld.so has been loaded by the inferior already > at that point. > > It would be clearer as: > > "libthread_db is loaded once GDB encounters libpthread, and at this > point, ld.so's symbols may not have been read by GDB yet. " I'm going to go with: “ libthread_db is loaded once GDB encounters libpthread, and at this point, ld.so may not have been processed by GDB yet. ” > If I understood that correctly, then the following sentence is also a > bit confusing: > > "As a result, _rtld_global cannot be accessed by regular means from > libthread_db." > > Because that sounds to me like you were perhaps talking about some > magic means to reference globals, some magic relocations, or some > other magic voodoo only understood by glibc experts. We use the magic that GDB provides to us (ps_pglobal_lookup, I think). I thought that this was understood by GDB experts only. 8-) Thanks, Florian
On 16/04/21 17:43, Florian Weimer wrote: > * Pedro Alves: > >> On 16/04/21 17:28, Florian Weimer wrote: >>> * Pedro Alves: >>> >>>> IIRC, the order which libraries are loaded by GDB hasn't changed. The >>>> issue is that until recently (before glibc 1daccf403b1b), the stacks >>>> lists lived in libpthread (stack_used/__stack_user), so the fact that >>>> GDB loaded libthread_db.so before ld.so's symbols were loaded didn't >>>> make a difference. Now they were moved to ld.so, so libthread_db.so >>>> can't find them until GDB reads the ld.so symbols. Is this assessment >>>> correct? >>> >>> Yes, I believe this is what happens. >>> >> >> OK, I believe what is confusing in your commit log was the reference to >> two different kinds of "loaded": >> >> "libthread_db is loaded once GDB encounters libpthread, and at this >> point, ld.so may not have been loaded yet. " >> >> The first loaded is about GDB dlopening libthread_db.so. The second loaded >> refers to reading symbols -- ld.so has been loaded by the inferior already >> at that point. >> >> It would be clearer as: >> >> "libthread_db is loaded once GDB encounters libpthread, and at this >> point, ld.so's symbols may not have been read by GDB yet. " > > I'm going to go with: > > “ > libthread_db is loaded once GDB encounters libpthread, and at this > point, ld.so may not have been processed by GDB yet. > ” Sounds good. > >> If I understood that correctly, then the following sentence is also a >> bit confusing: >> >> "As a result, _rtld_global cannot be accessed by regular means from >> libthread_db." >> >> Because that sounds to me like you were perhaps talking about some >> magic means to reference globals, some magic relocations, or some >> other magic voodoo only understood by glibc experts. > > We use the magic that GDB provides to us (ps_pglobal_lookup, I think). > I thought that this was understood by GDB experts only. 8-) LOL I skimmed the patch, and FWIW, it LGTM. Just spotted a couple typos: > +/* This test runs GDB against a forked copy of itself, to check > + whether libthreaddb can be loaded, and that access to thread-local libthreaddb -> libthread_db > +/* This function implements the subprocess un der test. It creates a "un der" -> "under" Thanks, Pedro Alves
On 2021-04-16 12:47 p.m., Pedro Alves wrote:> On 16/04/21 17:43, Florian Weimer wrote: >> * Pedro Alves: >> >>> On 16/04/21 17:28, Florian Weimer wrote: >>>> * Pedro Alves: >>>> >>>>> IIRC, the order which libraries are loaded by GDB hasn't changed. The >>>>> issue is that until recently (before glibc 1daccf403b1b), the stacks >>>>> lists lived in libpthread (stack_used/__stack_user), so the fact that >>>>> GDB loaded libthread_db.so before ld.so's symbols were loaded didn't >>>>> make a difference. Now they were moved to ld.so, so libthread_db.so >>>>> can't find them until GDB reads the ld.so symbols. Is this assessment >>>>> correct? >>>> >>>> Yes, I believe this is what happens. >>>> >>> >>> OK, I believe what is confusing in your commit log was the reference to >>> two different kinds of "loaded": >>> >>> "libthread_db is loaded once GDB encounters libpthread, and at this >>> point, ld.so may not have been loaded yet. " >>> >>> The first loaded is about GDB dlopening libthread_db.so. The second loaded >>> refers to reading symbols -- ld.so has been loaded by the inferior already >>> at that point. >>> >>> It would be clearer as: >>> >>> "libthread_db is loaded once GDB encounters libpthread, and at this >>> point, ld.so's symbols may not have been read by GDB yet. " >> >> I'm going to go with: >> >> “ >> libthread_db is loaded once GDB encounters libpthread, and at this >> point, ld.so may not have been processed by GDB yet. >> ” > > Sounds good. > >> >>> If I understood that correctly, then the following sentence is also a >>> bit confusing: >>> >>> "As a result, _rtld_global cannot be accessed by regular means from >>> libthread_db." >>> >>> Because that sounds to me like you were perhaps talking about some >>> magic means to reference globals, some magic relocations, or some >>> other magic voodoo only understood by glibc experts. >> >> We use the magic that GDB provides to us (ps_pglobal_lookup, I think). >> I thought that this was understood by GDB experts only. 8-) > > LOL > > I skimmed the patch, and FWIW, it LGTM. Just spotted a couple typos: > >> +/* This test runs GDB against a forked copy of itself, to check >> + whether libthreaddb can be loaded, and that access to thread-local > > libthreaddb -> libthread_db > >> +/* This function implements the subprocess un der test. It creates a > > "un der" -> "under" > > Thanks, > Pedro Alves > Do we need / want to fix GDB if this goes in glibc then? I have an updated version of my patch here [1] sitting here, that makes it work with GDBserver as well, with the "broken" glibc 2.33. I'm wondering if I should post it or not. Even without this bug, my patch can be beneficial from an efficiency point of view, since it delays sending a qSymbol to the remote side until all shared libraries are known. But then it would be a completely different rationale, I would have to word the commit message in terms of "make things more efficient" rather than "fix a bug while attaching". Simon [1] https://sourceware.org/pipermail/gdb-patches/2021-April/177477.html
On 16/04/21 17:53, Simon Marchi wrote: > Do we need / want to fix GDB if this goes in glibc then? I think so. We may need to discuss more the "needs_setup" hack, but we can do that in that thread. Given that libpthread.so is going away in the future, we should be thinking about addressing that as well. Does your patch fix that as side effect? If not, GDB should be keying the loading of libthread_db.so on something else. Making GDB try to load libthread_db in reaction to processing ld.so instead libthread_db.so would fix that, I think. And, it would fix this ordering problem at hand as well. So if we do that, maybe we don't need the other changes. > I have an > updated version of my patch here [1] sitting here, that makes it work > with GDBserver as well, with the "broken" glibc 2.33. I'm wondering if > I should post it or not. > > Even without this bug, my patch can be beneficial from an efficiency > point of view, since it delays sending a qSymbol to the remote side > until all shared libraries are known. Right, I like batching for this reason. > But then it would be a completely > different rationale, I would have to word the commit message in terms of > "make things more efficient" rather than "fix a bug while attaching". >
* Pedro Alves: > If not, GDB should be keying the loading of libthread_db.so on > something else. Making GDB try to load libthread_db in reaction to > processing ld.so instead libthread_db.so would fix that, I think. > And, it would fix this ordering problem at hand as well. So if we do > that, maybe we don't need the other changes. It may be a bit difficult to recognize ld.so though, given its non-portable soname. Thanks, Florian
* Pedro Alves: >> +/* This test runs GDB against a forked copy of itself, to check >> + whether libthreaddb can be loaded, and that access to thread-local > > libthreaddb -> libthread_db > >> +/* This function implements the subprocess un der test. It creates a > > "un der" -> "under" Thanks. Any suggestions what to do about the GDB assertion failure mentioned in the comment? I have tried to produce a small reproducer, but failed. Thanks, Florian
On 2021-04-16 1:18 p.m., Pedro Alves wrote: > On 16/04/21 17:53, Simon Marchi wrote: > >> Do we need / want to fix GDB if this goes in glibc then? > > I think so. We may need to discuss more the "needs_setup" hack, but we > can do that in that thread. In the new version I add a new inferior flag "in_initial_library_scan". It's perhaps not ideal but the result is better than what I had in v1. But yeah let's discuss that in that thread. > Given that libpthread.so is going away in the future, we should be thinking about > addressing that as well. Does your patch fix that as side effect? No, I think of that as a separate problem. > > If not, GDB should be keying the loading of libthread_db.so on something else. > Making GDB try to load libthread_db in reaction to processing ld.so instead > libthread_db.so would fix that, I think. And, it would fix this ordering problem at > hand as well. So if we do that, maybe we don't need the other changes. Is it possible to have a completely static executable that doesn't use ld.so but uses pthreads? I don't think that would work well for the "run" case, where ld-linux arrives before libpthreads.so (and before libc.so, for when pthreads is moved to libc.so). If we try to load libthread_db when ld-linux appears, the symbols provided by libpthreads (or libc) won't be found. We would have to key the loading on when both libpthread and ld-linux are there (or both libc and ld-linux). Another problem with the current state (but that would be fixed with Florian's patch I think) is that gdb has this setting "set auto-solib-add". If off, GDB won't load the symbols of shared libraries. _Except_ if the library if libpthread: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/solib.c;h=d32b2210005fdd6a5013cfd566620d6afeae9cf6;hb=HEAD#l968 But it means that if auto-solib-add is off, we won't load ld.so's symbols, and we fall back on the original problem, even if the ordering is right. Since Florian's patch makes libthread_db only access symbols of libpthread.so, I think that avoids this issue as well. But then when libpthreads is moved into libc, we'll have the problem that libthread_db simply won't get loaded... Simon
On 16/04/21 18:28, Florian Weimer wrote: > * Pedro Alves: > >>> +/* This test runs GDB against a forked copy of itself, to check >>> + whether libthreaddb can be loaded, and that access to thread-local >> >> libthreaddb -> libthread_db >> >>> +/* This function implements the subprocess un der test. It creates a >> >> "un der" -> "under" > > Thanks. Any suggestions what to do about the GDB assertion failure > mentioned in the comment? I have tried to produce a small reproducer, > but failed. Even without reducing, it is possible to make the program that triggers the assertion be a standalone program? Any reason you're using "add-symbol-file" in the first place, instead of "file"?
On 16/04/21 18:33, Simon Marchi wrote: > On 2021-04-16 1:18 p.m., Pedro Alves wrote: >> On 16/04/21 17:53, Simon Marchi wrote: >> >>> Do we need / want to fix GDB if this goes in glibc then? >> >> I think so. We may need to discuss more the "needs_setup" hack, but we >> can do that in that thread. > > In the new version I add a new inferior flag "in_initial_library_scan". > It's perhaps not ideal but the result is better than what I had in v1. > But yeah let's discuss that in that thread. > >> Given that libpthread.so is going away in the future, we should be thinking about >> addressing that as well. Does your patch fix that as side effect? > > No, I think of that as a separate problem. > >> >> If not, GDB should be keying the loading of libthread_db.so on something else. >> Making GDB try to load libthread_db in reaction to processing ld.so instead >> libthread_db.so would fix that, I think. And, it would fix this ordering problem at >> hand as well. So if we do that, maybe we don't need the other changes. > > Is it possible to have a completely static executable that doesn't use > ld.so but uses pthreads? Yes, see gdb.threads/staticthreads.exp. We handle that here, in linux-thread-db.c: /* Add ourselves to inferior_created event chain. This is needed to handle debugging statically linked programs where the new_objfile observer won't get called for libpthread. */ gdb::observers::inferior_created.attach (thread_db_inferior_created); Or do you mean, a static executable that then later on loads (or loads something that loads) libpthread.so, via dlopen? In that case, libpthread.so pulls in ld.so. > > I don't think that would work well for the "run" case, where ld-linux > arrives before libpthreads.so (and before libc.so, for when pthreads is > moved to libc.so). If we try to load libthread_db when ld-linux > appears, the symbols provided by libpthreads (or libc) won't be found. > > We would have to key the loading on when both libpthread and ld-linux > are there (or both libc and ld-linux). Yeah, I think this would work - try to load libthread_db.so when: #1 ld-linux is processed, and, #2 libpthread is processed _and_ iff ld-linux has been processed already Sounds like this should work for both run and attach, and for both current glibc and future glibc. > > Another problem with the current state (but that would be fixed with > Florian's patch I think) is that gdb has this setting "set > auto-solib-add". If off, GDB won't load the symbols of shared > libraries. _Except_ if the library if libpthread: > > https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/solib.c;h=d32b2210005fdd6a5013cfd566620d6afeae9cf6;hb=HEAD#l968 > > But it means that if auto-solib-add is off, we won't load ld.so's > symbols, and we fall back on the original problem, even if the ordering > is right. Since Florian's patch makes libthread_db only access symbols > of libpthread.so, I think that avoids this issue as well. > > But then when libpthreads is moved into libc, we'll have the problem > that libthread_db simply won't get loaded... Guess we should make GDB always read ld.so 's symbols as well, regardless of auto-solib-add.
* Simon Marchi: > Is it possible to have a completely static executable that doesn't use > ld.so but uses pthreads? Absolutely, the new nptl/tst-pthread-gdb-attach-static shows that TLS is already supported in GDB. (In v2 of the patch, the test will actually run …) Thanks, Florian
* Pedro Alves: > Even without reducing, it is possible to make the program that triggers the > assertion be a standalone program? git clone https://pagure.io/glibc/glibc-support cd glibc-support make Then drop the attached file into the tests subdirectory, and run: make LDFLAGS=-lpthread build/tst-pthread-gdb-attach This should reproduce the issue out side of the glibc tree. > Any reason you're using "add-symbol-file" in the first place, instead > of "file"? The main program is ld.so. Thanks, Florian /* Smoke testing GDB process attach with thread-local variable access. Copyright (C) 2021 Free Software Foundation, Inc. This file is part of the GNU C Library. The GNU C Library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either version 2.1 of the License, or (at your option) any later version. The GNU C Library is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details. You should have received a copy of the GNU Lesser General Public License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ /* This test runs GDB against a forked copy of itself, to check whether libthread_db can be loaded, and that access to thread-local variables works. */ #include <errno.h> #include <stdlib.h> #include <support/check.h> #include <support/support.h> #include <support/temp_file.h> #include <support/test-driver.h> #include <support/xstdio.h> #include <support/xthread.h> #include <support/xunistd.h> #include <unistd.h> /* Starts out as zero, changed to 1 or 2 by the debugger, depending on the thread. */ __thread volatile int altered_by_debugger; /* Writes the GDB script to run the test to PATH. */ static void write_gdbscript (const char *path, int tested_pid) { FILE *fp = xfopen (path, "w"); char *exepath = realpath ("/proc/self/exe", NULL); TEST_VERIFY_EXIT (exepath != NULL); fprintf (fp, "set trace-commands on\n" "set debug libthread-db 1\n" "add-symbol-file %s\n" "attach %d\n", exepath, tested_pid); fputs ("break debugger_inspection_point\n" "continue\n" "thread 1\n" "print altered_by_debugger\n" "print altered_by_debugger = 1\n" "thread 2\n" "print altered_by_debugger\n" "print altered_by_debugger = 2\n" "continue\n", fp); free (exepath); xfclose (fp); } /* The test sets a breakpoint on this function and alters the altered_by_debugger thread-local variable. */ void __attribute__ ((weak)) debugger_inspection_point (void) { } /* Thread function for the test thread in the subprocess. */ static void * subprocess_thread (void *closure) { /* Wait until altered_by_debugger changes the value away from 0. */ while (altered_by_debugger == 0) { usleep (100 * 1000); debugger_inspection_point (); } TEST_COMPARE (altered_by_debugger, 2); return NULL; } /* This function implements the subprocess under test. It creates a second thread, waiting for its value to change to 2, and checks that the main thread also changed its value to 1. */ static void in_subprocess (void) { pthread_t thr = xpthread_create (NULL, subprocess_thread, NULL); TEST_VERIFY (xpthread_join (thr) == NULL); TEST_COMPARE (altered_by_debugger, 1); _exit (0); } static int do_test (void) { pid_t tested_pid = xfork (); if (tested_pid == 0) in_subprocess (); char *tested_pid_string = xasprintf ("%d", tested_pid); char *gdbscript; xclose (create_temp_file ("tst-pthread-gdb-attach-", &gdbscript)); write_gdbscript (gdbscript, tested_pid); pid_t gdb_pid = xfork (); if (gdb_pid == 0) { clearenv (); xdup2 (STDOUT_FILENO, STDERR_FILENO); execlp ("gdb", "gdb", "-nx", "-batch", "-x", gdbscript, NULL); if (errno == ENOENT) _exit (EXIT_UNSUPPORTED); else _exit (1); } int status; TEST_COMPARE (xwaitpid (gdb_pid, &status, 0), gdb_pid); if (WIFEXITED (status) && WEXITSTATUS (status) == EXIT_UNSUPPORTED) /* gdb is not installed. */ return EXIT_UNSUPPORTED; TEST_COMPARE (status, 0); TEST_COMPARE (xwaitpid (tested_pid, &status, 0), tested_pid); TEST_COMPARE (status, 0); free (tested_pid_string); free (gdbscript); return 0; } #include <support/test-driver.c>
diff --git a/nptl/Makefile b/nptl/Makefile index 8fe92d43fa..8a6a0a0edd 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -313,7 +313,8 @@ tests = tst-attr2 tst-attr3 tst-default-attr \ tst-thread-affinity-sched \ tst-pthread-defaultattr-free \ tst-pthread-attr-sigmask \ - tst-pthread-timedlock-lockloop + tst-pthread-timedlock-lockloop \ + tst-pthread-gdb-attach tests-container = tst-pthread-getattr @@ -359,6 +360,19 @@ CPPFLAGS-test-cond-printers.c := $(CFLAGS-printers-tests) CPPFLAGS-test-rwlockattr-printers.c := $(CFLAGS-printers-tests) CPPFLAGS-test-rwlock-printers.c := $(CFLAGS-printers-tests) +# Reuse the CFLAGS setting for the GDB attaching test. It needs +# debugging information. +CFLAGS-tst-pthread-gdb-attach.c := $(CFLAGS-printers-tests) +CPPFLAGS-tst-pthread-gdb-attach.c := $(CFLAGS-printers-tests) +ifeq ($(build-shared)$(build-hardcoded-path-in-tests),yesno) +CPPFLAGS-tst-pthread-gdb-attach.c += -DDO_ADD_SYMBOL_FILE=1 +else +CPPFLAGS-tst-pthread-gdb-attach.c += -DDO_ADD_SYMBOL_FILE=0 +endif +CFLAGS-tst-pthread-gdb-attach-static.c := $(CFLAGS-printers-tests) +CPPFLAGS-tst-pthread-gdb-attach-static.c := \ + $(CFLAGS-printers-tests) -DDO_ADD_SYMBOL_FILE=0 + ifeq ($(build-shared),yes) tests-printers-libs := $(shared-thread-library) else @@ -430,7 +444,8 @@ link-libc-static := $(common-objpfx)libc.a $(static-gnulib) \ tests-static += tst-stackguard1-static \ tst-cancel24-static \ tst-mutex8-static tst-mutexpi8-static tst-sem11-static \ - tst-sem12-static tst-cond11-static + tst-sem12-static tst-cond11-static \ + tst-pthread-gdb-attach-static tests += tst-cancel24-static diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 6c645aff48..f13d8e44a4 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -51,6 +51,14 @@ static td_thr_events_t __nptl_threads_events __attribute_used__; /* Pointer to descriptor with the last event. */ static struct pthread *__nptl_last_event __attribute_used__; +#ifdef SHARED +/* This variable is used to access _rtld_global from libthread_db. If + GDB loads libpthread before ld.so, it is not possible to resolve + _rtld_global directly during libpthread initialization. */ +static struct rtld_global *__nptl_rtld_global __attribute_used__ + = &_rtld_global; +#endif + /* Number of threads running. */ unsigned int __nptl_nthreads = 1; diff --git a/nptl/tst-pthread-gdb-attach-static.c b/nptl/tst-pthread-gdb-attach-static.c new file mode 100644 index 0000000000..e159632cac --- /dev/null +++ b/nptl/tst-pthread-gdb-attach-static.c @@ -0,0 +1 @@ +#include "tst-pthread-gdb-attach.c" diff --git a/nptl/tst-pthread-gdb-attach.c b/nptl/tst-pthread-gdb-attach.c new file mode 100644 index 0000000000..36f2e4e526 --- /dev/null +++ b/nptl/tst-pthread-gdb-attach.c @@ -0,0 +1,134 @@ +/* Smoke testing GDB process attach with thread-local variable access. + Copyright (C) 2021 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +/* This test runs GDB against a forked copy of itself, to check + whether libthreaddb can be loaded, and that access to thread-local + variables works. */ + +#include <support/check.h> +#include <support/support.h> +#include <support/temp_file.h> +#include <support/xthread.h> +#include <support/xunistd.h> +#include <support/xstdio.h> +#include <unistd.h> +#include <stdlib.h> + +/* Starts out as zero, changed to 1 or 2 by the debugger, depending on + the thread. */ +__thread volatile int altered_by_debugger; + +/* Writes the GDB script to run the test to PATH. */ +static void +write_gdbscript (const char *path, int tested_pid) +{ + FILE *fp = xfopen (path, "w"); + fprintf (fp, + "set trace-commands on\n" + "set debug libthread-db 1\n" +#if DO_ADD_SYMBOL_FILE + /* Do not do this unconditionally to work around a GDB + assertion failure: ../../gdb/symtab.c:6404: + internal-error: CORE_ADDR get_msymbol_address(objfile*, + const minimal_symbol*): Assertion `(objf->flags & + OBJF_MAINLINE) == 0' failed. */ + "add-symbol-file %1$s/nptl/tst-pthread-gdb-attach\n" +#endif + "set auto-load safe-path %1$s/nptl_db\n" + "set libthread-db-search-path %1$s/nptl_db\n" + "attach %2$d\n", + support_objdir_root, tested_pid); + fputs ("break debugger_inspection_point\n" + "continue\n" + "thread 1\n" + "print altered_by_debugger\n" + "print altered_by_debugger = 1\n" + "thread 2\n" + "print altered_by_debugger\n" + "print altered_by_debugger = 2\n" + "continue\n", + fp); + xfclose (fp); +} + +/* The test sets a breakpoint on this function and alters the + altered_by_debugger thread-local variable. */ +void __attribute__ ((weak)) +debugger_inspection_point (void) +{ +} + +/* Thread function for the test thread in the subprocess. */ +static void * +subprocess_thread (void *closure) +{ + /* Wait until altered_by_debugger changes the value away from 0. */ + while (altered_by_debugger == 0) + { + usleep (100 * 1000); + debugger_inspection_point (); + } + + TEST_COMPARE (altered_by_debugger, 2); + return NULL; +} + +/* This function implements the subprocess un der test. It creates a + second thread, waiting for its value to change to 2, and checks + that the main thread also changed its value to 1. */ +static void +in_subprocess (void) +{ + pthread_t thr = xpthread_create (NULL, subprocess_thread, NULL); + TEST_VERIFY (xpthread_join (thr) == NULL); + TEST_COMPARE (altered_by_debugger, 1); + _exit (0); +} + +static int +do_test (void) +{ + pid_t tested_pid = xfork (); + if (tested_pid == 0) + in_subprocess (); + char *tested_pid_string = xasprintf ("%d", tested_pid); + + char *gdbscript; + xclose (create_temp_file ("tst-pthread-gdb-attach-", &gdbscript)); + write_gdbscript (gdbscript, tested_pid); + + pid_t gdb_pid = xfork (); + if (gdb_pid == 0) + { + clearenv (); + xdup2 (STDOUT_FILENO, STDERR_FILENO); + execlp ("gdb", "gdb", "-nx", "-batch", "-x", gdbscript, NULL); + } + + int status; + TEST_COMPARE (xwaitpid (gdb_pid, &status, 0), gdb_pid); + TEST_COMPARE (status, 0); + TEST_COMPARE (xwaitpid (tested_pid, &status, 0), tested_pid); + TEST_COMPARE (status, 0); + + free (tested_pid_string); + free (gdbscript); + return 0; +} + +#include <support/test-driver.c> diff --git a/nptl_db/structs.def b/nptl_db/structs.def index 999a9fc35a..8a613dd2f5 100644 --- a/nptl_db/structs.def +++ b/nptl_db/structs.def @@ -100,8 +100,7 @@ DB_STRUCT_FIELD (pthread, dtvp) #endif #if !(IS_IN (libpthread) && !defined SHARED) -DB_STRUCT (rtld_global) -DB_RTLD_VARIABLE (_rtld_global) +DB_VARIABLE (__nptl_rtld_global) #endif DB_RTLD_GLOBAL_FIELD (dl_tls_dtv_slotinfo_list) DB_RTLD_GLOBAL_FIELD (dl_stack_user) diff --git a/nptl_db/td_init.c b/nptl_db/td_init.c index 1d15681228..06b5adc5c2 100644 --- a/nptl_db/td_init.c +++ b/nptl_db/td_init.c @@ -33,13 +33,14 @@ td_init (void) bool __td_ta_rtld_global (td_thragent_t *ta) { - if (ta->ta_addr__rtld_global == 0 - && td_mod_lookup (ta->ph, LD_SO, SYM__rtld_global, - &ta->ta_addr__rtld_global) != PS_OK) + if (ta->ta_addr__rtld_global == 0) { - ta->ta_addr__rtld_global = (void*)-1; - return false; + psaddr_t rtldglobalp; + if (DB_GET_VALUE (rtldglobalp, ta, __nptl_rtld_global, 0) == TD_OK) + ta->ta_addr__rtld_global = rtldglobalp; + else + ta->ta_addr__rtld_global = (void *) -1; } - else - return ta->ta_addr__rtld_global != (void*)-1; + + return ta->ta_addr__rtld_global != (void *)-1; } diff --git a/nptl_db/thread_dbP.h b/nptl_db/thread_dbP.h index 580a70c471..712fa3aeb6 100644 --- a/nptl_db/thread_dbP.h +++ b/nptl_db/thread_dbP.h @@ -108,6 +108,8 @@ struct td_thragent # undef DB_SYMBOL # undef DB_VARIABLE + psaddr_t ta_addr__rtld_global; + /* The method of locating a thread's th_unique value. */ enum {
libthread_db is loaded once GDB encounters libpthread, and at this point, ld.so may not have been loaded yet. As a result, _rtld_global cannot be accessed by regular means from libthread_db. To make this work until GDB can be fixed, acess _rtld_global through a pointer stored in libpthread. The new test does not reproduce bug 27744 with --disable-hardcoded-path-in-tests, but is still a valid smoke test. With --enable-hardcoded-path-in-tests, it is necessary to avoid add-symbol-file because this can tickle a GDB bug. Fixes commit 1daccf403b1bd86370eb94edca794dc106d02039 ("nptl: Move stack list variables into _rtld_global"). Tested on i686-linux-gnu and x86_64-linux-gnu, the latter also with --enable-hardcoded-path-in-tests. --- nptl/Makefile | 19 ++++- nptl/pthread_create.c | 8 +++ nptl/tst-pthread-gdb-attach-static.c | 1 + nptl/tst-pthread-gdb-attach.c | 134 +++++++++++++++++++++++++++++++++++ nptl_db/structs.def | 3 +- nptl_db/td_init.c | 15 ++-- nptl_db/thread_dbP.h | 2 + 7 files changed, 171 insertions(+), 11 deletions(-)