Re: [PATCH] Add "thread-exited" annotation

[Amos Bird <amosbird@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1977 bytes --]


Hi Pedro,

Thanks for the suggestion. Patch updated.

Amos

Pedro Alves <palves@redhat.com> writes:

> On 5/17/19 4:51 PM, Amos Bird wrote:
>> +proc thread_exit {} {
>> +    global decimal
>> +    gdb_test_multiple "call (void)pthread_exit()" "thread 
>> exit" {
>
> Since this is calling a function, it would need to be gated
> with a gdb,cannot_call_functions check (grep for uses).
>
> Does that call return?  If the thread exits, the "call" command 
> will
> continue running without returning to the prompt, right?
>
> I guess that's why you don't expect the prompt here:
>
>> +    gdb_test_multiple "call (void)pthread_exit()" "thread 
>> exit" {
>> +	-re 
>> ".*\032\032thread-exited,id=\"${decimal}\",group-id=\"i${decimal}\"" 
>> {
>> +	    pass "thread exit"
>> +	}
>> +    }
>> +}
>
> Note that this approach means that if someone were to add a test
> after this one, we'd need to either ctrl-c the program, or 
> restart
> gdb.
>
> Please test this with
>
>  make check RUNTESTFLAGS="--target_board=native-gdbserver"
>
> too.  In that case, you're debugging with gdbserver, and 
> gdb/gdbserver
> won't notice that the thread exited until the program next 
> stops.
>
> Also, you probably don't have debug info for glibc/libpthread in 
> your
> system.  I say this because pthread_exit takes an argument and 
> above
> you're not passing any, resulting in undefined behavior.
>
> I'd be much better if the .c file were tweaked to make the 
> thread
> exit on its own, or a new .c file were added for that purpose, 
> IMO.
> E.g., make the main thread join a child thread and hit a 
> breakpoint
> once the thread exits and the join unlocks.
>
>> +	-re 
>> ".*\032\032thread-exited,id=\"${decimal}\",group-id=\"i${decimal}\"" 
>> {
>> +	    pass "thread exit"
>> +	}
>> +    }
>> +}
>> +
>>  thread_test
>>  thread_switch
>> +thread_exit
>>
>>  # restore the original prompt for the rest of the testsuite
>
> Thanks,
> Pedro Alves


--
Amos Bird
amosbird@gmail.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-thread-exit-annotation.patch --]
[-- Type: text/x-patch, Size: 7706 bytes --]

From 8fc6ecff07b2202f6d89802ee7686ee373732e1c Mon Sep 17 00:00:00 2001
From: Amos Bird <amosbird@gmail.com>
Date: Fri, 17 May 2019 21:23:09 +0800
Subject: [PATCH] Add "thread-exit" annotation.

---
 gdb/ChangeLog                             |  5 +++++
 gdb/NEWS                                  |  2 ++
 gdb/annotate.c                            | 14 +++++++++++++
 gdb/doc/ChangeLog                         |  5 +++++
 gdb/doc/annotate.texinfo                  |  7 +++++++
 gdb/testsuite/ChangeLog                   |  5 +++++
 gdb/testsuite/gdb.base/annota1.exp        | 12 ++++++++++-
 gdb/testsuite/gdb.base/watch_thread_num.c | 25 +++++++++++++++++------
 8 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e0120e7743..849ed30731 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2019-04-26  Amos Bird  <amosbird@gmail.com>
+
+	* annotate.c (annotate_thread_exited): Add "thread-exited"
+	annotation.
+
 2019-04-25  Keith Seitz  <keiths@redhat.com>
 
 	PR c++/24367
diff --git a/gdb/NEWS b/gdb/NEWS
index 5309a8f923..735d915ec6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,8 @@
 
 *** Changes since GDB 8.3
 
+* 'thread-exited' event is now available in the annotations interface.
+
 * New built-in convenience variables $_gdb_major and $_gdb_minor
   provide the GDB version.  They are handy for conditionally using
   features available only in or since specific GDB versions, in
diff --git a/gdb/annotate.c b/gdb/annotate.c
index 97cb4c8855..af804ddd1f 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -241,6 +241,19 @@ annotate_thread_changed (void)
     }
 }
 
+/* Emit notification on thread exit.  */
+
+static void
+annotate_thread_exited (struct thread_info *t, int silent)
+{
+  if (annotation_level > 1)
+    {
+      printf_filtered(("\n\032\032thread-exited,"
+                       "id=\"%d\",group-id=\"i%d\"\n"),
+                      t->global_num, t->inf->num);
+    }
+}
+
 void
 annotate_field_begin (struct type *type)
 {
@@ -595,4 +608,5 @@ _initialize_annotate (void)
   gdb::observers::breakpoint_created.attach (breakpoint_changed);
   gdb::observers::breakpoint_deleted.attach (breakpoint_changed);
   gdb::observers::breakpoint_modified.attach (breakpoint_changed);
+  gdb::observers::thread_exit.attach (annotate_thread_exited);
 }
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index ba152329d7..79b837aac6 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2019-04-26  Amos Bird  <amosbird@gmail.com>
+
+	* annotate.texinfo (Multi-threaded Apps): Add entry for thread-exited
+	annotation.
+
 2019-04-22  Pedro Alves  <palves@redhat.com>
 
 	* gdb.texinfo (Reverse Execution): Mention and xref process record
diff --git a/gdb/doc/annotate.texinfo b/gdb/doc/annotate.texinfo
index b85b759f9a..ee8147f52f 100644
--- a/gdb/doc/annotate.texinfo
+++ b/gdb/doc/annotate.texinfo
@@ -836,6 +836,13 @@ The selected thread has changed.  This may occur at the request of the
 user with the @code{thread} command, or as a result of execution,
 e.g., another thread hits a breakpoint.
 
+@findex thread-exited@r{, annotation}
+@item ^Z^Zthread-exited,id="@var{id}",group-id="@var{gid}"
+
+This annotation is issued once for each thread that exits.  The @var{id}
+field contains the global @value{GDBN} identifier of the thread.  The
+@var{gid} field identifies the thread group this thread belongs to.
+
 @end table
 
 @node GNU Free Documentation License
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 9b0725a033..d7a9b5611d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-04-26  Amos Bird  <amosbird@gmail.com>
+
+	* gdb.base/annota1.exp (thread_switch): Add test for
+	thread-exited annotation.
+
 2019-04-25  Keith Seitz  <keiths@redhat.com>
 
 	PR c++/24367
diff --git a/gdb/testsuite/gdb.base/annota1.exp b/gdb/testsuite/gdb.base/annota1.exp
index 5237bc9715..4e4e8439fb 100644
--- a/gdb/testsuite/gdb.base/annota1.exp
+++ b/gdb/testsuite/gdb.base/annota1.exp
@@ -428,7 +428,7 @@ if [target_info exists gdb,nosignals] {
     unsupported "signal sent"
 } else {
     gdb_test_multiple "signal SIGTRAP" "signal sent" {
-	-re ".*\032\032post-prompt\r\nContinuing with signal SIGTRAP.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n\r\n\032\032signalled\r\n\r\nProgram terminated with signal \r\n\032\032signal-name\r\nSIGTRAP\r\n\032\032signal-name-end\r\n, \r\n\032\032signal-string\r\nTrace.breakpoint trap\r\n\032\032signal-string-end\r\n.\r\nThe program no longer exists.\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
+-re ".*\032\032post-prompt\r\nContinuing with signal SIGTRAP.\r\n\r\n\032\032starting\(\r\n\r\n\032\032frames-invalid\)+\r\n\r\n\032\032signalled\r\n\r\nProgram terminated with signal \r\n\032\032signal-name\r\nSIGTRAP\r\n\032\032signal-name-end\r\n, \r\n\032\032signal-string\r\nTrace.breakpoint trap\r\n\032\032signal-string-end\r\n.\r\nThe program no longer exists.\r\n\r\n\032\032thread-exited,id=\"${decimal}\",group-id=\"i${decimal}\"\r\n\r\n\032\032stopped\r\n$gdb_prompt$" {
 	    pass "signal sent"
 	}
     }
@@ -450,6 +450,7 @@ if { [remote_file host exists core] } {
 proc thread_test {} {
     global subdir srcdir testfile srcfile binfile
     global gdb_prompt old_gdb_prompt
+    global decimal
     set srcfile watch_thread_num.c
     set binfile [standard_output_file ${testfile}-watch_thread_num]
     set gdb_prompt $old_gdb_prompt
@@ -468,6 +469,9 @@ proc thread_test {} {
 	set linenum [gdb_get_line_number "all threads started"]
 	gdb_breakpoint "$linenum"
 
+	set linenum [gdb_get_line_number "first child thread exited"]
+	gdb_breakpoint "$linenum"
+
 	set gdb_prompt \
 	    "\r\n\032\032pre-prompt\r\n$gdb_prompt \r\n\032\032prompt\r\n"
 
@@ -481,6 +485,12 @@ proc thread_test {} {
 		pass "new thread"
 	    }
 	}
+
+    gdb_test_multiple "continue" "thread exit" {
+		-re "\032\032thread-exited,id=\"${decimal}\",group-id=\"i${decimal}\"" {
+			pass "thread exit"
+		}
+    }
     }
 }
 
diff --git a/gdb/testsuite/gdb.base/watch_thread_num.c b/gdb/testsuite/gdb.base/watch_thread_num.c
index a5195c71f6..fe9d602476 100644
--- a/gdb/testsuite/gdb.base/watch_thread_num.c
+++ b/gdb/testsuite/gdb.base/watch_thread_num.c
@@ -29,6 +29,8 @@ void *thread_function (void *arg); /* Pointer to function executed by each threa
 
 static pthread_barrier_t threads_started_barrier;
 
+static pthread_barrier_t threads_started_barrier2;
+
 #define NUM 15
 
 static int num_threads = NUM;
@@ -43,6 +45,8 @@ int main () {
 
     pthread_barrier_init (&threads_started_barrier, NULL, NUM + 1);
 
+    pthread_barrier_init (&threads_started_barrier2, NULL, 2);
+
     for (i = 0; i < NUM; i++)
       {
         res = pthread_create (&threads[i],
@@ -53,7 +57,11 @@ int main () {
 
     pthread_barrier_wait (&threads_started_barrier);
 
-    sleep (180); /* all threads started */
+    pthread_barrier_wait (&threads_started_barrier2);  /* all threads started */
+
+    pthread_join (threads[0], NULL);
+
+    sleep (180);  /* first child thread exited */
 
     exit (EXIT_SUCCESS);
 }
@@ -68,13 +76,18 @@ void *thread_function (void *arg) {
 
     pthread_barrier_wait (&threads_started_barrier);
 
-    /* Don't run forever.  Run just short of it :)  */
-    while (shared_var > 0)
+    if (my_number > 0)
       {
-        shared_var++;
-	usleep (1); /* Loop increment.  */
-	loop ();
+	/* Don't run forever.  Run just short of it :)  */
+	while (shared_var > 0)
+	  {
+	    shared_var++;
+	    usleep (1); /* Loop increment.  */
+	    loop ();
+	  }
       }
+    else
+      pthread_barrier_wait (&threads_started_barrier2);
 
     pthread_exit (NULL);
 }
-- 
2.21.0