[PATCH 2/3] change gdbserver to use hashtab: preparatory cleanup

[Doug Evans <dje at google dot com>]

Hi.

This is patch 2/3 to speed up gdbserver attach/detach (and in general).

It contains:
misc. prepatory cleanup, in particular while there are routines
to traverse the lists, some code traverses the list itself.
These need to be rewritten to always use the provided traversal routines.

This patch contains a change to Makefile.in to build hashtab.o.
I know there's plans to link gdbserver with libiberty.
This is easily updated when that happens (or the libiberty conversion
patch can be updated).  In the meantime I don't want to tightly couple
the two patch sets.

2014-01-24  Doug Evans  <dje@google.com>

	* Makefile.in (OBS): Add hashtab.o.
	(hashtab.o): New rule.
	* dll.h (struct dll_info): Add comment.
	* gdbthread.h (struct thread_info): Add comment.
	(current_ptid): Simplify.
	* inferiors.c (for_each_inferior_with_data): New function.
	(find_thread_ptid): Simplify.
	* inferiors.h (struct process_info): Add comment.
	(for_each_inferior_with_data): Declare.
	* linux-low.c (second_thread_of_pid_p): Simplify fetch of thread id.
	(wait_for_sigstop): Simplify fetch of inferior id.
	* linux-low.h (struct lwp_info): Add comment.
	* regcache.h (struct inferior_table_entry): Delete forward decl.
	* server.c (accumulate_file_name_length): New function.
	(emit_dll_description): New function.
	(handle_qxfer_libraries): Use API function to loop over all inferiors.
	(handle_qxfer_threads_worker): New function.
	(handle_qxfer_threads_proper): Use API function to loop over all
	inferiors.
	(process_serial_event): Simplify fetch of thread id.
	* tracepoint.c (same_process_p): New function.
	(gdb_agent_about_to_close): Use API function to loop over all
	inferiors.
	* win32-low.c (current_inferior_ptid): Simplify fetch of inferior id.
	(match_dll_by_basename): New function.
	(dll_is_loaded_by_basename): New function.
	(win32_ensure_ntdll_loaded): Use API function to loop over all
	inferiors.

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 46324e6..1b88145 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -176,7 +176,7 @@ OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       target.o waitstatus.o utils.o debug.o version.o vec.o gdb_vecs.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
-      tdesc.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
+      tdesc.o hashtab.o $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
 XM_CLIBS = @LIBS@
@@ -549,6 +549,10 @@ safe-ctype.o: $(srcdir)/../../libiberty/safe-ctype.c
 lbasename.o: $(srcdir)/../../libiberty/lbasename.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+# Pass -DHAVE_CONFIG_H, we want it to pick up our config.h.
+hashtab.o: $(srcdir)/../../libiberty/hashtab.c
+	$(COMPILE) $< -DHAVE_CONFIG_H
+	$(POSTCOMPILE)
 
 aarch64.c : $(srcdir)/../regformats/aarch64.dat $(regdat_sh)
 	$(SHELL) $(regdat_sh) $(srcdir)/../regformats/aarch64.dat aarch64.c
diff --git a/gdb/gdbserver/dll.h b/gdb/gdbserver/dll.h
index bd6d7c5..381dd60 100644
--- a/gdb/gdbserver/dll.h
+++ b/gdb/gdbserver/dll.h
@@ -20,7 +20,9 @@
 
 struct dll_info
 {
+  /* This must appear first.  See inferiors.h.  */
   struct inferior_table_entry head;
+
   char *name;
   CORE_ADDR base_addr;
 };
diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h
index b050369..93fb006 100644
--- a/gdb/gdbserver/gdbthread.h
+++ b/gdb/gdbserver/gdbthread.h
@@ -26,7 +26,9 @@ struct btrace_target_info;
 
 struct thread_info
 {
+  /* This must appear first.  See inferiors.h.  */
   struct inferior_table_entry head;
+
   void *target_data;
   void *regcache_data;
 
@@ -76,6 +78,6 @@ void add_thread (ptid_t ptid, void *target_data);
 struct thread_info *find_thread_ptid (ptid_t ptid);
 
 /* Get current thread ID (Linux task ID).  */
-#define current_ptid ((struct inferior_table_entry *) current_inferior)->id
+#define current_ptid (current_inferior->head.id)
 
 #endif /* GDB_THREAD_H */
diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
index f7242f9..86c0307 100644
--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -59,6 +59,24 @@ for_each_inferior (struct inferior_table *list,
     }
 }
 
+/* Invoke ACTION for each inferior in LIST, passing DATA to ACTION.  */
+
+void
+for_each_inferior_with_data (struct inferior_table *list,
+			     void (*action) (struct inferior_table_entry *,
+					     void *),
+			     void *data)
+{
+  struct inferior_table_entry *cur = list->head, *next;
+
+  while (cur != NULL)
+    {
+      next = cur->next;
+      (*action) (cur, data);
+      cur = next;
+    }
+}
+
 void
 remove_inferior (struct inferior_table *list,
 		 struct inferior_table_entry *entry)
@@ -114,17 +132,7 @@ thread_to_gdb_id (struct thread_info *thread)
 struct thread_info *
 find_thread_ptid (ptid_t ptid)
 {
-  struct inferior_table_entry *inf = all_threads.head;
-
-  while (inf != NULL)
-    {
-      struct thread_info *thread = get_thread (inf);
-      if (ptid_equal (thread->head.id, ptid))
-	return thread;
-      inf = inf->next;
-    }
-
-  return NULL;
+  return (struct thread_info *) find_inferior_id (&all_threads, ptid);
 }
 
 ptid_t
diff --git a/gdb/gdbserver/inferiors.h b/gdb/gdbserver/inferiors.h
index 18be8f8..dcccbed 100644
--- a/gdb/gdbserver/inferiors.h
+++ b/gdb/gdbserver/inferiors.h
@@ -42,6 +42,7 @@ struct process_info_private;
 
 struct process_info
 {
+  /* This must appear first.  */
   struct inferior_table_entry head;
 
   /* Nonzero if this child process was attached rather than
@@ -84,6 +85,11 @@ void add_inferior_to_table (struct inferior_table *table,
 void for_each_inferior (struct inferior_table *table,
 			void (*action) (struct inferior_table_entry *));
 
+void for_each_inferior_with_data
+  (struct inferior_table *table,
+   void (*action) (struct inferior_table_entry *, void *),
+   void *data);
+
 extern struct thread_info *current_inferior;
 void remove_inferior (struct inferior_table *table,
 		      struct inferior_table_entry *entry);
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index f0c2dfc..d7c13c4 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -865,7 +865,7 @@ second_thread_of_pid_p (struct inferior_table_entry *entry, void *args)
 static int
 last_thread_of_process_p (struct thread_info *thread)
 {
-  ptid_t ptid = ((struct inferior_table_entry *)thread)->id;
+  ptid_t ptid = thread->head.id;
   int pid = ptid_get_pid (ptid);
   struct counter counter = { pid , 0 };
 
@@ -2926,7 +2926,7 @@ wait_for_sigstop (struct inferior_table_entry *entry)
 
   saved_inferior = current_inferior;
   if (saved_inferior != NULL)
-    saved_tid = ((struct inferior_table_entry *) saved_inferior)->id;
+    saved_tid = saved_inferior->head.id;
   else
     saved_tid = null_ptid; /* avoid bogus unused warning */
 
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 6c39c71..6e879d4 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -235,6 +235,7 @@ extern struct linux_target_ops the_low_target;
 
 struct lwp_info
 {
+  /* This must appear first.  See inferiors.h.  */
   struct inferior_table_entry head;
 
   /* If this flag is set, the next SIGSTOP will be ignored (the
diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h
index b795dad..891fead 100644
--- a/gdb/gdbserver/regcache.h
+++ b/gdb/gdbserver/regcache.h
@@ -19,7 +19,6 @@
 #ifndef REGCACHE_H
 #define REGCACHE_H
 
-struct inferior_table_entry;
 struct thread_info;
 struct target_desc;
 
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index a7ba458..d5748dc 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1135,6 +1135,48 @@ handle_qxfer_features (const char *annex,
   return len;
 }
 
+/* Worker routine for handle_qxfer_libraries.
+   Add to the length pointed to by ARG a conservative estimate of the
+   length needed to transmit the file name of INF.  */
+
+static void
+accumulate_file_name_length (struct inferior_table_entry *inf, void *arg)
+{
+  struct dll_info *dll = (struct dll_info *) inf;
+  unsigned int *total_len = arg;
+
+  /* Over-estimate the necessary memory.  Assume that every character
+     in the library name must be escaped.  */
+  *total_len += 128 + 6 * strlen (dll->name);
+}
+
+/* Worker routine for handle_qxfer_libraries.
+   Emit the XML to describe the library in INF.  */
+
+static void
+emit_dll_description (struct inferior_table_entry *inf, void *arg)
+{
+  struct dll_info *dll = (struct dll_info *) inf;
+  char **p_ptr = arg;
+  char *p = *p_ptr;
+  char *name;
+
+  strcpy (p, "  <library name=\"");
+  p = p + strlen (p);
+  name = xml_escape_text (dll->name);
+  strcpy (p, name);
+  free (name);
+  p = p + strlen (p);
+  strcpy (p, "\"><segment address=\"");
+  p = p + strlen (p);
+  sprintf (p, "0x%lx", (long) dll->base_addr);
+  p = p + strlen (p);
+  strcpy (p, "\"/></library>\n");
+  p = p + strlen (p);
+
+  *p_ptr = p;
+}
+
 /* Handle qXfer:libraries:read.  */
 
 static int
@@ -1144,7 +1186,6 @@ handle_qxfer_libraries (const char *annex,
 {
   unsigned int total_len;
   char *document, *p;
-  struct inferior_table_entry *dll_ptr;
 
   if (writebuf != NULL)
     return -2;
@@ -1152,11 +1193,9 @@ handle_qxfer_libraries (const char *annex,
   if (annex[0] != '\0' || !target_running ())
     return -1;
 
-  /* Over-estimate the necessary memory.  Assume that every character
-     in the library name must be escaped.  */
   total_len = 64;
-  for (dll_ptr = all_dlls.head; dll_ptr != NULL; dll_ptr = dll_ptr->next)
-    total_len += 128 + 6 * strlen (((struct dll_info *) dll_ptr)->name);
+  for_each_inferior_with_data (&all_dlls, accumulate_file_name_length,
+			       &total_len);
 
   document = malloc (total_len);
   if (document == NULL)
@@ -1165,24 +1204,7 @@ handle_qxfer_libraries (const char *annex,
   strcpy (document, "<library-list>\n");
   p = document + strlen (document);
 
-  for (dll_ptr = all_dlls.head; dll_ptr != NULL; dll_ptr = dll_ptr->next)
-    {
-      struct dll_info *dll = (struct dll_info *) dll_ptr;
-      char *name;
-
-      strcpy (p, "  <library name=\"");
-      p = p + strlen (p);
-      name = xml_escape_text (dll->name);
-      strcpy (p, name);
-      free (name);
-      p = p + strlen (p);
-      strcpy (p, "\"><segment address=\"");
-      p = p + strlen (p);
-      sprintf (p, "0x%lx", (long) dll->base_addr);
-      p = p + strlen (p);
-      strcpy (p, "\"/></library>\n");
-      p = p + strlen (p);
-    }
+  for_each_inferior_with_data (&all_dlls, emit_dll_description, &p);
 
   strcpy (p, "</library-list>\n");
 
@@ -1284,36 +1306,43 @@ handle_qxfer_statictrace (const char *annex,
   return nbytes;
 }
 
-/* Helper for handle_qxfer_threads.  */
+/* Helper for handle_qxfer_threads_proper.
+   Emit the XML to describe the thread of INF.  */
 
 static void
-handle_qxfer_threads_proper (struct buffer *buffer)
+handle_qxfer_threads_worker (struct inferior_table_entry *inf, void *arg)
 {
-  struct inferior_table_entry *thread;
+  struct thread_info *thread = (struct thread_info *) inf;
+  struct buffer *buffer = arg;
+  ptid_t ptid = thread_to_gdb_id (thread);
+  char ptid_s[100];
+  int core = target_core_of_thread (ptid);
+  char core_s[21];
 
-  buffer_grow_str (buffer, "<threads>\n");
+  write_ptid (ptid_s, ptid);
 
-  for (thread = all_threads.head; thread; thread = thread->next)
+  if (core != -1)
+    {
+      sprintf (core_s, "%d", core);
+      buffer_xml_printf (buffer, "<thread id=\"%s\" core=\"%s\"/>\n",
+			 ptid_s, core_s);
+    }
+  else
     {
-      ptid_t ptid = thread_to_gdb_id ((struct thread_info *)thread);
-      char ptid_s[100];
-      int core = target_core_of_thread (ptid);
-      char core_s[21];
+      buffer_xml_printf (buffer, "<thread id=\"%s\"/>\n",
+			 ptid_s);
+    }
+}
 
-      write_ptid (ptid_s, ptid);
+/* Helper for handle_qxfer_threads.  */
 
-      if (core != -1)
-	{
-	  sprintf (core_s, "%d", core);
-	  buffer_xml_printf (buffer, "<thread id=\"%s\" core=\"%s\"/>\n",
-			     ptid_s, core_s);
-	}
-      else
-	{
-	  buffer_xml_printf (buffer, "<thread id=\"%s\"/>\n",
-			     ptid_s);
-	}
-    }
+static void
+handle_qxfer_threads_proper (struct buffer *buffer)
+{
+  buffer_grow_str (buffer, "<threads>\n");
+
+  for_each_inferior_with_data (&all_threads, handle_qxfer_threads_worker,
+			       buffer);
 
   buffer_grow_str0 (buffer, "</threads>\n");
 }
@@ -3516,7 +3545,7 @@ process_serial_event (void)
 		  break;
 		}
 
-	      thread_id = ((struct inferior_table_entry *)thread)->id;
+	      thread_id = thread->head.id;
 	    }
 	  else
 	    {
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 3e2b74d..9d94133 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -3942,6 +3942,17 @@ cmd_qtstmat (char *packet)
     run_inferior_command (packet, strlen (packet) + 1);
 }
 
+/* Helper for gdb_agent_about_to_close.
+   Return non-zero if thread ENTRY is in the same process in DATA.  */
+
+static int
+same_process_p (struct inferior_table_entry *inf, void *data)
+{
+  int *pid = data;
+
+  return ptid_get_pid (inf->id) == *pid;
+}
+
 /* Sent the agent a command to close it.  */
 
 void
@@ -3952,19 +3963,12 @@ gdb_agent_about_to_close (int pid)
   if (!maybe_write_ipa_not_loaded (buf))
     {
       struct thread_info *save_inferior;
-      struct inferior_table_entry *inf = all_threads.head;
 
       save_inferior = current_inferior;
 
-      /* Find a certain thread which belongs to process PID.  */
-      while (inf != NULL)
-	{
-	  if (ptid_get_pid (inf->id) == pid)
-	    break;
-	  inf = inf->next;
-	}
-
-      current_inferior = (struct thread_info *) inf;
+      /* Find any thread which belongs to process PID.  */
+      current_inferior = (struct thread_info *)
+	find_inferior (&all_threads, same_process_p, &pid);
 
       strcpy (buf, "close");
 
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 0a93759..912ad08 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -114,7 +114,7 @@ static void win32_ensure_ntdll_loaded (void);
 static ptid_t
 current_inferior_ptid (void)
 {
-  return ((struct inferior_table_entry*) current_inferior)->id;
+  return current_inferior->head.id;
 }
 
 /* The current debug event from WaitForDebugEvent.  */
@@ -1142,6 +1142,28 @@ failed:
 }
 
 #ifndef _WIN32_WCE
+
+/* Helper routine for dll_is_loaded_by_basename.
+   Return non-zero if the basename in ARG matches the DLL in INF.  */
+
+static int
+match_dll_by_basename (struct inferior_table_entry *inf, void *arg)
+{
+  struct dll_info *iter = (void *) inf;
+  const char *basename = arg;
+
+  return strcasecmp (lbasename (iter->name), basename) == 0;
+}
+
+/* Return non-zero if the DLL specified by BASENAME is loaded.  */
+
+static int
+dll_is_loaded_by_basename (const char *basename)
+{
+  return find_inferior (&all_dlls, match_dll_by_basename,
+			(void *) basename) != NULL;
+}
+
 /* On certain versions of Windows, the information about ntdll.dll
    is not available yet at the time we get the LOAD_DLL_DEBUG_EVENT,
    thus preventing us from reporting this DLL as an SO. This has been
@@ -1158,20 +1180,14 @@ failed:
 static void
 win32_ensure_ntdll_loaded (void)
 {
-  struct inferior_table_entry *dll_e;
   size_t i;
   HMODULE dh_buf[1];
   HMODULE *DllHandle = dh_buf;
   DWORD cbNeeded;
   BOOL ok;
 
-  for (dll_e = all_dlls.head; dll_e != NULL; dll_e = dll_e->next)
-    {
-      struct dll_info *dll = (struct dll_info *) dll_e;
-
-      if (strcasecmp (lbasename (dll->name), "ntdll.dll") == 0)
-	return;
-    }
+  if (dll_is_loaded_by_basename ("ntdll.dll"))
+    return;
 
   if (!load_psapi ())
     return;