diff mbox series

nptl_db: different libpthread/ld.so load orders (bug 27744)

Message ID 87sg3qnrz3.fsf@oldenburg.str.redhat.com
State Superseded
Headers show
Series nptl_db: different libpthread/ld.so load orders (bug 27744) | expand

Commit Message

Florian Weimer April 16, 2021, 3:56 p.m. UTC
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(-)

Comments

Simon Marchi April 16, 2021, 4:07 p.m. UTC | #1
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
Florian Weimer April 16, 2021, 4:12 p.m. UTC | #2
* 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
Pedro Alves April 16, 2021, 4:25 p.m. UTC | #3
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
Florian Weimer April 16, 2021, 4:28 p.m. UTC | #4
* 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
Pedro Alves April 16, 2021, 4:33 p.m. UTC | #5
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
>
Florian Weimer April 16, 2021, 4:43 p.m. UTC | #6
* 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
Pedro Alves April 16, 2021, 4:47 p.m. UTC | #7
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
Simon Marchi April 16, 2021, 4:53 p.m. UTC | #8
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
Pedro Alves April 16, 2021, 5:18 p.m. UTC | #9
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".
>
Florian Weimer April 16, 2021, 5:26 p.m. UTC | #10
* 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
Florian Weimer April 16, 2021, 5:28 p.m. UTC | #11
* 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
Simon Marchi April 16, 2021, 5:33 p.m. UTC | #12
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
Pedro Alves April 16, 2021, 5:43 p.m. UTC | #13
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"?
Pedro Alves April 16, 2021, 6:29 p.m. UTC | #14
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.
Florian Weimer April 16, 2021, 6:35 p.m. UTC | #15
* 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
Florian Weimer April 19, 2021, 9:06 a.m. UTC | #16
* 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 mbox series

Patch

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
     {