diff mbox

Add "thread-exited" annotation

Message ID 87pnohcc5t.fsf@gmail.com
State New
Headers show

Commit Message

Amos Bird May 17, 2019, 3:51 p.m. UTC
Fixed and updated patch. Thanks!

Simon Marchi <simark@simark.ca> writes:

> On 2019-05-17 9:24 a.m., Amos Bird wrote:
>>
>> Hi Simon,
>>
>>> I don't mind if we start using MI-like syntax for annotation 
>>> parameters, but it should
>>> be a conscious decision to do so.
>>
>> To be honest I don't use the extra information at all in cgdb, 
>> only "thread-exited". So
>> I'm OK to whatever format it ends up to have.
>>
>> Patch updated, thanks!
>
> The test fails here, please verify that the it passes using:
>
>   gdb/ $ make check TESTS="gdb.base/annota1.exp"
>
> You'll need at least to declare "global decimal" to be able to 
> access the decimal variable.
> When adding that, I get a FAIL that I don't get without the 
> patch applied:
>
> FAIL: gdb.base/annota1.exp: signal sent (timeout)
>
> Any interesting output will end up in testsuite/gdb.log.
>
> Simon


--
Amos Bird
amosbird@gmail.com

Comments

Pedro Alves May 17, 2019, 4:26 p.m. UTC | #1
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
diff mbox

Patch

From 7da413c7e6a2d7bb68fb66bcefb1679cdd7ba2fa 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 +++++++++++-
 7 files changed, 49 insertions(+), 1 deletion(-)

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..25ed1dc439 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"
 	}
     }
@@ -492,8 +492,18 @@  proc thread_switch {} {
     }
 }
 
+proc thread_exit {} {
+    global decimal
+    gdb_test_multiple "call (void)pthread_exit()" "thread exit" {
+	-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
 
-- 
2.21.0