* [PATCH 0/3] Better error handling when creating threads with clone()
@ 2026-04-07 10:22 Andrew Burgess
2026-04-07 10:22 ` [PATCH 1/3] gdb: int to bool conversion in linux-thread-db.c Andrew Burgess
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Andrew Burgess @ 2026-04-07 10:22 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
A Fedora GDB bug got me looking at error handling relating to GDB's
clone handling, and thread-db thread adoption.
The first two patches are some refactoring in this area, and the third
patch changes and improves error handling so that GDB is better able
to track threads even if thread-db thread adoption fails.
Thanks,
Andrew
---
Andrew Burgess (3):
gdb: int to bool conversion in linux-thread-db.c
gdb: add some asserts to thread_db_notice_clone
gdb: improve error handling for thread-db thread adoption
gdb/linux-nat.c | 25 +-
gdb/linux-nat.h | 2 +-
gdb/linux-thread-db.c | 67 +++--
gdb/testsuite/gdb.threads/clone-bad-tls.c | 193 +++++++++++++
gdb/testsuite/gdb.threads/clone-bad-tls.exp | 294 ++++++++++++++++++++
gdbserver/thread-db.cc | 14 +-
6 files changed, 553 insertions(+), 42 deletions(-)
create mode 100644 gdb/testsuite/gdb.threads/clone-bad-tls.c
create mode 100644 gdb/testsuite/gdb.threads/clone-bad-tls.exp
base-commit: 9d3cf9efd51ebae3f45bb49e3544cb7eeb63a138
--
2.25.4
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/3] gdb: int to bool conversion in linux-thread-db.c 2026-04-07 10:22 [PATCH 0/3] Better error handling when creating threads with clone() Andrew Burgess @ 2026-04-07 10:22 ` Andrew Burgess 2026-05-05 18:19 ` Keith Seitz 2026-04-07 10:22 ` [PATCH 2/3] gdb: add some asserts to thread_db_notice_clone Andrew Burgess 2026-04-07 10:22 ` [PATCH 3/3] gdb: improve error handling for thread-db thread adoption Andrew Burgess 2 siblings, 1 reply; 7+ messages in thread From: Andrew Burgess @ 2026-04-07 10:22 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess Some 'int' to 'bool' cleanup in linux-thread-db.c. There should be no user visible changes after this commit. --- gdb/linux-nat.h | 2 +- gdb/linux-thread-db.c | 40 ++++++++++++++++++++-------------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h index 0f128892735..180059a9496 100644 --- a/gdb/linux-nat.h +++ b/gdb/linux-nat.h @@ -316,7 +316,7 @@ lwp_info_safe_range all_lwps_safe (); does whatever is required to have the child LWP under the thread_db's control --- e.g., enabling event reporting. Returns true on success, false if the process isn't using libpthread. */ -extern int thread_db_notice_clone (ptid_t parent, ptid_t child); +extern bool thread_db_notice_clone (ptid_t parent, ptid_t child); /* Return the number of signals used by the threads library. */ extern unsigned int lin_thread_get_thread_signal_num (void); diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c index 0753d2e7915..cd25c55a898 100644 --- a/gdb/linux-thread-db.c +++ b/gdb/linux-thread-db.c @@ -429,7 +429,7 @@ thread_from_lwp (thread_info *stopped, ptid_t ptid) /* See linux-nat.h. */ -int +bool thread_db_notice_clone (ptid_t parent, ptid_t child) { struct thread_db_info *info; @@ -437,7 +437,7 @@ thread_db_notice_clone (ptid_t parent, ptid_t child) info = get_thread_db_info (linux_target, child.pid ()); if (info == NULL) - return 0; + return false; thread_info *stopped = linux_target->find_thread (parent); @@ -446,7 +446,7 @@ thread_db_notice_clone (ptid_t parent, ptid_t child) /* If we do not know about the main thread's pthread info yet, this would be a good time to find it. */ thread_from_lwp (stopped, parent); - return 1; + return true; } static void * @@ -460,10 +460,10 @@ verbose_dlsym (void *handle, const char *name) } /* Verify inferior's '\0'-terminated symbol VER_SYMBOL starts with "%d.%d" and - return 1 if this version is lower (and not equal) to - VER_MAJOR_MIN.VER_MINOR_MIN. Return 0 in all other cases. */ + return true if this version is lower (and not equal) to + VER_MAJOR_MIN.VER_MINOR_MIN. Return false in all other cases. */ -static int +static bool inferior_has_bug (const char *ver_symbol, int ver_major_min, int ver_minor_min) { CORE_ADDR version_addr; @@ -472,7 +472,7 @@ inferior_has_bug (const char *ver_symbol, int ver_major_min, int ver_minor_min) bound_minimal_symbol version_msym = lookup_minimal_symbol (current_program_space, ver_symbol); if (version_msym.minsym == NULL) - return 0; + return false; version_addr = version_msym.value_address (); gdb::unique_xmalloc_ptr<char> version @@ -493,10 +493,10 @@ inferior_has_bug (const char *ver_symbol, int ver_major_min, int ver_minor_min) /* Similar as thread_db_find_new_threads_1, but try to silently ignore errors if appropriate. - Return 1 if the caller should abort libthread_db initialization. Return 0 - otherwise. */ + Return true if the caller should abort libthread_db initialization. + Return false otherwise. */ -static int +static bool thread_db_find_new_threads_silently (thread_info *stopped) { @@ -518,25 +518,25 @@ thread_db_find_new_threads_silently (thread_info *stopped) itself. If the nptl bug is NOT present in the inferior and still thread_db - reports an error return 1. It means the inferior has corrupted thread - list and GDB should fall back only to LWPs. + reports an error return true. It means the inferior has corrupted + thread list and GDB should fall back only to LWPs. - If the nptl bug is present in the inferior return 0 to silently ignore - such errors, and let gdb enumerate threads again later. In such case - GDB cannot properly display LWPs if the inferior thread list is - corrupted. For core files it does not apply, no 'later enumeration' - is possible. */ + If the nptl bug is present in the inferior return false to + silently ignore such errors, and let gdb enumerate threads again + later. In such case GDB cannot properly display LWPs if the + inferior thread list is corrupted. For core files it does not + apply, no 'later enumeration' is possible. */ if (!target_has_execution () || !inferior_has_bug ("nptl_version", 2, 7)) { exception_fprintf (gdb_stderr, except, _("Warning: couldn't activate thread debugging " "using libthread_db: ")); - return 1; + return true; } } - return 0; + return false; } /* Lookup a library in which given symbol resides. @@ -924,7 +924,7 @@ try_thread_db_load_1 (struct thread_db_info *info) linux_unstop_all_lwps (); } - else if (thread_db_find_new_threads_silently (inferior_thread ()) != 0) + else if (thread_db_find_new_threads_silently (inferior_thread ())) { /* Even if libthread_db initializes, if the thread list is corrupted, we'd not manage to list any threads. Better reject this -- 2.25.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] gdb: int to bool conversion in linux-thread-db.c 2026-04-07 10:22 ` [PATCH 1/3] gdb: int to bool conversion in linux-thread-db.c Andrew Burgess @ 2026-05-05 18:19 ` Keith Seitz 0 siblings, 0 replies; 7+ messages in thread From: Keith Seitz @ 2026-05-05 18:19 UTC (permalink / raw) To: Andrew Burgess, gdb-patches On 4/7/26 3:22 AM, Andrew Burgess wrote: > Some 'int' to 'bool' cleanup in linux-thread-db.c. There should be no > user visible changes after this commit. More int -> bool +1000! Reviewed-By: Keith Seitz <keiths@redhat.com> Keith > --- > gdb/linux-nat.h | 2 +- > gdb/linux-thread-db.c | 40 ++++++++++++++++++++-------------------- > 2 files changed, 21 insertions(+), 21 deletions(-) > > diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h > index 0f128892735..180059a9496 100644 > --- a/gdb/linux-nat.h > +++ b/gdb/linux-nat.h > @@ -316,7 +316,7 @@ lwp_info_safe_range all_lwps_safe (); > does whatever is required to have the child LWP under the > thread_db's control --- e.g., enabling event reporting. Returns > true on success, false if the process isn't using libpthread. */ > -extern int thread_db_notice_clone (ptid_t parent, ptid_t child); > +extern bool thread_db_notice_clone (ptid_t parent, ptid_t child); > > /* Return the number of signals used by the threads library. */ > extern unsigned int lin_thread_get_thread_signal_num (void); > diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c > index 0753d2e7915..cd25c55a898 100644 > --- a/gdb/linux-thread-db.c > +++ b/gdb/linux-thread-db.c > @@ -429,7 +429,7 @@ thread_from_lwp (thread_info *stopped, ptid_t ptid) > > /* See linux-nat.h. */ > > -int > +bool > thread_db_notice_clone (ptid_t parent, ptid_t child) > { > struct thread_db_info *info; > @@ -437,7 +437,7 @@ thread_db_notice_clone (ptid_t parent, ptid_t child) > info = get_thread_db_info (linux_target, child.pid ()); > > if (info == NULL) > - return 0; > + return false; > > thread_info *stopped = linux_target->find_thread (parent); > > @@ -446,7 +446,7 @@ thread_db_notice_clone (ptid_t parent, ptid_t child) > /* If we do not know about the main thread's pthread info yet, this > would be a good time to find it. */ > thread_from_lwp (stopped, parent); > - return 1; > + return true; > } > > static void * > @@ -460,10 +460,10 @@ verbose_dlsym (void *handle, const char *name) > } > > /* Verify inferior's '\0'-terminated symbol VER_SYMBOL starts with "%d.%d" and > - return 1 if this version is lower (and not equal) to > - VER_MAJOR_MIN.VER_MINOR_MIN. Return 0 in all other cases. */ > + return true if this version is lower (and not equal) to > + VER_MAJOR_MIN.VER_MINOR_MIN. Return false in all other cases. */ > > -static int > +static bool > inferior_has_bug (const char *ver_symbol, int ver_major_min, int ver_minor_min) > { > CORE_ADDR version_addr; > @@ -472,7 +472,7 @@ inferior_has_bug (const char *ver_symbol, int ver_major_min, int ver_minor_min) > bound_minimal_symbol version_msym > = lookup_minimal_symbol (current_program_space, ver_symbol); > if (version_msym.minsym == NULL) > - return 0; > + return false; > > version_addr = version_msym.value_address (); > gdb::unique_xmalloc_ptr<char> version > @@ -493,10 +493,10 @@ inferior_has_bug (const char *ver_symbol, int ver_major_min, int ver_minor_min) > /* Similar as thread_db_find_new_threads_1, but try to silently ignore errors > if appropriate. > > - Return 1 if the caller should abort libthread_db initialization. Return 0 > - otherwise. */ > + Return true if the caller should abort libthread_db initialization. > + Return false otherwise. */ > > -static int > +static bool > thread_db_find_new_threads_silently (thread_info *stopped) > { > > @@ -518,25 +518,25 @@ thread_db_find_new_threads_silently (thread_info *stopped) > itself. > > If the nptl bug is NOT present in the inferior and still thread_db > - reports an error return 1. It means the inferior has corrupted thread > - list and GDB should fall back only to LWPs. > + reports an error return true. It means the inferior has corrupted > + thread list and GDB should fall back only to LWPs. > > - If the nptl bug is present in the inferior return 0 to silently ignore > - such errors, and let gdb enumerate threads again later. In such case > - GDB cannot properly display LWPs if the inferior thread list is > - corrupted. For core files it does not apply, no 'later enumeration' > - is possible. */ > + If the nptl bug is present in the inferior return false to > + silently ignore such errors, and let gdb enumerate threads again > + later. In such case GDB cannot properly display LWPs if the > + inferior thread list is corrupted. For core files it does not > + apply, no 'later enumeration' is possible. */ > > if (!target_has_execution () || !inferior_has_bug ("nptl_version", 2, 7)) > { > exception_fprintf (gdb_stderr, except, > _("Warning: couldn't activate thread debugging " > "using libthread_db: ")); > - return 1; > + return true; > } > } > > - return 0; > + return false; > } > > /* Lookup a library in which given symbol resides. > @@ -924,7 +924,7 @@ try_thread_db_load_1 (struct thread_db_info *info) > > linux_unstop_all_lwps (); > } > - else if (thread_db_find_new_threads_silently (inferior_thread ()) != 0) > + else if (thread_db_find_new_threads_silently (inferior_thread ())) > { > /* Even if libthread_db initializes, if the thread list is > corrupted, we'd not manage to list any threads. Better reject this ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] gdb: add some asserts to thread_db_notice_clone 2026-04-07 10:22 [PATCH 0/3] Better error handling when creating threads with clone() Andrew Burgess 2026-04-07 10:22 ` [PATCH 1/3] gdb: int to bool conversion in linux-thread-db.c Andrew Burgess @ 2026-04-07 10:22 ` Andrew Burgess 2026-05-05 18:24 ` Keith Seitz 2026-04-07 10:22 ` [PATCH 3/3] gdb: improve error handling for thread-db thread adoption Andrew Burgess 2 siblings, 1 reply; 7+ messages in thread From: Andrew Burgess @ 2026-04-07 10:22 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess This commit adds some asserts to thread_db_notice_clone, and reorders some code. This is really just a refactor ahead of the next commit. The changes made here are: 1. Add an assert that STOPPED is not NULL. We pass STOPPED as the first argument to thread_from_lwp, which already assumes that the pointer is not NULL. If STOPPED was ever NULL then we would have hit a segfault in thread_from_lwp long ago. 2. Move the thread_from_lwp call relating to the PARENT thread before the call relating to the CHILD thread, and assert that the value we get back is STOPPED. The thread_from_lwp might also gather thread-db related information about PARENT, so the call is important, but even if PARENT cannot be managed via thread-db, the result we get back should always still be STOPPED (see record_thread for more details). Moving this call before the call relating to the CHILD thread makes the next commit cleaner. There should be no user visible changes after this commit as I believe both of the things being asserted have always been true. --- gdb/linux-thread-db.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c index cd25c55a898..1807352ed76 100644 --- a/gdb/linux-thread-db.c +++ b/gdb/linux-thread-db.c @@ -440,12 +440,17 @@ thread_db_notice_clone (ptid_t parent, ptid_t child) return false; thread_info *stopped = linux_target->find_thread (parent); - - thread_from_lwp (stopped, child); + gdb_assert (stopped != nullptr); /* If we do not know about the main thread's pthread info yet, this - would be a good time to find it. */ - thread_from_lwp (stopped, parent); + would be a good time to find it. This should return the same + thread_info as STOPPED, but a side effect of this call is that the + pthread/thread-db information might have been filled in if it was not + already known. */ + thread_info *parent_info = thread_from_lwp (stopped, parent); + gdb_assert (parent_info == stopped); + + thread_from_lwp (stopped, child); return true; } -- 2.25.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] gdb: add some asserts to thread_db_notice_clone 2026-04-07 10:22 ` [PATCH 2/3] gdb: add some asserts to thread_db_notice_clone Andrew Burgess @ 2026-05-05 18:24 ` Keith Seitz 0 siblings, 0 replies; 7+ messages in thread From: Keith Seitz @ 2026-05-05 18:24 UTC (permalink / raw) To: Andrew Burgess, gdb-patches Hi, On 4/7/26 3:22 AM, Andrew Burgess wrote: > This commit adds some asserts to thread_db_notice_clone, and reorders > some code. This is really just a refactor ahead of the next commit. > The changes made here are: > > 1. Add an assert that STOPPED is not NULL. We pass STOPPED as the > first argument to thread_from_lwp, which already assumes that the > pointer is not NULL. If STOPPED was ever NULL then we would have > hit a segfault in thread_from_lwp long ago. > > 2. Move the thread_from_lwp call relating to the PARENT thread > before the call relating to the CHILD thread, and assert that the > value we get back is STOPPED. The thread_from_lwp might also > gather thread-db related information about PARENT, so the call is > important, but even if PARENT cannot be managed via thread-db, > the result we get back should always still be STOPPED (see > record_thread for more details). Moving this call before the > call relating to the CHILD thread makes the next commit cleaner. > > There should be no user visible changes after this commit as I believe > both of the things being asserted have always been true. I'm no expert at this, but your assumption seems correct as far as I understand it. So this LGTM. Reviewed-By: Keith Seitz <keiths@redhat.com> Keith > --- > gdb/linux-thread-db.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c > index cd25c55a898..1807352ed76 100644 > --- a/gdb/linux-thread-db.c > +++ b/gdb/linux-thread-db.c > @@ -440,12 +440,17 @@ thread_db_notice_clone (ptid_t parent, ptid_t child) > return false; > > thread_info *stopped = linux_target->find_thread (parent); > - > - thread_from_lwp (stopped, child); > + gdb_assert (stopped != nullptr); > > /* If we do not know about the main thread's pthread info yet, this > - would be a good time to find it. */ > - thread_from_lwp (stopped, parent); > + would be a good time to find it. This should return the same > + thread_info as STOPPED, but a side effect of this call is that the > + pthread/thread-db information might have been filled in if it was not > + already known. */ > + thread_info *parent_info = thread_from_lwp (stopped, parent); > + gdb_assert (parent_info == stopped); > + > + thread_from_lwp (stopped, child); > return true; > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] gdb: improve error handling for thread-db thread adoption 2026-04-07 10:22 [PATCH 0/3] Better error handling when creating threads with clone() Andrew Burgess 2026-04-07 10:22 ` [PATCH 1/3] gdb: int to bool conversion in linux-thread-db.c Andrew Burgess 2026-04-07 10:22 ` [PATCH 2/3] gdb: add some asserts to thread_db_notice_clone Andrew Burgess @ 2026-04-07 10:22 ` Andrew Burgess 2026-05-06 19:39 ` Keith Seitz 2 siblings, 1 reply; 7+ messages in thread From: Andrew Burgess @ 2026-04-07 10:22 UTC (permalink / raw) To: gdb-patches; +Cc: Andrew Burgess This work started with a bug report against Fedora GDB. The bug link is included at the end of this message, but it isn't super informative and there was no follow up, so all we have is the original report. The bug was that GDB crashed with this backtrace: #4 dump_core at ../../gdb/utils.c:225 #5 internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_list_tag __va_list_tag *) at ../../gdb/utils.c:477 #6 internal_verror at ../../gdb/utils.c:503 #7 internal_error_loc at ../../gdbsupport/errors.cc:57 #8 report_exit_events_for at ../../gdb/linux-nat.c:331 #10 linux_nat_filter_event at ../../gdb/linux-nat.c:3063 #11 linux_nat_wait_1 at ../../gdb/linux-nat.c:3377 #12 linux_nat_target::wait at ../../gdb/linux-nat.c:3603 #13 thread_db_target::wait at ../../gdb/linux-thread-db.c:1401 #14 target_wait at ../../gdb/target.c:2582 #15 do_target_wait_1 at ../../gdb/infrun.c:4144 #16 operator() at ../../gdb/infrun.c:4206 #17 do_target_wait at ../../gdb/infrun.c:4225 #18 fetch_inferior_event at ../../gdb/infrun.c:4666 #19 gdb_wait_for_event at ../../gdbsupport/event-loop.cc:694 #20 gdb_do_one_event at ../../gdbsupport/event-loop.cc:216 #21 start_event_loop at ../../gdb/main.c:402 #22 captured_command_loop at ../../gdb/main.c:466 #23 captured_main at ../../gdb/main.c:1343 #24 gdb_main at ../../gdb/main.c:1362 Frame #8 is where things start going wrong, and the line in question is this gdb_assert: thread_info *thr = linux_target->find_thread (lp->ptid); gdb_assert (thr != nullptr); <---- This is line 331. So what we know is that while reporting a thread exit event GDB failed to find the thread_info for a thread. We can imagine many bugs that might cause this to occur, some of which could even be outside GDB. But without additional information I'm not inclined to start looking for bugs outside of GDB. So I started looking for ways that the bug could be triggered from within GDB. One way the bug could occur is if GDB fails to create a thread_info for a new thread. This led me to look at linux_nat_target::follow_clone, which led to looking at thread_db_notice_clone. In ::follow_clone GDB already has code to handle the case where thread_db_notice_clone fails to add a thread: if (!thread_db_notice_clone (inferior_ptid, new_lp->ptid)) { /* The process is not using thread_db. Add the LWP to GDB's list. */ add_thread (linux_target, new_lp->ptid); } But does this always do what we need? In thread_db_notice_clone, if the CHILD thread was not adopted by thread-db then the thread_from_lwp will return NULL, but we don't currently check this, instead we always return true from thread_db_notice_clone. This is an easy fix, we can just check the return value of thread_from_lwp in thread_db_notice_clone. Next, looking through thread_from_lwp, we can see several calls to `error`. If any of these trigger then GDB will throw an exception, in which case we'll unwind back through ::follow_clone without adding the thread to GDB's data structures. I think this can be fixed by using a FORWARD_SCOPE_EXIT in ::follow_clone which will add the thread to GDB's data structures. Now, if an exception triggers, we will still record the thread. Initially, I'd thought just adding the thread on the exception path would be enough, on x86-64 the target I initially tested on, I wasn't able to trigger the error cases, so I was writing code to cover this path "just in case". But when I started testing on AArch64 and PPC64le, I found that my same test case (more details below) was triggering the error path. What I found is that throwing an exception from thread_from_lwp was causing GDB to stop the inferior unnecessarily. GDB would see a CLONE event from the inferior, try to add the new thread to GDB, but this would result in the `error` path, and an exception being thrown. The exception was not handled anywhere, so GDB would drop back to the prompt, leaving the inferior stopped. This is despite the fallback code that I added above handling the failure of thread-db to adopt the thread. My solution for this is to convert the `error` calls into warnings, and just have thread_from_lwp return NULL if thread-db cannot adopt the thread. This potentially makes the use of FORWARD_SCOPE_EXIT in ::follow_clone unnecessary, I no longer expect any exceptions to be thrown from thread_db_notice_clone. However, I think maintaining the use of FORWARD_SCOPE_EXIT is still a good idea. If I've missed any exception routes, or any are added in the future, then this will ensure the threads are always added to GDB correctly. The testing for this change is a little weird. It involves creating a thread using a direct clone call, and manually setting the thread's TLS pointer to NULL. So long as we don't do anything that might require TLS access within the thread then this works on at least some target architectures (tested on x86-64), but the NULL TLS pointer is enough to prevent adoption of the thread by thread-db. We do already have some tests in gdb.threads/ that test using raw clone calls, so this isn't the first test in this area. Without this patch in place the new test will trigger the assertion that was the cause of the original bug report, but also triggers a couple of segfaults from within GDB, places where we assume we should have a thread_info available. With the patch in place the test works as expected. I also tested against gdbserver, and that works fine too, though I did need to perform a similar error to warning conversion as I have in GDB. With that done gdbserver testing works fine. There are some more warnings printed by gdbserver that we don't see when doing native GDB testing. The warnings can be found in the gdb.log after running with the native-gdbserver or native-extended-gdbserver boards, and look like this: gdbserver: PID mismatch! Expected 633347, got 633343 gdbserver: Cannot find thread after clone. gdbserver: PID mismatch! Expected 633347, got 633343 gdbserver: PID mismatch! Expected 633347, got 633343 These warnings are just a consequence of gdbserver not using thread-db to manage the threads. Maybe we could/should silence these warnings as gdbserver is still happily managing the threads, just outside of thread-db, but I don't plan to do anything about these right now. Tested on GNU/Linux on x86-64, AArch64, RISC-V, and PPC64le using unix, native-gdbserver, and native-extended-gdbserver boards. Bug: https://bugzilla.redhat.com/show_bug.cgi?id=2414004 --- gdb/linux-nat.c | 25 +- gdb/linux-thread-db.c | 18 +- gdb/testsuite/gdb.threads/clone-bad-tls.c | 193 +++++++++++++ gdb/testsuite/gdb.threads/clone-bad-tls.exp | 294 ++++++++++++++++++++ gdbserver/thread-db.cc | 14 +- 5 files changed, 525 insertions(+), 19 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/clone-bad-tls.c create mode 100644 gdb/testsuite/gdb.threads/clone-bad-tls.exp diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 1d0a4609a3a..bdb948aebaf 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -1971,15 +1971,22 @@ linux_nat_target::follow_clone (ptid_t child_ptid) lwp_info *new_lp = add_lwp (child_ptid); new_lp->stopped = 1; - /* If the thread_db layer is active, let it record the user - level thread id and status, and add the thread to GDB's - list. */ - if (!thread_db_notice_clone (inferior_ptid, new_lp->ptid)) - { - /* The process is not using thread_db. Add the LWP to - GDB's list. */ - add_thread (linux_target, new_lp->ptid); - } + /* If the thread_db layer is active, let it record the user level thread + id and status, and add the thread to GDB's list. */ + { + /* If the thread_db layer fails to record the thread, including if it + throws an error, then we should make sure GDB always adds the thread + to its data structures otherwise lots of things can go wrong. For + example, GDB will not resume the thread, or if/when the thread + exits, GDB will be surprised and raise an assertion. */ + using scoped_add_thread = FORWARD_SCOPE_EXIT (add_thread); + scoped_add_thread add_thread_fallback (linux_target, new_lp->ptid); + + /* If the thread_db layer successfully adopts the thread then we no + longer need our fallback to add the thread for us. */ + if (thread_db_notice_clone (inferior_ptid, new_lp->ptid)) + add_thread_fallback.release (); + } /* We just created NEW_LP so it cannot yet contain STATUS. */ gdb_assert (new_lp->status == 0); diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c index 1807352ed76..1ae9323108a 100644 --- a/gdb/linux-thread-db.c +++ b/gdb/linux-thread-db.c @@ -413,13 +413,19 @@ thread_from_lwp (thread_info *stopped, ptid_t ptid) err = info->td_ta_map_lwp2thr_p (info->thread_agent, ptid.lwp (), &th); if (err != TD_OK) - error (_("Cannot find user-level thread for LWP %ld: %s"), - ptid.lwp (), thread_db_err_str (err)); + { + warning (_("Cannot find user-level thread for LWP %ld: %s"), + ptid.lwp (), thread_db_err_str (err)); + return nullptr; + } err = info->td_thr_get_info_p (&th, &ti); if (err != TD_OK) - error (_("thread_get_info_callback: cannot get thread info: %s"), - thread_db_err_str (err)); + { + warning (_("thread_get_info_callback: cannot get thread info: %s"), + thread_db_err_str (err)); + return nullptr; + } /* Fill the cache. */ tp = stopped->inf->process_target ()->find_thread (ptid); @@ -450,8 +456,8 @@ thread_db_notice_clone (ptid_t parent, ptid_t child) thread_info *parent_info = thread_from_lwp (stopped, parent); gdb_assert (parent_info == stopped); - thread_from_lwp (stopped, child); - return true; + thread_info *child_info = thread_from_lwp (stopped, child); + return child_info != nullptr; } static void * diff --git a/gdb/testsuite/gdb.threads/clone-bad-tls.c b/gdb/testsuite/gdb.threads/clone-bad-tls.c new file mode 100644 index 00000000000..dfb3ff79c17 --- /dev/null +++ b/gdb/testsuite/gdb.threads/clone-bad-tls.c @@ -0,0 +1,193 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2026 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. + + Test that GDB doesn't lose an event for a thread it didn't know + about, until an event is reported for it. */ + +#define _GNU_SOURCE +#include <sched.h> +#include <assert.h> +#include <stdlib.h> +#include <sys/types.h> +#include <sys/wait.h> +#include <unistd.h> +#include <pthread.h> +#include <linux/futex.h> +#include <sys/syscall.h> + +/* Global synchronization variables. */ +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; +pthread_cond_t cond = PTHREAD_COND_INITIALIZER; +int ready = 0; + +#define STACK_SIZE 0x1000 + +/* Wrap futex syscall. See 'man 2 futex'. */ + +static int +futex (int *uaddr, int futex_op, int val, const struct timespec *timeout, + int *uaddr2, int val3) +{ + return syscall (SYS_futex, uaddr, futex_op, val, timeout, uaddr2, val3); +} + +/* Function to run in the raw clone thread. This just spins until the + integer flag pointed to by ARG is set to non-zero. */ + +static int +clone_fn (void *arg) +{ + int *thread_exit = (int *) arg; /* Break inside raw thread. */ + *thread_exit = 0; + + while (*thread_exit == 0) + ; +} + +/* A variable into which the kernel will write the child thread-id. This + will be cleared when the thread exits. This only gets used when + USE_FUTEX is defined. */ + +static int thread_tid; + +/* Create a thread using a raw clone call. */ + +void * +do_raw_clone () +{ + unsigned char *stack; + int res; + int clone_pid; + + /* This flag is shared with the raw thread. It is initially -1, then + set to 0 in the raw thread to show that the thread has started up. + This flag is then set to 1 by the main thread to indicate that the + raw thread should exit. Using a volatile for thread synchronisation + is not great, but this avoids having to make library calls from the + raw thread, which might trigger a need for TLS to be setup correctly + on some targets. */ + volatile int thread_exit = -1; + + stack = malloc (STACK_SIZE); + assert (stack != NULL); /* Break before raw thread created. */ + +#ifdef USE_FUTEX +#define TID_FLAGS (CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID) +#else +#define TID_FLAGS (0) +#endif + +#define CLONE_FLAGS (CLONE_THREAD | CLONE_SIGHAND | CLONE_VM \ + | CLONE_SETTLS | TID_FLAGS) + + clone_pid = clone (clone_fn, stack + STACK_SIZE, CLONE_FLAGS, + (void *) &thread_exit, &thread_tid, + NULL, &thread_tid); + + assert (clone_pid > 0); /* Immediately after the clone. */ + + while (thread_exit != 0) + usleep (1000); + + /* Trigger the clone thread to exit. */ + thread_exit = 1; /* Break after raw thread created. */ + +#ifdef USE_FUTEX + /* In this mode we rely on the futex to notify us when the thread has + exited. In a broken GDB we used to deadlock at this point as GDB + would fail to restart the raw thread, and so the kernel would never + wake this futex. */ + while (thread_tid != 0) + futex (&thread_tid, FUTEX_WAIT, clone_pid, NULL, NULL, 0); + + /* We do know that the raw thread has completed at this point, so we + could free its stack. We don't though, just to be consistent with the + no futex path. This doesn't really matter, this is just a small + test. */ +#else + /* There's no attempt to synchronise with the raw clone thread on this + path. In a broken GDB the raw thread might not be resumed, in which + case this timeout will expire. */ + sleep (2); + + /* We cannot be sure that the raw thread has finished by this point, so + we cannot free the stack. This isn't critical, this is just a small + test case. */ +#endif + + return NULL; /* Break after raw thread exits. */ +} + +/* Something for our pthread thread to do. This just blocks until the main + thread releases it, at which point this thread will exit. */ + +void * +worker_thread (void *arg) +{ + pthread_mutex_lock(&mutex); + + /* Let the main thread know we are live by clearing the ready flag. */ + ready = 0; + pthread_cond_signal(&cond); + + /* Now spin until the main thread sets this flag back to non-zero. */ + while (ready == 0) + { + /* The thread will block here until main signals 'cond' */ + pthread_cond_wait (&cond, &mutex); + } + + pthread_mutex_unlock (&mutex); + + return NULL; +} + +int +main (void) +{ + pthread_t thread_id; + volatile int flag = 0; + int res; + + /* The pthread will set the READY flag back to zero. */ + ready = 1; + + alarm (300); + + res = pthread_create (&thread_id, NULL, worker_thread, (void *)&flag); + assert (res == 0); + + /* Wait for the pthread to set READY to zero. */ + pthread_mutex_lock(&mutex); + while (ready == 1) + pthread_cond_wait (&cond, &mutex); + pthread_mutex_unlock (&mutex); + + /* Create a thread using a raw clone call. */ + do_raw_clone (); /* Break before do_raw_clone. */ + + /* Unblock the pthread. */ + pthread_mutex_lock (&mutex); + ready = 1; + pthread_cond_signal (&cond); + pthread_mutex_unlock (&mutex); + + /* Wait for the worker to finish. */ + pthread_join (thread_id, NULL); + + return 0; +} diff --git a/gdb/testsuite/gdb.threads/clone-bad-tls.exp b/gdb/testsuite/gdb.threads/clone-bad-tls.exp new file mode 100644 index 00000000000..2de16c63b6e --- /dev/null +++ b/gdb/testsuite/gdb.threads/clone-bad-tls.exp @@ -0,0 +1,294 @@ +# This testcase is part of GDB, the GNU debugger. + +# Copyright 2026 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# This test creates a raw thread using a clone() call without going +# through the pthread library. Further, the thread that is created +# deliberately sets its TLS pointer to NULL. This used to cause +# problems for GDB as GDB would fail to recognize the new thread, +# which means no thread_info would be created, which would lead to +# segfaults and assertion failures. +# +# When running this test on a remote target we do see some warnings +# from gdbserver about failure to find the raw thread. This is +# because the thread is never owned by thread-db. In a GDB only world +# we don't warn about such threads, we just silently handle them. +# Maybe we should do the same in gdbserver? For now though, we just +# ignore the warnings. Functionally, gdbserver handles the raw thread +# just fine, it just gives a warning each time we try to look it up in +# thread-db (and fail). + +# This only works on targets with the Linux kernel, and the source +# makes use of the clone call, which has a different API on IA64. +require {!istarget "ia64-*-*"} +require {istarget "*-*-linux*"} + +standard_testfile + +# When debugging on a remote target GDB sees the thread as a full +# thread, so we get the 'Thread ...' type message. When using the +# native linux target within GDB, as this thread is not owned by +# thread-db we get a 'LWP ...' type message. Setup a global to be +# used in the test patterns below. +if {[gdb_protocol_is_remote]} { + set raw_thread_re "Thread $::decimal\\.$::decimal" +} else { + set raw_thread_re "LWP $::decimal" +} + +# Restart GDB using TESTFILE, the basename of an executable. Run to +# the function 'do_raw_clone', which is where we start all the tests. +# Then use BREAK_LINE_PATTERN to create a breakpoint on the matching +# line. +proc setup_for_test { testfile break_line_pattern } { + clean_restart $testfile + + # Run until the do_raw_clone function. By this point the pthread + # will have been created. + if {![runto do_raw_clone]} { + return + } + + gdb_breakpoint [gdb_get_line_number $break_line_pattern] +} + +# Use TESTFILE as the basename of the executable to run. Place a +# breakpoint after the creation of a raw thread, in the parent thread, +# not the raw thread. Run until the breakpoint is hit, check we see +# the raw thread announced, and that the thread shows up in the 'info +# threads' output. +proc_with_prefix stop_after_creating_raw_thread { testfile } { + # Start GDB. Run to do_raw_clone. Place a breakpoint after the + # raw thread has been created. + setup_for_test $testfile "Break after raw thread created." + + # Continue until the raw thread has been created. Check we see + # the raw thread announced, and that we hit the expected breakpoint. + set saw_breakpoint_line false + set saw_source_line false + set saw_new_thread_line false + gdb_test_multiple "continue" "" { + -re "^\\\[New $::raw_thread_re \\(id 3\\)\\\]\r\n" { + set saw_new_thread_line true + exp_continue + } + + -re "^Thread 1 \[^\r\n\]+ hit Breakpoint $::decimal, do_raw_clone \\(\\)\[^\r\n\]+\r\n" { + set saw_breakpoint_line true + exp_continue + } + + -re "^$::decimal\\s+\[^\r\n\]+Break after raw thread created\\.\[^\r\n\]+\r\n" { + set saw_source_line true + exp_continue + } + + -re "^$::gdb_prompt $" { + gdb_assert { $saw_breakpoint_line && $saw_source_line \ + && $saw_new_thread_line } $gdb_test_name + } + + -re "^\[^\r\n\]*\r\n" { + exp_continue + } + } + + # Ensure the raw thread shows up in the 'info threads' output. + gdb_test "info threads" \ + "\r\n\\s+3\\s+$::raw_thread_re \[^\r\n\]+ clone_fn \\(arg=$::hex\\) \[^\r\n\]+" + + gdb_test "thread 3" + gdb_test "bt" +} + +# Use TESTFILE as the basename of the executable to run. Place a +# breakpoint within the raw thread function. Run until the breakpoint +# is hit, check we see the raw thread announced, the switch to the raw +# thread, and that the thread shows up in the 'info threads' output. +proc_with_prefix stop_in_raw_thread { testfile } { + setup_for_test $testfile "Break inside raw thread." + + # Continue until the raw thread has been created. Check we see + # the raw thread announced, and that we hit the expected breakpoint. + set saw_breakpoint_line false + set saw_source_line false + set saw_new_thread_line false + set saw_thread_switch false + gdb_test_multiple "continue" "" { + -re "^\\\[New $::raw_thread_re \\(id 3\\)\\\]\r\n" { + set saw_new_thread_line true + exp_continue + } + + -re "^\\\[Switching to $::raw_thread_re\\\]\r\n" { + set saw_thread_switch true + exp_continue + } + + -re "^Thread 3 \[^\r\n\]+ hit Breakpoint $::decimal, clone_fn \\(arg=$::hex\\)\[^\r\n\]+\r\n" { + set saw_breakpoint_line true + exp_continue + } + + -re "^$::decimal\\s+\[^\r\n\]+Break inside raw thread\\.\[^\r\n\]+\r\n" { + set saw_source_line true + exp_continue + } + + -re "^$::gdb_prompt $" { + gdb_assert { $saw_breakpoint_line && $saw_source_line \ + && $saw_thread_switch && $saw_new_thread_line } \ + $gdb_test_name + } + + -re "^\[^\r\n\]*\r\n" { + exp_continue + } + } + + # Ensure the raw thread shows up in the 'info threads' output. + gdb_test "info threads" \ + "\r\n\\*\\s+3\\s+$::raw_thread_re \[^\r\n\]+ clone_fn \\(arg=$::hex\\) \[^\r\n\]+" +} + +# Use TESTFILE as the basename of the executable to run. Break before +# the raw thread is created, set scheduler-locking mode on to stop GDB +# setting the raw thread running, then step (using 'next') over the +# creation of the raw thread. Once we have stepped past the clone +# turn scheduler-locking off again and run until the inferior exits, +# we expect to see both the pthread and the raw thread exit. +# +# In broken GDB, in the delay build (no futex) when the inferior +# exits, the raw thread will also exit. GDB knows nothing about the +# raw thread, and an assertion is triggered. +# +# In broken GDB, in futex mode the inferior will deadlock waiting on +# the futex, this is because GDB doesn't know about the raw thread, +# and never set it running again. As such the futex condition +# variable is never cleared, and so the inferior never exits. +# +# This should all now be fixed, GDB should see the raw thread +# correctly, and the inferior should exit in both modes. +proc_with_prefix next_over_raw_thread_creation { testfile } { + setup_for_test $testfile "Break before raw thread created." + + gdb_continue_to_breakpoint "before raw thread created" + + gdb_test_no_output "set scheduler-locking on" + + set saw_thread_announced false + set reached_line_after_clone false + for { set i 0 } { $i < 10 } { incr i } { + gdb_test_multiple "next" "next, $i" { + -re "^\\\[New $::raw_thread_re \\(id 3\\)\\\]\r\n" { + set saw_thread_announced true + exp_continue + } + + -re "^$::decimal\\s+\[^\r\n\]+Immediately after the clone\\.\[^\r\n\]+\r\n" { + set reached_line_after_clone true + exp_continue + } + + -re "^$::gdb_prompt " { + # Nothing. + } + + -re "^\[^\r\n\]*\r\n" { + exp_continue + } + } + + if { $reached_line_after_clone } { + break + } + } + + gdb_assert { $reached_line_after_clone && $saw_thread_announced } \ + "have passed clone and seen raw thread" + + # Ensure the raw thread shows up in the 'info threads' output. + # Due to scheduler-locking the new thread will not have made any + # progress yet. Having 'clone ()' in the expected output might be + # risky; on some targets it's possible this could show up as some + # other function name. + gdb_test "info threads" \ + "\r\n\\s+3\\s+$::raw_thread_re \[^\r\n\]+ clone \\(\\) \[^\r\n\]+" + + gdb_test_no_output "set scheduler-locking off" + + # Run until the inferior exits. We want to do this because if GDB + # failed to track the raw thread, then when the inferior exits the + # raw thread will also exit, and this thread exit event used to + # trigger an assertion in GDB. + set saw_raw_thread_exit false + set saw_pthread_exit false + set saw_inferior_exit false + gdb_test_multiple "continue" "continue to exit" { + -re "^\\\[$::raw_thread_re \\(id 3\\) exited\\\]\r\n" { + set saw_raw_thread_exit true + exp_continue + } + + -re "^\\\[Thread \[^\r\n\]+ \\(id 2\\) exited\\\]\r\n" { + set saw_pthread_exit true + exp_continue + } + + -re "^\\\[Inferior 1 \[^\r\n\]+ exited normally\\\]\r\n" { + set saw_inferior_exit true + exp_continue + } + + -re "^$::gdb_prompt " { + # Remote targets don't send thread exit events for every + # thread when the entire inferior exits. Maybe GDB itself + # should synthesize these events? For now though just + # invert the state of the thread exit flags when testing + # with a remote protocol. + if {[gdb_protocol_is_remote]} { + set saw_raw_thread_exit [expr {!$saw_raw_thread_exit}] + set saw_pthread_exit [expr {!$saw_pthread_exit}] + } + + gdb_assert { $saw_raw_thread_exit && $saw_pthread_exit \ + && $saw_inferior_exit } $gdb_test_name + } + + -re "^\[^\r\n\]*\r\n" { + exp_continue + } + } +} + + +# Run the tests. +foreach_with_prefix mode { delay futex } { + set the_testfile $testfile-$mode + + set flags {debug pthreads} + if { $mode eq "futex" } { + lappend flags "additional_flags=-DUSE_FUTEX" + } + + if {[build_executable "failed to build" $the_testfile $srcfile $flags]} { + return + } + + stop_after_creating_raw_thread $the_testfile + stop_in_raw_thread $the_testfile + next_over_raw_thread_creation $the_testfile +} diff --git a/gdbserver/thread-db.cc b/gdbserver/thread-db.cc index 0bdcda2ced8..b15d5e472a0 100644 --- a/gdbserver/thread-db.cc +++ b/gdbserver/thread-db.cc @@ -178,14 +178,20 @@ find_one_thread (ptid_t ptid) td_err_e err = thread_db->td_ta_map_lwp2thr_p (thread_db->thread_agent, lwpid, &th); if (err != TD_OK) - error ("Cannot get thread handle for LWP %d: %s", - lwpid, thread_db_err_str (err)); + { + warning ("Cannot get thread handle for LWP %d: %s", + lwpid, thread_db_err_str (err)); + return 0; + } td_thrinfo_t ti; err = thread_db->td_thr_get_info_p (&th, &ti); if (err != TD_OK) - error ("Cannot get thread info for LWP %d: %s", - lwpid, thread_db_err_str (err)); + { + warning ("Cannot get thread info for LWP %d: %s", + lwpid, thread_db_err_str (err)); + return 0; + } threads_debug_printf ("Found thread %ld (LWP %d)", (unsigned long) ti.ti_tid, ti.ti_lid); -- 2.25.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] gdb: improve error handling for thread-db thread adoption 2026-04-07 10:22 ` [PATCH 3/3] gdb: improve error handling for thread-db thread adoption Andrew Burgess @ 2026-05-06 19:39 ` Keith Seitz 0 siblings, 0 replies; 7+ messages in thread From: Keith Seitz @ 2026-05-06 19:39 UTC (permalink / raw) To: Andrew Burgess, gdb-patches Hi, On 4/7/26 3:22 AM, Andrew Burgess wrote: > > We can imagine many bugs that might cause this to occur, some of which > could even be outside GDB. But without additional information I'm not > inclined to start looking for bugs outside of GDB. So I started > looking for ways that the bug could be triggered from within GDB. I don't blame you one iota. :-) > One way the bug could occur is if GDB fails to create a thread_info > for a new thread. Thank you for the detailed explanation! It really helps thread-handling novices like me. > My solution for this is to convert the `error` calls into warnings, > and just have thread_from_lwp return NULL if thread-db cannot adopt > the thread. > > This potentially makes the use of FORWARD_SCOPE_EXIT in ::follow_clone > unnecessary, I no longer expect any exceptions to be thrown from > thread_db_notice_clone. However, I think maintaining the use of > FORWARD_SCOPE_EXIT is still a good idea. If I've missed any exception > routes, or any are added in the future, then this will ensure the > threads are always added to GDB correctly. I don't mind the defensive strategy here > Tested on GNU/Linux on x86-64, AArch64, RISC-V, and PPC64le using > unix, native-gdbserver, and native-extended-gdbserver boards. Did you try -m32 on x86_64? I am seeing a bunch of the new clone-bad-tls.exp tests fail. On native, we hit an assert ("`clone_pid > 0' failed") in the test program. On gdbservers all of the "info thread" commands show the current frame as "__kernel_vsyscall". I discovered no other regressions on architectures supported by Fedora 43 (except ppc64le where gdbserver currently isn't building). One largely rhetorical question below... > diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c > index 1807352ed76..1ae9323108a 100644 > --- a/gdb/linux-thread-db.c > +++ b/gdb/linux-thread-db.c > @@ -413,13 +413,19 @@ thread_from_lwp (thread_info *stopped, ptid_t ptid) > err = info->td_ta_map_lwp2thr_p (info->thread_agent, ptid.lwp (), > &th); > if (err != TD_OK) > - error (_("Cannot find user-level thread for LWP %ld: %s"), > - ptid.lwp (), thread_db_err_str (err)); > + { > + warning (_("Cannot find user-level thread for LWP %ld: %s"), > + ptid.lwp (), thread_db_err_str (err)); > + return nullptr; > + } > > err = info->td_thr_get_info_p (&th, &ti); > if (err != TD_OK) > - error (_("thread_get_info_callback: cannot get thread info: %s"), > - thread_db_err_str (err)); > + { > + warning (_("thread_get_info_callback: cannot get thread info: %s"), > + thread_db_err_str (err)); > + return nullptr; > + } I have to confess, I am a bit apprehensive about these new warnings. From a technical perspective, I certainly understand the issue here and that they can largely be ignored (what other choice is there?). However, I wonder how a "developer just trying to debug some code" would react to seeing these warnings? Mind you, I'm not 100% sure how to improve wording, and I"m not even sure it is worth further effort, given that this should be pretty edge-case-y. So consider this largely rhetorical unless something obvious comes to mind. Keith > > /* Fill the cache. */ > tp = stopped->inf->process_target ()->find_thread (ptid); > @@ -450,8 +456,8 @@ thread_db_notice_clone (ptid_t parent, ptid_t child) > thread_info *parent_info = thread_from_lwp (stopped, parent); > gdb_assert (parent_info == stopped); > > - thread_from_lwp (stopped, child); > - return true; > + thread_info *child_info = thread_from_lwp (stopped, child); > + return child_info != nullptr; > } > > static void * > diff --git a/gdb/testsuite/gdb.threads/clone-bad-tls.c b/gdb/testsuite/gdb.threads/clone-bad-tls.c > new file mode 100644 > index 00000000000..dfb3ff79c17 > --- /dev/null > +++ b/gdb/testsuite/gdb.threads/clone-bad-tls.c > @@ -0,0 +1,193 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2026 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program 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 General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see <http://www.gnu.org/licenses/>. > + > + Test that GDB doesn't lose an event for a thread it didn't know > + about, until an event is reported for it. */ > + > +#define _GNU_SOURCE > +#include <sched.h> > +#include <assert.h> > +#include <stdlib.h> > +#include <sys/types.h> > +#include <sys/wait.h> > +#include <unistd.h> > +#include <pthread.h> > +#include <linux/futex.h> > +#include <sys/syscall.h> > + > +/* Global synchronization variables. */ > +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; > +pthread_cond_t cond = PTHREAD_COND_INITIALIZER; > +int ready = 0; > + > +#define STACK_SIZE 0x1000 > + > +/* Wrap futex syscall. See 'man 2 futex'. */ > + > +static int > +futex (int *uaddr, int futex_op, int val, const struct timespec *timeout, > + int *uaddr2, int val3) > +{ > + return syscall (SYS_futex, uaddr, futex_op, val, timeout, uaddr2, val3); > +} > + > +/* Function to run in the raw clone thread. This just spins until the > + integer flag pointed to by ARG is set to non-zero. */ > + > +static int > +clone_fn (void *arg) > +{ > + int *thread_exit = (int *) arg; /* Break inside raw thread. */ > + *thread_exit = 0; > + > + while (*thread_exit == 0) > + ; > +} > + > +/* A variable into which the kernel will write the child thread-id. This > + will be cleared when the thread exits. This only gets used when > + USE_FUTEX is defined. */ > + > +static int thread_tid; > + > +/* Create a thread using a raw clone call. */ > + > +void * > +do_raw_clone () > +{ > + unsigned char *stack; > + int res; > + int clone_pid; > + > + /* This flag is shared with the raw thread. It is initially -1, then > + set to 0 in the raw thread to show that the thread has started up. > + This flag is then set to 1 by the main thread to indicate that the > + raw thread should exit. Using a volatile for thread synchronisation > + is not great, but this avoids having to make library calls from the > + raw thread, which might trigger a need for TLS to be setup correctly > + on some targets. */ > + volatile int thread_exit = -1; > + > + stack = malloc (STACK_SIZE); > + assert (stack != NULL); /* Break before raw thread created. */ > + > +#ifdef USE_FUTEX > +#define TID_FLAGS (CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID) > +#else > +#define TID_FLAGS (0) > +#endif > + > +#define CLONE_FLAGS (CLONE_THREAD | CLONE_SIGHAND | CLONE_VM \ > + | CLONE_SETTLS | TID_FLAGS) > + > + clone_pid = clone (clone_fn, stack + STACK_SIZE, CLONE_FLAGS, > + (void *) &thread_exit, &thread_tid, > + NULL, &thread_tid); > + > + assert (clone_pid > 0); /* Immediately after the clone. */ > + > + while (thread_exit != 0) > + usleep (1000); > + > + /* Trigger the clone thread to exit. */ > + thread_exit = 1; /* Break after raw thread created. */ > + > +#ifdef USE_FUTEX > + /* In this mode we rely on the futex to notify us when the thread has > + exited. In a broken GDB we used to deadlock at this point as GDB > + would fail to restart the raw thread, and so the kernel would never > + wake this futex. */ > + while (thread_tid != 0) > + futex (&thread_tid, FUTEX_WAIT, clone_pid, NULL, NULL, 0); > + > + /* We do know that the raw thread has completed at this point, so we > + could free its stack. We don't though, just to be consistent with the > + no futex path. This doesn't really matter, this is just a small > + test. */ > +#else > + /* There's no attempt to synchronise with the raw clone thread on this > + path. In a broken GDB the raw thread might not be resumed, in which > + case this timeout will expire. */ > + sleep (2); > + > + /* We cannot be sure that the raw thread has finished by this point, so > + we cannot free the stack. This isn't critical, this is just a small > + test case. */ > +#endif > + > + return NULL; /* Break after raw thread exits. */ > +} > + > +/* Something for our pthread thread to do. This just blocks until the main > + thread releases it, at which point this thread will exit. */ > + > +void * > +worker_thread (void *arg) > +{ > + pthread_mutex_lock(&mutex); > + > + /* Let the main thread know we are live by clearing the ready flag. */ > + ready = 0; > + pthread_cond_signal(&cond); > + > + /* Now spin until the main thread sets this flag back to non-zero. */ > + while (ready == 0) > + { > + /* The thread will block here until main signals 'cond' */ > + pthread_cond_wait (&cond, &mutex); > + } > + > + pthread_mutex_unlock (&mutex); > + > + return NULL; > +} > + > +int > +main (void) > +{ > + pthread_t thread_id; > + volatile int flag = 0; > + int res; > + > + /* The pthread will set the READY flag back to zero. */ > + ready = 1; > + > + alarm (300); > + > + res = pthread_create (&thread_id, NULL, worker_thread, (void *)&flag); > + assert (res == 0); > + > + /* Wait for the pthread to set READY to zero. */ > + pthread_mutex_lock(&mutex); > + while (ready == 1) > + pthread_cond_wait (&cond, &mutex); > + pthread_mutex_unlock (&mutex); > + > + /* Create a thread using a raw clone call. */ > + do_raw_clone (); /* Break before do_raw_clone. */ > + > + /* Unblock the pthread. */ > + pthread_mutex_lock (&mutex); > + ready = 1; > + pthread_cond_signal (&cond); > + pthread_mutex_unlock (&mutex); > + > + /* Wait for the worker to finish. */ > + pthread_join (thread_id, NULL); > + > + return 0; > +} > diff --git a/gdb/testsuite/gdb.threads/clone-bad-tls.exp b/gdb/testsuite/gdb.threads/clone-bad-tls.exp > new file mode 100644 > index 00000000000..2de16c63b6e > --- /dev/null > +++ b/gdb/testsuite/gdb.threads/clone-bad-tls.exp > @@ -0,0 +1,294 @@ > +# This testcase is part of GDB, the GNU debugger. > + > +# Copyright 2026 Free Software Foundation, Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program 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 General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +# This test creates a raw thread using a clone() call without going > +# through the pthread library. Further, the thread that is created > +# deliberately sets its TLS pointer to NULL. This used to cause > +# problems for GDB as GDB would fail to recognize the new thread, > +# which means no thread_info would be created, which would lead to > +# segfaults and assertion failures. > +# > +# When running this test on a remote target we do see some warnings > +# from gdbserver about failure to find the raw thread. This is > +# because the thread is never owned by thread-db. In a GDB only world > +# we don't warn about such threads, we just silently handle them. > +# Maybe we should do the same in gdbserver? For now though, we just > +# ignore the warnings. Functionally, gdbserver handles the raw thread > +# just fine, it just gives a warning each time we try to look it up in > +# thread-db (and fail). > + > +# This only works on targets with the Linux kernel, and the source > +# makes use of the clone call, which has a different API on IA64. > +require {!istarget "ia64-*-*"} > +require {istarget "*-*-linux*"} > + > +standard_testfile > + > +# When debugging on a remote target GDB sees the thread as a full > +# thread, so we get the 'Thread ...' type message. When using the > +# native linux target within GDB, as this thread is not owned by > +# thread-db we get a 'LWP ...' type message. Setup a global to be > +# used in the test patterns below. > +if {[gdb_protocol_is_remote]} { > + set raw_thread_re "Thread $::decimal\\.$::decimal" > +} else { > + set raw_thread_re "LWP $::decimal" > +} > + > +# Restart GDB using TESTFILE, the basename of an executable. Run to > +# the function 'do_raw_clone', which is where we start all the tests. > +# Then use BREAK_LINE_PATTERN to create a breakpoint on the matching > +# line. > +proc setup_for_test { testfile break_line_pattern } { > + clean_restart $testfile > + > + # Run until the do_raw_clone function. By this point the pthread > + # will have been created. > + if {![runto do_raw_clone]} { > + return > + } > + > + gdb_breakpoint [gdb_get_line_number $break_line_pattern] > +} > + > +# Use TESTFILE as the basename of the executable to run. Place a > +# breakpoint after the creation of a raw thread, in the parent thread, > +# not the raw thread. Run until the breakpoint is hit, check we see > +# the raw thread announced, and that the thread shows up in the 'info > +# threads' output. > +proc_with_prefix stop_after_creating_raw_thread { testfile } { > + # Start GDB. Run to do_raw_clone. Place a breakpoint after the > + # raw thread has been created. > + setup_for_test $testfile "Break after raw thread created." > + > + # Continue until the raw thread has been created. Check we see > + # the raw thread announced, and that we hit the expected breakpoint. > + set saw_breakpoint_line false > + set saw_source_line false > + set saw_new_thread_line false > + gdb_test_multiple "continue" "" { > + -re "^\\\[New $::raw_thread_re \\(id 3\\)\\\]\r\n" { > + set saw_new_thread_line true > + exp_continue > + } > + > + -re "^Thread 1 \[^\r\n\]+ hit Breakpoint $::decimal, do_raw_clone \\(\\)\[^\r\n\]+\r\n" { > + set saw_breakpoint_line true > + exp_continue > + } > + > + -re "^$::decimal\\s+\[^\r\n\]+Break after raw thread created\\.\[^\r\n\]+\r\n" { > + set saw_source_line true > + exp_continue > + } > + > + -re "^$::gdb_prompt $" { > + gdb_assert { $saw_breakpoint_line && $saw_source_line \ > + && $saw_new_thread_line } $gdb_test_name > + } > + > + -re "^\[^\r\n\]*\r\n" { > + exp_continue > + } > + } > + > + # Ensure the raw thread shows up in the 'info threads' output. > + gdb_test "info threads" \ > + "\r\n\\s+3\\s+$::raw_thread_re \[^\r\n\]+ clone_fn \\(arg=$::hex\\) \[^\r\n\]+" > + > + gdb_test "thread 3" > + gdb_test "bt" > +} > + > +# Use TESTFILE as the basename of the executable to run. Place a > +# breakpoint within the raw thread function. Run until the breakpoint > +# is hit, check we see the raw thread announced, the switch to the raw > +# thread, and that the thread shows up in the 'info threads' output. > +proc_with_prefix stop_in_raw_thread { testfile } { > + setup_for_test $testfile "Break inside raw thread." > + > + # Continue until the raw thread has been created. Check we see > + # the raw thread announced, and that we hit the expected breakpoint. > + set saw_breakpoint_line false > + set saw_source_line false > + set saw_new_thread_line false > + set saw_thread_switch false > + gdb_test_multiple "continue" "" { > + -re "^\\\[New $::raw_thread_re \\(id 3\\)\\\]\r\n" { > + set saw_new_thread_line true > + exp_continue > + } > + > + -re "^\\\[Switching to $::raw_thread_re\\\]\r\n" { > + set saw_thread_switch true > + exp_continue > + } > + > + -re "^Thread 3 \[^\r\n\]+ hit Breakpoint $::decimal, clone_fn \\(arg=$::hex\\)\[^\r\n\]+\r\n" { > + set saw_breakpoint_line true > + exp_continue > + } > + > + -re "^$::decimal\\s+\[^\r\n\]+Break inside raw thread\\.\[^\r\n\]+\r\n" { > + set saw_source_line true > + exp_continue > + } > + > + -re "^$::gdb_prompt $" { > + gdb_assert { $saw_breakpoint_line && $saw_source_line \ > + && $saw_thread_switch && $saw_new_thread_line } \ > + $gdb_test_name > + } > + > + -re "^\[^\r\n\]*\r\n" { > + exp_continue > + } > + } > + > + # Ensure the raw thread shows up in the 'info threads' output. > + gdb_test "info threads" \ > + "\r\n\\*\\s+3\\s+$::raw_thread_re \[^\r\n\]+ clone_fn \\(arg=$::hex\\) \[^\r\n\]+" > +} > + > +# Use TESTFILE as the basename of the executable to run. Break before > +# the raw thread is created, set scheduler-locking mode on to stop GDB > +# setting the raw thread running, then step (using 'next') over the > +# creation of the raw thread. Once we have stepped past the clone > +# turn scheduler-locking off again and run until the inferior exits, > +# we expect to see both the pthread and the raw thread exit. > +# > +# In broken GDB, in the delay build (no futex) when the inferior > +# exits, the raw thread will also exit. GDB knows nothing about the > +# raw thread, and an assertion is triggered. > +# > +# In broken GDB, in futex mode the inferior will deadlock waiting on > +# the futex, this is because GDB doesn't know about the raw thread, > +# and never set it running again. As such the futex condition > +# variable is never cleared, and so the inferior never exits. > +# > +# This should all now be fixed, GDB should see the raw thread > +# correctly, and the inferior should exit in both modes. > +proc_with_prefix next_over_raw_thread_creation { testfile } { > + setup_for_test $testfile "Break before raw thread created." > + > + gdb_continue_to_breakpoint "before raw thread created" > + > + gdb_test_no_output "set scheduler-locking on" > + > + set saw_thread_announced false > + set reached_line_after_clone false > + for { set i 0 } { $i < 10 } { incr i } { > + gdb_test_multiple "next" "next, $i" { > + -re "^\\\[New $::raw_thread_re \\(id 3\\)\\\]\r\n" { > + set saw_thread_announced true > + exp_continue > + } > + > + -re "^$::decimal\\s+\[^\r\n\]+Immediately after the clone\\.\[^\r\n\]+\r\n" { > + set reached_line_after_clone true > + exp_continue > + } > + > + -re "^$::gdb_prompt " { > + # Nothing. > + } > + > + -re "^\[^\r\n\]*\r\n" { > + exp_continue > + } > + } > + > + if { $reached_line_after_clone } { > + break > + } > + } > + > + gdb_assert { $reached_line_after_clone && $saw_thread_announced } \ > + "have passed clone and seen raw thread" > + > + # Ensure the raw thread shows up in the 'info threads' output. > + # Due to scheduler-locking the new thread will not have made any > + # progress yet. Having 'clone ()' in the expected output might be > + # risky; on some targets it's possible this could show up as some > + # other function name. > + gdb_test "info threads" \ > + "\r\n\\s+3\\s+$::raw_thread_re \[^\r\n\]+ clone \\(\\) \[^\r\n\]+" > + > + gdb_test_no_output "set scheduler-locking off" > + > + # Run until the inferior exits. We want to do this because if GDB > + # failed to track the raw thread, then when the inferior exits the > + # raw thread will also exit, and this thread exit event used to > + # trigger an assertion in GDB. > + set saw_raw_thread_exit false > + set saw_pthread_exit false > + set saw_inferior_exit false > + gdb_test_multiple "continue" "continue to exit" { > + -re "^\\\[$::raw_thread_re \\(id 3\\) exited\\\]\r\n" { > + set saw_raw_thread_exit true > + exp_continue > + } > + > + -re "^\\\[Thread \[^\r\n\]+ \\(id 2\\) exited\\\]\r\n" { > + set saw_pthread_exit true > + exp_continue > + } > + > + -re "^\\\[Inferior 1 \[^\r\n\]+ exited normally\\\]\r\n" { > + set saw_inferior_exit true > + exp_continue > + } > + > + -re "^$::gdb_prompt " { > + # Remote targets don't send thread exit events for every > + # thread when the entire inferior exits. Maybe GDB itself > + # should synthesize these events? For now though just > + # invert the state of the thread exit flags when testing > + # with a remote protocol. > + if {[gdb_protocol_is_remote]} { > + set saw_raw_thread_exit [expr {!$saw_raw_thread_exit}] > + set saw_pthread_exit [expr {!$saw_pthread_exit}] > + } > + > + gdb_assert { $saw_raw_thread_exit && $saw_pthread_exit \ > + && $saw_inferior_exit } $gdb_test_name > + } > + > + -re "^\[^\r\n\]*\r\n" { > + exp_continue > + } > + } > +} > + > + > +# Run the tests. > +foreach_with_prefix mode { delay futex } { > + set the_testfile $testfile-$mode > + > + set flags {debug pthreads} > + if { $mode eq "futex" } { > + lappend flags "additional_flags=-DUSE_FUTEX" > + } > + > + if {[build_executable "failed to build" $the_testfile $srcfile $flags]} { > + return > + } > + > + stop_after_creating_raw_thread $the_testfile > + stop_in_raw_thread $the_testfile > + next_over_raw_thread_creation $the_testfile > +} > diff --git a/gdbserver/thread-db.cc b/gdbserver/thread-db.cc > index 0bdcda2ced8..b15d5e472a0 100644 > --- a/gdbserver/thread-db.cc > +++ b/gdbserver/thread-db.cc > @@ -178,14 +178,20 @@ find_one_thread (ptid_t ptid) > td_err_e err = thread_db->td_ta_map_lwp2thr_p (thread_db->thread_agent, lwpid, > &th); > if (err != TD_OK) > - error ("Cannot get thread handle for LWP %d: %s", > - lwpid, thread_db_err_str (err)); > + { > + warning ("Cannot get thread handle for LWP %d: %s", > + lwpid, thread_db_err_str (err)); > + return 0; > + } > > td_thrinfo_t ti; > err = thread_db->td_thr_get_info_p (&th, &ti); > if (err != TD_OK) > - error ("Cannot get thread info for LWP %d: %s", > - lwpid, thread_db_err_str (err)); > + { > + warning ("Cannot get thread info for LWP %d: %s", > + lwpid, thread_db_err_str (err)); > + return 0; > + } > > threads_debug_printf ("Found thread %ld (LWP %d)", > (unsigned long) ti.ti_tid, ti.ti_lid); ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-06 19:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-04-07 10:22 [PATCH 0/3] Better error handling when creating threads with clone() Andrew Burgess 2026-04-07 10:22 ` [PATCH 1/3] gdb: int to bool conversion in linux-thread-db.c Andrew Burgess 2026-05-05 18:19 ` Keith Seitz 2026-04-07 10:22 ` [PATCH 2/3] gdb: add some asserts to thread_db_notice_clone Andrew Burgess 2026-05-05 18:24 ` Keith Seitz 2026-04-07 10:22 ` [PATCH 3/3] gdb: improve error handling for thread-db thread adoption Andrew Burgess 2026-05-06 19:39 ` Keith Seitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox