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

[Amos Bird <amosbird@gmail.com>]

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


Updated. Thanks!

Pedro Alves <palves@redhat.com> writes:

> On 5/17/19 6:14 PM, Amos Bird wrote:
>>  } 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"
>
> That lost the tab before "-re" -- was that on purpose?
>
>
>>  	}
>>      }
>> @@ -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"
>> +		}
>> +    }
>
> This should use the same indentation levels as the similar code 
> above.
>
> Also, this isn't expecting the prompt.  Is there a reason for 
> that?
> Not matching/expecting the prompt is usually not desired, 
> because it
> leaves the prompt in the expect buffer, which can confuse 
> following
> tests.
>
> Also, this causes some regressions:
>
>  Running src/gdb/testsuite/gdb.cp/annota2.exp ...
>  FAIL: gdb.cp/annota2.exp: continue until exit (timeout)
>  FAIL: gdb.cp/annota2.exp: delete bps
>  FAIL: gdb.cp/annota2.exp: break at main (got interactive 
>  prompt)
>
> Please take a look at those and make sure to run the whole
> testsuite (I only ran the "gdb.*/annota*.exp" tests).
>
> 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: 8271 bytes --]

From 68d708fb2f25d3c635be0429b54792970313ac9d Mon Sep 17 00:00:00 2001
From: Amos Bird <amosbird@gmail.com>
Date: Fri, 7 Jun 2019 01:10:14 +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 +++++++++++++++++------
 gdb/testsuite/gdb.cp/annota2.exp          |  2 ++
 9 files changed, 70 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..dfa30831e6 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}\".*\r\n$gdb_prompt$" {
+		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);
 }
diff --git a/gdb/testsuite/gdb.cp/annota2.exp b/gdb/testsuite/gdb.cp/annota2.exp
index 982020a11e..745f461716 100644
--- a/gdb/testsuite/gdb.cp/annota2.exp
+++ b/gdb/testsuite/gdb.cp/annota2.exp
@@ -138,6 +138,8 @@ set pat [multi_line "" \
 	     "\032\032exited 0" \
 	     "$inferior_exited_re normally." \
 	     "" \
+	     "\032\032thread-exited,id=\"1\",group-id=\"i1\"" \
+	     "" \
 	     "\032\032stopped" \
 	     $gdb_prompt$]
 gdb_test_multiple "continue" "continue until exit" {
-- 
2.21.0