This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[commit] [patch 1/3] Refactor/simplify (+fix) svr4_current_sos


On Thu, 13 Oct 2011 01:17:55 +0200, Jan Kratochvil wrote:
> Anyway I do not have an opinion on it, if you like it better this way.

The former patch version is more close to the original code, the IGNORE_FIRST
change can be an incremental one; I do not like it much, it IMO complicates it
more.  Added there at least the function comment part.

Due to inter-patch dependencies hell checked it in


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2011-10/msg00117.html

--- src/gdb/ChangeLog	2011/10/13 16:26:22	1.13435
+++ src/gdb/ChangeLog	2011/10/14 07:55:21	1.13436
@@ -1,3 +1,21 @@
+2011-10-14  Jan Kratochvil  <jan.kratochvil@redhat.com>
+	    Paul Pluzhnikov  <ppluzhnikov@google.com>
+
+	* defs.h (struct so_list): New forward declaration.
+	(make_cleanup_free_so): New declaration.
+	* solib-svr4.c (ignore_first_link_map_entry): Remove.
+	(svr4_free_so): Move the function here from downwards.  Handle NULL
+	so->lm_info.
+	(svr4_free_library_list): New.
+	(svr4_read_so_list): New, moved here code from svr4_current_sos.
+	Use more cleanups.  Use new parameter ignore_first instead of
+	ignore_first_link_map_entry.
+	(svr4_current_sos): New variable ignore_first, initialize it.  New
+	variable back_to, use it for svr4_free_library_list protection.
+	(svr4_free_so): Remove - move upwards.
+	* utils.c: Include solist.h.
+	(do_free_so, make_cleanup_free_so): New functions.
+
 2011-10-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	Fix internal error regression.
--- src/gdb/defs.h	2011/10/09 22:21:42	1.303
+++ src/gdb/defs.h	2011/10/14 07:55:26	1.304
@@ -375,6 +375,9 @@
 extern struct cleanup *make_cleanup_value_free_to_mark (struct value *);
 extern struct cleanup *make_cleanup_value_free (struct value *);
 
+struct so_list;
+extern struct cleanup *make_cleanup_free_so (struct so_list *so);
+
 extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *);
 
 extern struct cleanup *make_my_cleanup (struct cleanup **,
--- src/gdb/solib-svr4.c	2011/10/02 21:05:59	1.155
+++ src/gdb/solib-svr4.c	2011/10/14 07:55:26	1.156
@@ -302,17 +302,6 @@
 				ptr_type);
 }
 
-static int
-ignore_first_link_map_entry (struct so_list *so)
-{
-  /* Assume that everything is a library if the dynamic loader was loaded
-     late by a static executable.  */
-  if (exec_bfd && bfd_get_section_by_name (exec_bfd, ".dynamic") == NULL)
-    return 0;
-
-  return lm_prev (so) == 0;
-}
-
 /* Per pspace SVR4 specific data.  */
 
 struct svr4_info
@@ -942,6 +931,32 @@
   return 1;
 }
 
+/* Implementation for target_so_ops.free_so.  */
+
+static void
+svr4_free_so (struct so_list *so)
+{
+  if (so->lm_info)
+    xfree (so->lm_info->lm);
+  xfree (so->lm_info);
+}
+
+/* Free so_list built so far (called via cleanup).  */
+
+static void
+svr4_free_library_list (void *p_list)
+{
+  struct so_list *list = *(struct so_list **) p_list;
+
+  while (list != NULL)
+    {
+      struct so_list *next = list->next;
+
+      svr4_free_so (list);
+      list = next;
+    }
+}
+
 /* If no shared library information is available from the dynamic
    linker, build a fallback list from other sources.  */
 
@@ -971,47 +986,31 @@
   return new;
 }
 
-/* Implement the "current_sos" target_so_ops method.  */
+/* Read the whole inferior libraries chain starting at address LM.  Add the
+   entries to the tail referenced by LINK_PTR_PTR.  Ignore the first entry if
+   IGNORE_FIRST and set global MAIN_LM_ADDR according to it.  */
 
-static struct so_list *
-svr4_current_sos (void)
+static void
+svr4_read_so_list (CORE_ADDR lm, struct so_list ***link_ptr_ptr,
+		   int ignore_first)
 {
-  CORE_ADDR lm, prev_lm;
-  struct so_list *head = 0;
-  struct so_list **link_ptr = &head;
-  CORE_ADDR ldsomap = 0;
-  struct svr4_info *info;
-
-  info = get_svr4_info ();
-
-  /* Always locate the debug struct, in case it has moved.  */
-  info->debug_base = 0;
-  locate_base (info);
+  CORE_ADDR prev_lm = 0, next_lm;
 
-  /* If we can't find the dynamic linker's base structure, this
-     must not be a dynamically linked executable.  Hmm.  */
-  if (! info->debug_base)
-    return svr4_default_sos ();
-
-  /* Walk the inferior's link map list, and build our list of
-     `struct so_list' nodes.  */
-  prev_lm = 0;
-  lm = solib_svr4_r_map (info);
-
-  while (lm)
+  for (; lm != 0; prev_lm = lm, lm = next_lm)
     {
       struct link_map_offsets *lmo = svr4_fetch_link_map_offsets ();
-      struct so_list *new = XZALLOC (struct so_list);
-      struct cleanup *old_chain = make_cleanup (xfree, new);
-      CORE_ADDR next_lm;
+      struct so_list *new;
+      struct cleanup *old_chain;
+      int errcode;
+      char *buffer;
 
-      new->lm_info = xmalloc (sizeof (struct lm_info));
-      make_cleanup (xfree, new->lm_info);
+      new = XZALLOC (struct so_list);
+      old_chain = make_cleanup_free_so (new);
 
-      new->lm_info->l_addr = (CORE_ADDR)-1;
+      new->lm_info = xmalloc (sizeof (struct lm_info));
+      new->lm_info->l_addr = (CORE_ADDR) -1;
       new->lm_info->lm_addr = lm;
       new->lm_info->lm = xzalloc (lmo->link_map_size);
-      make_cleanup (xfree, new->lm_info->lm);
 
       read_memory (lm, new->lm_info->lm, lmo->link_map_size);
 
@@ -1020,8 +1019,8 @@
       if (lm_prev (new) != prev_lm)
 	{
 	  warning (_("Corrupted shared library list"));
-	  free_so (new);
-	  next_lm = 0;
+	  do_cleanups (old_chain);
+	  break;
 	}
 
       /* For SVR4 versions, the first entry in the link map is for the
@@ -1029,58 +1028,93 @@
          SVR4, it has no name.  For others (Solaris 2.3 for example), it
          does have a name, so we can no longer use a missing name to
          decide when to ignore it.  */
-      else if (ignore_first_link_map_entry (new) && ldsomap == 0)
+      if (ignore_first && lm_prev (new) == 0)
 	{
+	  struct svr4_info *info = get_svr4_info ();
+
 	  info->main_lm_addr = new->lm_info->lm_addr;
-	  free_so (new);
+	  do_cleanups (old_chain);
+	  continue;
 	}
-      else
-	{
-	  int errcode;
-	  char *buffer;
 
-	  /* Extract this shared object's name.  */
-	  target_read_string (lm_name (new), &buffer,
-			      SO_NAME_MAX_PATH_SIZE - 1, &errcode);
-	  if (errcode != 0)
-	    warning (_("Can't read pathname for load map: %s."),
-		     safe_strerror (errcode));
-	  else
-	    {
-	      strncpy (new->so_name, buffer, SO_NAME_MAX_PATH_SIZE - 1);
-	      new->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
-	      strcpy (new->so_original_name, new->so_name);
-	    }
-	  xfree (buffer);
-
-	  /* If this entry has no name, or its name matches the name
-	     for the main executable, don't include it in the list.  */
-	  if (! new->so_name[0]
-	      || match_main (new->so_name))
-	    free_so (new);
-	  else
-	    {
-	      new->next = 0;
-	      *link_ptr = new;
-	      link_ptr = &new->next;
-	    }
+      /* Extract this shared object's name.  */
+      target_read_string (lm_name (new), &buffer,
+			  SO_NAME_MAX_PATH_SIZE - 1, &errcode);
+      if (errcode != 0)
+	{
+	  warning (_("Can't read pathname for load map: %s."),
+		   safe_strerror (errcode));
+	  do_cleanups (old_chain);
+	  continue;
 	}
 
-      prev_lm = lm;
-      lm = next_lm;
-
-      /* On Solaris, the dynamic linker is not in the normal list of
-	 shared objects, so make sure we pick it up too.  Having
-	 symbol information for the dynamic linker is quite crucial
-	 for skipping dynamic linker resolver code.  */
-      if (lm == 0 && ldsomap == 0)
+      strncpy (new->so_name, buffer, SO_NAME_MAX_PATH_SIZE - 1);
+      new->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
+      strcpy (new->so_original_name, new->so_name);
+      xfree (buffer);
+
+      /* If this entry has no name, or its name matches the name
+	 for the main executable, don't include it in the list.  */
+      if (! new->so_name[0] || match_main (new->so_name))
 	{
-	  lm = ldsomap = solib_svr4_r_ldsomap (info);
-	  prev_lm = 0;
+	  do_cleanups (old_chain);
+	  continue;
 	}
 
       discard_cleanups (old_chain);
+      new->next = 0;
+      **link_ptr_ptr = new;
+      *link_ptr_ptr = &new->next;
     }
+}
+
+/* Implement the "current_sos" target_so_ops method.  */
+
+static struct so_list *
+svr4_current_sos (void)
+{
+  CORE_ADDR lm;
+  struct so_list *head = NULL;
+  struct so_list **link_ptr = &head;
+  struct svr4_info *info;
+  struct cleanup *back_to;
+  int ignore_first;
+
+  info = get_svr4_info ();
+
+  /* Always locate the debug struct, in case it has moved.  */
+  info->debug_base = 0;
+  locate_base (info);
+
+  /* If we can't find the dynamic linker's base structure, this
+     must not be a dynamically linked executable.  Hmm.  */
+  if (! info->debug_base)
+    return svr4_default_sos ();
+
+  /* Assume that everything is a library if the dynamic loader was loaded
+     late by a static executable.  */
+  if (exec_bfd && bfd_get_section_by_name (exec_bfd, ".dynamic") == NULL)
+    ignore_first = 0;
+  else
+    ignore_first = 1;
+
+  back_to = make_cleanup (svr4_free_library_list, &head);
+
+  /* Walk the inferior's link map list, and build our list of
+     `struct so_list' nodes.  */
+  lm = solib_svr4_r_map (info);
+  if (lm)
+    svr4_read_so_list (lm, &link_ptr, ignore_first);
+
+  /* On Solaris, the dynamic linker is not in the normal list of
+     shared objects, so make sure we pick it up too.  Having
+     symbol information for the dynamic linker is quite crucial
+     for skipping dynamic linker resolver code.  */
+  lm = solib_svr4_r_ldsomap (info);
+  if (lm)
+    svr4_read_so_list (lm, &link_ptr, 0);
+
+  discard_cleanups (back_to);
 
   if (head == NULL)
     return svr4_default_sos ();
@@ -2048,14 +2082,6 @@
   info->debug_loader_name = NULL;
 }
 
-static void
-svr4_free_so (struct so_list *so)
-{
-  xfree (so->lm_info->lm);
-  xfree (so->lm_info);
-}
-
-
 /* Clear any bits of ADDR that wouldn't fit in a target-format
    data pointer.  "Data pointer" here refers to whatever sort of
    address the dynamic linker uses to manage its sections.  At the
--- src/gdb/utils.c	2011/10/09 22:21:42	1.263
+++ src/gdb/utils.c	2011/10/14 07:55:26	1.264
@@ -60,6 +60,7 @@
 #include "gdbcore.h"
 #include "top.h"
 #include "main.h"
+#include "solist.h"
 
 #include "inferior.h"		/* for signed_pointer_to_address */
 
@@ -495,6 +496,24 @@
   return make_my_cleanup (&cleanup_chain, do_value_free, value);
 }
 
+/* Helper for make_cleanup_free_so.  */
+
+static void
+do_free_so (void *arg)
+{
+  struct so_list *so = arg;
+
+  free_so (so);
+}
+
+/* Make cleanup handler calling free_so for SO.  */
+
+struct cleanup *
+make_cleanup_free_so (struct so_list *so)
+{
+  return make_my_cleanup (&cleanup_chain, do_free_so, so);
+}
+
 struct cleanup *
 make_my_cleanup2 (struct cleanup **pmy_chain, make_cleanup_ftype *function,
 		  void *arg,  void (*free_arg) (void *))


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]