Re: [RFA] osfsolib.c: support Tru64 5.x

Re: [RFA] osfsolib.c: support Tru64 5.x

[Nick Duffek]

On 25-May-2001, Kevin Buettner wrote:

>I think this is slightly less efficient

You're talking about the repeated calls to target_resize_to_sections,
right?

I agree.  It wouldn't be too hard to pre-widen the section array before
entering the loop and then narrow it afterward.  If you think it's worth
doing, let me know and I'll post a follow-on patch.

Nick

Re: [RFA] osfsolib.c: support Tru64 5.x

[Kevin Buettner]

On May 25,  8:47pm, Nick Duffek wrote:

> On 25-May-2001, Kevin Buettner wrote:
> 
> >I think this is slightly less efficient
> 
> You're talking about the repeated calls to target_resize_to_sections,
> right?

Yes, along with all of the memory allocation, deallocation, and
copying that go along with it.

> I agree.  It wouldn't be too hard to pre-widen the section array before
> entering the loop and then narrow it afterward.  If you think it's worth
> doing, let me know and I'll post a follow-on patch.

I think it's okay as is.  (Fifteen years ago, I would've felt differently.)

Kevin

Re: [RFA] osfsolib.c: support Tru64 5.x

[Nick Duffek]

On 25-May-2001, Kevin Buettner wrote:

>This is approved so long as you fix the typo in the line below:
>
>> +	     that later noes in the list can query this object, as is needed

Fixed and committed.

Nick

[RFA] osfsolib.c: support Tru64 5.x

[Nicholas Duffek]

This patch updates the shared module list reader in osfsolib.c to work
with Tru64 5.x.

On 5.x, ldr_module_info_t.modinfo_addr doesn't hold the address of its
containing structure, like it does on 4.x.  osfsolib.c uses that address
to check for the end of the shared module list, so on 5.x, it falls off
the end of the list.

The patch fixes that by overriding the modinfo_addr field with the
expected address in GDB's copy of the module list.

There was also a problem, on both 5.x and 4.x, where GDB would issue "Hit
heuristic-fence-post without finding enclosing function" warnings the
second time a program was run.

The fix is to add a make_run_cleanup() hook to call clear_solib().

I also made several cosmetic changes:
  - declare some unknown module fields as "char *" because they sometimes
    contain strings
  - split some large conditionals into multiple "if"s to simplify
    debugging
  - merge two similar code sequences into a single function
  - add/update various comments

ChangeLog:

	* osfsolib.c: Update libxproc.a comment.
	(ldr_module_info_t): Declare some unknown fields as char *
	instead of CORE_ADDR.
	(ldr_context_t): Document.
	(solib_cleanup_queued): New variable.
	(do_clear_solib, link_map_member): New functions.
	(first_link_map_member, next_link_map_member): Split large "if"
	conditional into multiple conditionals.  Rename {first,next}_lm
	to lm.  Move common code to link_map_member().
	(find_solib): Call make_run_cleanup().
	(clear_solib): Delete free_all_symtabs comment.  Don't call
	disable_breakpoints_in_shlibs().

Tested on alpha-dec-osf4.0 and alphaev6-dec-osf5.1, with
in_solib_dynsym_resolve_code() and .sbss patches similar to the ones Joel
Brobecker recently posted.  Okay to apply?

Nicholas Duffek
<nsd@redhat.com>

[patch follows]

Index: gdb/osfsolib.c
===================================================================
diff -up gdb/osfsolib.c gdb/osfsolib.c
--- gdb/osfsolib.c	Sat May 12 18:55:08 2001
+++ gdb/osfsolib.c	Sat May 12 18:18:33 2001
@@ -1,5 +1,5 @@
 /* Handle OSF/1 shared libraries for GDB, the GNU Debugger.
-   Copyright 1993, 1994, 1995, 1996, 1998, 1999, 2000
+   Copyright 1993, 1994, 1995, 1996, 1998, 1999, 2000, 2001
    Free Software Foundation, Inc.
 
    This file is part of GDB.
@@ -51,17 +51,19 @@
    current load address).
 
    Under OSF/1 there are two possibilities to get at this information:
+
    1) Peek around in the runtime loader structures.
    These are not documented, and they are not defined in the system
    header files. The definitions below were obtained by experimentation,
    but they seem stable enough.
-   2) Use the undocumented libxproc.a library, which contains the
-   equivalent ldr_* routines.
-   This approach is somewhat cleaner, but it requires that the GDB
-   executable is dynamically linked. In addition it requires a
-   NAT_CLIBS= -lxproc -Wl,-expect_unresolved,ldr_process_context
-   linker specification for GDB and all applications that are using
-   libgdb.
+
+   2) Use the libxproc.a library, which contains the equivalent ldr_*
+   routines.  The library is documented in Tru64 5.x, but as of 5.1, it
+   only allows a process to examine itself.  On earlier versions, it
+   may require that the GDB executable be dynamically linked and that
+   NAT_CLIBS include -lxproc -Wl,-expect_unresolved,ldr_process_context
+   for GDB and all applications that are using libgdb.
+
    We will use the peeking approach until it becomes unwieldy.  */
 
 #ifndef USE_LDR_ROUTINES
@@ -73,12 +75,13 @@ typedef struct
   {
     CORE_ADDR next;
     CORE_ADDR previous;
-    CORE_ADDR unknown1;
+    char *unknown1;
     char *module_name;
-    CORE_ADDR modinfo_addr;
+    CORE_ADDR modinfo_addr;	/* used by next_link_map_member() to detect
+				   the end of the shared module list */
     long module_id;
-    CORE_ADDR unknown2;
-    CORE_ADDR unknown3;
+    char *unknown2;
+    char *unknown3;
     long region_count;
     CORE_ADDR regioninfo_addr;
   }
@@ -96,6 +99,9 @@ typedef struct
   }
 ldr_region_info_t;
 
+/* Structure at RLD_CONTEXT_ADDRESS specifying the start and finish addresses
+   of the shared module list.  */
+
 typedef struct
   {
     CORE_ADDR unknown1;
@@ -170,10 +176,14 @@ struct so_list
 
 static struct so_list *so_list_head;	/* List of known shared objects */
 
+static int solib_cleanup_queued = 0;	/* make_run_cleanup called */
+
 extern int fdmatch (int, int);	/* In libiberty */
 
 /* Local function prototypes */
 
+static void do_clear_solib (PTR);
+
 static void sharedlibrary_command (char *, int);
 
 static void info_sharedlibrary_command (char *, int);
@@ -286,6 +296,62 @@ solib_map_sections (void *arg)
   return (1);
 }
 
+/* Read a link map member into LM.  If MODINFO is null, read the first
+   member, else read the one in MODINFO->next.  Return success.  */
+
+#ifndef USE_LDR_ROUTINES
+static int
+link_map_member (struct link_map *lm, ldr_module_info_t *modinfo)
+{
+  CORE_ADDR ldr_context_addr, prev, next;
+
+  /* Read the runtime loader list header.  ldr_context.tail changes as new
+     libraries are loaded, so don't cache across calls.  */
+  if (target_read_memory ((CORE_ADDR) RLD_CONTEXT_ADDRESS,
+			  (char *) &ldr_context_addr,
+			  sizeof (CORE_ADDR)) != 0)
+    return 0;
+  if (target_read_memory (ldr_context_addr,
+			  (char *) &ldr_context,
+			  sizeof (ldr_context_t)) != 0)
+    return 0;
+
+  if (!modinfo)
+    {
+      prev = ldr_context.tail + 1;
+      next = ldr_context.head;
+    }
+  else
+    {
+      prev = modinfo->modinfo_addr;
+      next = modinfo->next;
+    }
+
+  for (;;)
+    {
+      /* Check for the end of the list.  */
+      if (prev == ldr_context.tail)
+	return 0;
+
+      if (target_read_memory (next,
+			      (char *) &lm->module_info,
+			      sizeof (ldr_module_info_t)) != 0)
+	return 0;
+
+      /* Skip past invalid members, e.g. /sbin/loader on Tru64 5.x.  */
+      if (lm->module_info.modinfo_addr)
+	break;
+      prev = next;
+      next = lm->module_info.next;
+    }
+
+  /* Override bogus value on Tru64 5.x.  */
+  lm->module_info.modinfo_addr = next;
+
+  return 1;
+}
+#endif   /* !USE_LDR_ROUTINES */
+
 /*
 
    LOCAL FUNCTION
@@ -306,8 +372,7 @@ solib_map_sections (void *arg)
 static struct link_map *
 first_link_map_member (void)
 {
-  struct link_map *lm = NULL;
-  static struct link_map first_lm;
+  static struct link_map lm;
 
 #ifdef USE_LDR_ROUTINES
   ldr_module_t mod_id = LDR_NULL_MODULE;
@@ -316,75 +381,52 @@ first_link_map_member (void)
   fake_ldr_process = ldr_core_process ();
   ldr_set_core_reader (ldr_read_memory);
   ldr_xdetach (fake_ldr_process);
-  if (ldr_xattach (fake_ldr_process) != 0
-      || ldr_next_module (fake_ldr_process, &mod_id) != 0
-      || mod_id == LDR_NULL_MODULE
-      || ldr_inq_module (fake_ldr_process, mod_id,
-			 &first_lm.module_info, sizeof (ldr_module_info_t),
-			 &retsize) != 0)
-    return lm;
+  if (ldr_xattach (fake_ldr_process) != 0)
+    return NULL;
+  if (ldr_next_module (fake_ldr_process, &mod_id) != 0)
+    return NULL;
+  if (mod_id == LDR_NULL_MODULE)
+    return NULL;
+  if (ldr_inq_module (fake_ldr_process, mod_id,
+		      &lm.module_info, sizeof (ldr_module_info_t),
+		      &retsize) != 0)
+    return NULL;
 #else
-  CORE_ADDR ldr_context_addr;
-
-  if (target_read_memory ((CORE_ADDR) RLD_CONTEXT_ADDRESS,
-			  (char *) &ldr_context_addr,
-			  sizeof (CORE_ADDR)) != 0
-      || target_read_memory (ldr_context_addr,
-			     (char *) &ldr_context,
-			     sizeof (ldr_context_t)) != 0
-      || target_read_memory ((CORE_ADDR) ldr_context.head,
-			     (char *) &first_lm.module_info,
-			     sizeof (ldr_module_info_t)) != 0)
-    return lm;
+  if (!link_map_member (&lm, NULL))
+    return NULL;
 #endif
 
-  lm = &first_lm;
-
   /* The first entry is for the main program and should be skipped.  */
-  lm->l_name = NULL;
+  lm.l_name = NULL;
 
-  return lm;
+  return &lm;
 }
 
 static struct link_map *
 next_link_map_member (struct so_list *so_list_ptr)
 {
-  struct link_map *lm = NULL;
-  static struct link_map next_lm;
+  static struct link_map lm;
+
 #ifdef USE_LDR_ROUTINES
   ldr_module_t mod_id = so_list_ptr->lm.module_info.lmi_modid;
   size_t retsize;
 
-  if (ldr_next_module (fake_ldr_process, &mod_id) != 0
-      || mod_id == LDR_NULL_MODULE
-      || ldr_inq_module (fake_ldr_process, mod_id,
-			 &next_lm.module_info, sizeof (ldr_module_info_t),
-			 &retsize) != 0)
-    return lm;
-
-  lm = &next_lm;
-  lm->l_name = lm->module_info.lmi_name;
+  if (ldr_next_module (fake_ldr_process, &mod_id) != 0)
+    return NULL;
+  if (mod_id == LDR_NULL_MODULE)
+    return NULL;
+  if (ldr_inq_module (fake_ldr_process, mod_id,
+		      &lm.module_info, sizeof (ldr_module_info_t),
+		      &retsize) != 0)
+    return NULL;
+  lm.l_name = lm.module_info.lmi_name;
 #else
-  CORE_ADDR ldr_context_addr;
-
-  /* Reread context in case ldr_context.tail was updated.  */
-
-  if (target_read_memory ((CORE_ADDR) RLD_CONTEXT_ADDRESS,
-			  (char *) &ldr_context_addr,
-			  sizeof (CORE_ADDR)) != 0
-      || target_read_memory (ldr_context_addr,
-			     (char *) &ldr_context,
-			     sizeof (ldr_context_t)) != 0
-      || so_list_ptr->lm.module_info.modinfo_addr == ldr_context.tail
-      || target_read_memory (so_list_ptr->lm.module_info.next,
-			     (char *) &next_lm.module_info,
-			     sizeof (ldr_module_info_t)) != 0)
-    return lm;
-
-  lm = &next_lm;
-  lm->l_name = lm->module_info.module_name;
+  if (!link_map_member (&lm, &so_list_ptr->lm.module_info))
+    return NULL;
+  lm.l_name = lm.module_info.module_name;
 #endif
-  return lm;
+
+  return &lm;
 }
 
 static void
@@ -514,6 +556,12 @@ find_solib (struct so_list *so_list_ptr)
   struct link_map *lm = NULL;
   struct so_list *new;
 
+  if (!solib_cleanup_queued)
+    {
+      make_run_cleanup (do_clear_solib, NULL);
+      solib_cleanup_queued = 1;
+    }
+
   if (so_list_ptr == NULL)
     {
       /* We are setting up for a new scan through the loaded images. */
@@ -784,16 +832,12 @@ solib_address (CORE_ADDR address)
   return (0);
 }
 
-/* Called by free_all_symtabs */
-
 void
 clear_solib (void)
 {
   struct so_list *next;
   char *bfd_filename;
 
-  disable_breakpoints_in_shlibs (1);
-
   while (so_list_head)
     {
       if (so_list_head->sections)
@@ -817,6 +861,20 @@ clear_solib (void)
       xfree (so_list_head);
       so_list_head = next;
     }
+}
+
+static void
+do_clear_solib (PTR dummy)
+{
+  clear_solib ();
+
+  /* Clear solib_cleanup_queued after calling clear_solib(), because
+     clear_solib() checks/sets solib_cleanup_queued via this call sequence:
+       1. disable_breakpoints_in_shlibs()
+       2. PC_SOLIB()
+       3. solib_address()
+       4. find_solib()  */
+  solib_cleanup_queued = 0;
 }
 
 /*

Re: [RFA] osfsolib.c: support Tru64 5.x

[Kevin Buettner]

On May 12,  7:47pm, Nicholas Duffek wrote:

> 	* osfsolib.c: Update libxproc.a comment.
> 	(ldr_module_info_t): Declare some unknown fields as char *
> 	instead of CORE_ADDR.
> 	(ldr_context_t): Document.
> 	(solib_cleanup_queued): New variable.
> 	(do_clear_solib, link_map_member): New functions.
> 	(first_link_map_member, next_link_map_member): Split large "if"
> 	conditional into multiple conditionals.  Rename {first,next}_lm
> 	to lm.  Move common code to link_map_member().
> 	(find_solib): Call make_run_cleanup().
> 	(clear_solib): Delete free_all_symtabs comment.  Don't call
> 	disable_breakpoints_in_shlibs().

Nick,

Is there any chance that osfsolib.c could be reworked into a solib.c
backend?  (See solib-svr4.c and solib-aix5.c for examples.)  I don't
know if you have the time and/or energy to work on this at the moment,
but that's what I would like to see done.  (Eventually, anyway.)

In the interim, I think your patch can go in so long as you've tested it
on Tru64 4.x and 5.x.

Kevin

Re: [RFA] osfsolib.c: support Tru64 5.x

[Nick Duffek]

On 13-May-2001, Kevin Buettner wrote:

>Is there any chance that osfsolib.c could be reworked into a solib.c
>backend?

Okay, I've just finished doing that.  It's nice to delete so much
duplicated code; the new file is 60% the size of the old one.

Is there any pending gdbint.texinfo documentation on how to implement
shared library support using solib.c?  If not, I volunteer to write a
brief explanation, while it's fresh in my mind.

I ran into a couple of problems:

1. It's necessary for the target_so_ops.current_sos callback to zero at
least the so_list.section field of each element it returns.  Otherwise,
solib_map_sections() passes a bogus pointer to build_section_table(),
which frees the bogus pointer.

What do you think about a patch like this?

Index: gdb/solist.h
===================================================================
diff -up gdb/solist.h gdb/solist.h
--- gdb/solist.h	Mon May 14 21:17:56 2001
+++ gdb/solist.h	Mon May 14 21:17:46 2001
@@ -52,7 +52,9 @@ struct so_list
 
     /* The following fields of the structure are built from
        information gathered from the shared object file itself, and
-       are initialized when we actually add it to our symbol tables.  */
+       are set when we actually add it to our symbol tables.
+
+       current_sos must initialize these fields to 0.  */
 
     bfd *abfd;
     char symbols_loaded;	/* flag: symbols read in yet? */

2. Because Tru64 doesn't use ELF binaries, "info sharedlibrary"
incorrectly infers the address size to be 32 bits, so output address
fields are too narrow:

  (gdb) inf shared
  From        To          Syms Read   Shared Object Library
  0x3ff800cd8100x3ff801ca290Yes         /usr/shlib/libc.so
  (gdb) 

Some possible fixes:
  (a) Default to 64 instead of 32 bits.
  (b) Use something like bfd_arch_bits_per_address(exec_bfd) instead of
      or in addition to bfd_get_arch_size (exec_bfd).

I'll post the new osfsolib.c patch as soon as I finish regression-testing
it.

Nick

Re: [RFA] osfsolib.c: support Tru64 5.x

[Kevin Buettner]

On May 14,  9:44pm, Nick Duffek wrote:

> >Is there any chance that osfsolib.c could be reworked into a solib.c
> >backend?
> 
> Okay, I've just finished doing that.  It's nice to delete so much
> duplicated code; the new file is 60% the size of the old one.

Very cool.  Thanks for doing this.

> Is there any pending gdbint.texinfo documentation on how to implement
> shared library support using solib.c?  If not, I volunteer to write a
> brief explanation, while it's fresh in my mind.

I have nothing planned.  I think it'd be great if you'd write up the
documentation.

> I ran into a couple of problems:
> 
> 1. It's necessary for the target_so_ops.current_sos callback to zero at
> least the so_list.section field of each element it returns.  Otherwise,
> solib_map_sections() passes a bogus pointer to build_section_table(),
> which frees the bogus pointer.
> 
> What do you think about a patch like this?
> 
> Index: gdb/solist.h
> ===================================================================
> diff -up gdb/solist.h gdb/solist.h
> --- gdb/solist.h	Mon May 14 21:17:56 2001
> +++ gdb/solist.h	Mon May 14 21:17:46 2001
> @@ -52,7 +52,9 @@ struct so_list
>  
>      /* The following fields of the structure are built from
>         information gathered from the shared object file itself, and
> -       are initialized when we actually add it to our symbol tables.  */
> +       are set when we actually add it to our symbol tables.
> +
> +       current_sos must initialize these fields to 0.  */
>  
>      bfd *abfd;
>      char symbols_loaded;	/* flag: symbols read in yet? */

Good catch.  Please check this in.

> 2. Because Tru64 doesn't use ELF binaries, "info sharedlibrary"
> incorrectly infers the address size to be 32 bits, so output address
> fields are too narrow:
> 
>   (gdb) inf shared
>   From        To          Syms Read   Shared Object Library
>   0x3ff800cd8100x3ff801ca290Yes         /usr/shlib/libc.so
>   (gdb) 
> 
> Some possible fixes:
>   (a) Default to 64 instead of 32 bits.
>   (b) Use something like bfd_arch_bits_per_address(exec_bfd) instead of
>       or in addition to bfd_get_arch_size (exec_bfd).

Option (b) sounds better to me.

> I'll post the new osfsolib.c patch as soon as I finish regression-testing
> it.

Cool.  I'm looking forward to seeing it.

Thanks,

Kevin

Re: [RFA] osfsolib.c: support Tru64 5.x

[Nick Duffek]

On 15-May-2001, Kevin Buettner wrote:

>On May 14,  9:44pm, Nick Duffek wrote:
[...]
>> Index: gdb/solist.h
[...]
>> +       current_sos must initialize these fields to 0.  */

>Good catch.  Please check this in.

Checked in.

>> 2. Because Tru64 doesn't use ELF binaries, "info sharedlibrary"
>> incorrectly infers the address size to be 32 bits
[...]
>> Some possible fixes:
>>   (a) Default to 64 instead of 32 bits.
>>   (b) Use something like bfd_arch_bits_per_address(exec_bfd) instead of
>>       or in addition to bfd_get_arch_size (exec_bfd).
>
>Option (b) sounds better to me.

The appended patch does that.

I ran into another problem: section name addresses in Tru64 shared module
lists refer to areas in /sbin/loader, which is the first element in the
shared module list.  Consequently, when the relocate_section_addresses()
callback tries to match BFD section names with those from the shared
module list, it can't read the latter.  (Actually, it can when debugging a
live process, because /sbin/loader is part of the address space viewable
from procfs, but not when debugging a core file.)

A solution is to reorganize update_solib_list() in solib.c to do this:

  for (each module) {
    call target_so_ops.relocate_section_addresses();
    add sections to lookup table using target_resize_to_sections();
  }

instead of this:

  for (each module) {
    call target_so_ops.relocate_section_addresses();
  }
  for (each module) {
    add sections to lookup table using target_resize_to_sections();
  }

That way, each relocate_section_addresses() callback can read addresses
from all objects earlier in the module list.

The appended patch makes that change as well.  With the patch and with the
new solib-osf.c backend that I'll post shortly, there are no regressions
(and several progressions) on Tru64 5.0 and Digital UNIX 4.0.

Okay to apply?

Nick

2001-05-24  Nick Duffek  <nsd@redhat.com>

	* solib.c (update_solib_list): Move target_resize_to_sections()
	into solib_map_sections() loop.
	(info_sharedlibrary_command): Try bfd_arch_bits_per_address() if
	bfd_get_arch_size() fails.

Index: gdb/solib.c
===================================================================
diff -up gdb/solib.c gdb/solib.c
--- gdb/solib.c	Thu May 24 21:22:28 2001
+++ gdb/solib.c	Thu May 24 21:22:12 2001
@@ -461,30 +461,20 @@ update_solib_list (int from_tty, struct 
 	  catch_errors (solib_map_sections, i,
 			"Error while mapping shared library sections:\n",
 			RETURN_MASK_ALL);
-	}
-
-      /* If requested, add the shared objects' sections to the the
-	 TARGET's section table.  */
-      if (target)
-	{
-	  int new_sections;
 
-	  /* Figure out how many sections we'll need to add in total.  */
-	  new_sections = 0;
-	  for (i = inferior; i; i = i->next)
-	    new_sections += (i->sections_end - i->sections);
-
-	  if (new_sections > 0)
+	  /* If requested, add the shared object's sections to the TARGET's
+	     section table.  Do this immediately after mapping the object so
+	     that later noes in the list can query this object, as is needed
+	     in solib-osf.c.  */
+	  if (target)
 	    {
-	      int space = target_resize_to_sections (target, new_sections);
-
-	      for (i = inferior; i; i = i->next)
+	      int count = (i->sections_end - i->sections);
+	      if (count > 0)
 		{
-		  int count = (i->sections_end - i->sections);
+		  int space = target_resize_to_sections (target, count);
 		  memcpy (target->to_sections + space,
 			  i->sections,
 			  count * sizeof (i->sections[0]));
-		  space += count;
 		}
 	    }
 	}
@@ -605,7 +595,10 @@ info_sharedlibrary_command (char *ignore
     }
 
   arch_size = bfd_get_arch_size (exec_bfd);
-  /* Default to 32-bit in case of failure (non-elf). */
+  if (arch_size == -1)
+    arch_size = bfd_arch_bits_per_address(exec_bfd);
+
+  /* Default to 32-bit in case of failure.  */
   if (arch_size == 32 || arch_size == -1)
     {
       addr_width = 8 + 4;