public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@palves.net>
To: gdb-patches@sourceware.org
Subject: [PATCH 02/11] Fix gdb.threads/clone-new-thread-event.exp race
Date: Thu,  3 Mar 2022 14:40:11 +0000	[thread overview]
Message-ID: <20220303144020.3601082-3-pedro@palves.net> (raw)
In-Reply-To: <20220303144020.3601082-1-pedro@palves.net>

If we make GDB report the process EXIT event for the leader thread,
instead of whatever is the last thread in the LWP list, as will be
done in a latter patch of this series, then
gdb.threads/current-lwp-dead.exp starts failing:

 (gdb) FAIL: gdb.threads/clone-new-thread-event.exp: catch SIGUSR1 (the program exited)

This is a testcase race -- the main thread does not wait for the
spawned clone "thread" to finish before exiting, so the main program
may exit before the second thread is scheduled and reports its
SIGUSR1.  With the change to make GDB report the EXIT for the leader,
the race is 100% reproducible by adding a sleep(), like so:

 --- c/gdb/testsuite/gdb.threads/clone-new-thread-event.c
 +++ w/gdb/testsuite/gdb.threads/clone-new-thread-event.c
 @@ -51,6 +51,7 @@ local_gettid (void)
  static int
  fn (void *unused)
  {
 +  sleep (1);
    tkill (local_gettid (), SIGUSR1);
    return 0;
  }

Resulting in:

 Breakpoint 1, main (argc=1, argv=0x7fffffffd418) at gdb.threads/clone-new-thread-event.c:65
 65        stack = malloc (STACK_SIZE);
 (gdb) continue
 Continuing.
 [New LWP 3715562]
 [Inferior 1 (process 3715555) exited normally]
 (gdb) FAIL: gdb.threads/clone-new-thread-event.exp: catch SIGUSR1 (the program exited)

That inferior exit reported is actually correct.  The main thread has
indeed exited, and that's the thread that has the right exit code to
report to the user, as that's the exit code that is reported to the
program's parent.  In this case, GDB managed to collect the exit code
for the leader thread before reaping the other thread, because in
reality, the testcase isn't creating standard threads, it is using raw
clone, and the new clones are put in their own thread group.

Fix it by making the main thread wait for the child to exit.  Also,
run the program to completion for completeness.

Change-Id: I315cd3dc2b9e860395dcab9658341ea868d7a6bf
---
 .../gdb.threads/clone-new-thread-event.c         | 16 +++++++++++++++-
 .../gdb.threads/clone-new-thread-event.exp       |  2 ++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.threads/clone-new-thread-event.c b/gdb/testsuite/gdb.threads/clone-new-thread-event.c
index 800514d0b6d..9e7ceb97a65 100644
--- a/gdb/testsuite/gdb.threads/clone-new-thread-event.c
+++ b/gdb/testsuite/gdb.threads/clone-new-thread-event.c
@@ -26,6 +26,7 @@
 #include <sys/types.h>
 #include <unistd.h>
 #include <sys/syscall.h>
+#include <sys/wait.h>
 
 #include <features.h>
 #ifdef __UCLIBC__
@@ -59,7 +60,7 @@ int
 main (int argc, char **argv)
 {
   unsigned char *stack;
-  int new_pid;
+  int new_pid, status, ret;
 
   stack = malloc (STACK_SIZE);
   assert (stack != NULL);
@@ -71,5 +72,18 @@ main (int argc, char **argv)
 		   , NULL, NULL, NULL, NULL);
   assert (new_pid > 0);
 
+  /* Note the clone call above didn't use CLONE_THREAD, so it actually
+     put the new thread in a new thread group.  However, the new clone
+     is still reported with PTRACE_EVENT_CLONE to GDB, since we didn't
+     use CLONE_VFORK (results in PTRACE_EVENT_VFORK) nor set the
+     termination signal to SIGCHLD (results in PTRACE_EVENT_FORK), so
+     GDB thinks of it as a new thread of the same inferior.  It's a
+     bit of an odd setup, but it's not important for what we're
+     testing, and, it let's us conveniently use waitpid to wait for
+     the clone, which you can't with CLONE_THREAD.  */
+  ret = waitpid (new_pid, &status, __WALL);
+  assert (ret == new_pid);
+  assert (WIFSIGNALED (status) && WTERMSIG (status) == SIGUSR1);
+
   return 0;
 }
diff --git a/gdb/testsuite/gdb.threads/clone-new-thread-event.exp b/gdb/testsuite/gdb.threads/clone-new-thread-event.exp
index fdebd488d8a..db5ae39aded 100644
--- a/gdb/testsuite/gdb.threads/clone-new-thread-event.exp
+++ b/gdb/testsuite/gdb.threads/clone-new-thread-event.exp
@@ -31,3 +31,5 @@ if { ![runto_main] } {
 gdb_test "continue" \
     "Thread 2 received signal SIGUSR1, User defined signal 1.*" \
     "catch SIGUSR1"
+
+gdb_continue_to_end "" continue 1
-- 
2.26.2


  parent reply	other threads:[~2022-03-03 14:40 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-03 14:40 [PATCH 00/11] Linux: Fix issues around thread group leader exits Pedro Alves
2022-03-03 14:40 ` [PATCH 01/11] Fix gdbserver/linux target_waitstatus logging assert Pedro Alves
2022-03-03 14:40 ` Pedro Alves [this message]
2022-03-03 14:40 ` [PATCH 03/11] Fix gdb.threads/current-lwp-dead.exp race Pedro Alves
2022-03-03 14:40 ` [PATCH 04/11] gdb: Reorganize linux_nat_filter_event Pedro Alves
2022-03-03 14:40 ` [PATCH 05/11] gdbserver: Reorganize linux_process_target::filter_event Pedro Alves
2022-03-03 14:40 ` [PATCH 06/11] gdbserver: Reindent check_zombie_leaders Pedro Alves
2022-03-03 14:40 ` [PATCH 07/11] Re-add zombie leader on exit, gdb/linux Pedro Alves
2022-03-07 20:08   ` Simon Marchi
2022-03-07 20:27     ` Pedro Alves
2022-03-07 20:31       ` Simon Marchi
2022-03-09 14:37         ` Pedro Alves
2022-03-03 14:40 ` [PATCH 08/11] Re-add zombie leader on exit, gdbserver/linux Pedro Alves
2022-03-03 14:40 ` [PATCH 09/11] Ensure EXIT is last event, gdb/linux Pedro Alves
2022-03-07 20:24   ` Simon Marchi
2022-03-09  0:21     ` Lancelot SIX
2022-03-09 14:45       ` Pedro Alves
2022-03-09 22:29         ` Lancelot SIX
2022-03-10 11:46           ` Pedro Alves
2022-03-03 14:40 ` [PATCH 10/11] Ensure EXIT is last event, gdbserver/linux Pedro Alves
2022-03-03 14:40 ` [PATCH 11/11] Process exit status is leader exit status testcase Pedro Alves
2023-06-22 11:28   ` Ilya Leoshkevich
2023-06-22 13:07     ` Tom de Vries

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220303144020.3601082-3-pedro@palves.net \
    --to=pedro@palves.net \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).