RE: [PATCH] gdb: fix an issue with thread list corruption

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

Hello Simon,

>The general issue is that safe-iterations should not be nested.  They should
>only
>be used with the intent of deleting the current item.  A comment may be more
>effective at preventing mis-use.
>
>Both for_each_thread() and find_thread() use all_threads_safe().  They
>probably
>shouldn't.  So, instead of fixing this one case by not using for_each_thread(),
>we
>should probably change for_each_thread() to use all_threads() and find those
>few
>cases, if any, that do use for_each_thread() to delete threads and change
>them
>to explicitly use all_threads_safe().

It turns out that there are only few calls to for_each_thread() and it looks like
one such call in mi/mi-main.c would run into a similar issue that this patch
fixes for infcmd.c.  I have not found a call that deletes the thread in the callback.

I'm testing a patch [1] that changes for_each_thread().

Regards,
markus.

[1]:
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index b3052b28d1a..1bdbf621982 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -796,7 +796,14 @@ void thread_change_ptid (process_stratum_target *targ,
 using for_each_thread_callback_ftype
   = gdb::function_view<void (thread_info *)>;
 
-/* Call CALLBACK once for each known thread.  */
+/* Call CALLBACK once for each known thread.
+
+   CALLBACK must not delete the thread.  To delete threads, use:
+
+     for (thread_info &t : all_threads_safe ())
+       if (some_condition ())
+        delete &t;
+*/
 
 extern void for_each_thread (for_each_thread_callback_ftype callback);
 
diff --git a/gdb/thread.c b/gdb/thread.c
index 4f62ca280cd..110e58a70aa 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -605,7 +605,7 @@ find_thread_by_handle (gdb::array_view<const gdb_byte> handle,
 void
 for_each_thread (for_each_thread_callback_ftype callback)
 {
-  for (thread_info &tp : all_threads_safe ())
+  for (thread_info &tp : all_threads ())
     callback (&tp);
 }

Intel Deutschland GmbH
Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928