RE: [PATCH] gdb, thread-iter: handle null_ptid

["Metzger, Markus T" <markus.t.metzger@intel.com>]

Hello Simon,

>Is there an easy way to reproduce this situation?  Is is an existing
>test that fails when running against a gdbserver target board?

This was found by gdb.btrace/enable-new-thread.exp.


>> #10 0x0000000000b513d4 in target_read_partial (ops=0x2441680
><record_btrace_ops>, object=TARGET_OBJECT_BTRACE_CONF, annex=0x148cade
>"", buf=0x2eef9e0 "\320\360B\367\377\177", offset=0, len=4096,
>xfered_len=0x7fffffffc168) at gdb/target.c:1974
>> #11 0x0000000000b65940 in target_read_alloc_1<char> (ops=0x2441680
><record_btrace_ops>, object=TARGET_OBJECT_BTRACE_CONF, annex=0x148cade
>"") at gdb/target.c:2309
>> #12 0x0000000000b51d27 in target_read_stralloc (ops=0x2441680
><record_btrace_ops>, object=TARGET_OBJECT_BTRACE_CONF, annex=0x148cade
>"") at gdb/target.c:2348
>> #13 0x0000000000a0f881 in btrace_read_config (conf=0x2a6f5e8) at
>gdb/remote.c:14048
>> #14 0x0000000000a0fdbc in remote_target::enable_btrace (this=0x299a290,
>ptid=..., conf=0x261ae90 <record_btrace_conf>) at gdb/remote.c:14162
>
>I think your patch is just hiding the real problem.  Between frames 13
>and 14, we are going from a function that takes the ptid as a parameter
>to a function that takes the ptid implicitly through the inferior_ptid
>global.  Something needs to temporarily set inferior_ptid.

all_matching_threads_iterator() does not handle null_ptid, which leads to the
assert in find_inferior_pid().  I would say that this is a bug.  Would you agree?

Looks like there is another bug in record_btrace_enable_warn(), which should
temporarily switch to the new thread.

How about this [1]?  I'd keep it as a separate patch in addition to this one.

Regards,
Markus.

[1]
[PATCH] gdb, btrace: switch to new thread when enabling tracing

When new_thread overservers are notified, the new thread is not stopped,
yet, and inferior_ptid is stale.

Temporarily switch to the new thread in record-btrace's new_thread
observer.  Also change the function name to make it clear it is only used
as new-thread callback.
---
 gdb/record-btrace.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 3fcfd6a4761..198306a657d 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -279,16 +279,21 @@ require_btrace (void)
   return &tp->btrace;
 }
 
-/* Enable branch tracing for one thread.  Warn on errors.  */
+/* The new thread observer.  */
 
 static void
-record_btrace_enable_warn (struct thread_info *tp)
+record_btrace_on_new_thread (struct thread_info *tp)
 {
   /* Ignore this thread if its inferior is not recorded by us.  */
   target_ops *rec = tp->inf->target_at (record_stratum);
   if (rec != &record_btrace_ops)
     return;
 
+  /* TP hasn't stopped, yet, so INFERIOR_PTID is stale.  Temporarily set
+     INFERIOR_PTID to TP while we enable tracing for TP.  */
+  scoped_restore_current_thread restore_thread;
+  switch_to_thread (tp);
+
   try
     {
       btrace_enable (tp, &record_btrace_conf);
@@ -306,7 +311,7 @@ record_btrace_auto_enable (void)
 {
   DEBUG ("attach thread observer");
 
-  gdb::observers::new_thread.attach (record_btrace_enable_warn,
+  gdb::observers::new_thread.attach (record_btrace_on_new_thread,
 				     record_btrace_thread_observer_token,
 				     "record-btrace");
 }
-- 
2.31.1


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928