[PATCH]: gdb: fdpic/frv: fix shared library loading

[PATCH]: gdb: fdpic/frv: fix shared library loading

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 4423 bytes --]

The recent change to reload_shared_libraries() broke FDPIC shared libraries as
the solib-frv.c code was implicitly relying on the initial order of calls
(first solib_addr() and then solib_create_inferior_hook()).  It was
maintaining internal state via enable_break{1,2}_done to handle this.

While I could tweak these values a bit more, the original code wasn't terribly
bullet proof -- if during the initial debug you attempted to view shared
libraries, the enable2_break() code would whine about the ldso internal debug
addresses being unfetchable (and would actually attempt to read address 0x8 on
the target).  So I've dropped this implicit dependency on order (i.e.
enable_break1_done) and updated the ldso poking code (i.e. enable_break2) to
silently return when the internal debug address is still set to 0.  It will
remain this way until the ldso gets a chance to initialize at which point the
code will act the same as before.

While I have no way of testing the FRV, the Blackfin FDPIC code is using this
same base in a 100% copy & paste method since we implemented FDPIC the same
way as the FRV guys (I'll address this in the future).  This fix was required
in order to handle shared libraries with Blackfin FDPIC properly, and I see no
reason why it wouldn't also work for FRV (since the uClibc ldso FDPIC code is
the same too and that's really what this is poking).
-mike

2010-04-14  Mike Frysinger  <vapier@gentoo.org>

	* solib-frv.c (enable_break1_done): Delete.
	(enable_break2): Do not check enable_break1_done.  Move the
	enable_break2_done setting and call to
	remove_solib_event_breakpoints() to the end.  Return without
	warning when the contents of _dl_debug_addr are 0.
	(enable_break): Do not set enable_break1_done.
	(frv_clear_solib): Likewise.

RCS file: /cvs/src/src/gdb/solib-frv.c,v
retrieving revision 1.33
diff -u -p -r1.33 solib-frv.c
--- gdb/solib-frv.c	24 Feb 2010 00:29:02 -0000	1.33
+++ gdb/solib-frv.c	14 Apr 2010 06:49:48 -0000
@@ -631,7 +631,6 @@ enable_break_failure_warning (void)
 
  */
 
-static int enable_break1_done = 0;
 static int enable_break2_done = 0;
 
 static int
@@ -642,15 +641,9 @@ enable_break2 (void)
   char **bkpt_namep;
   asection *interp_sect;
 
-  if (!enable_break1_done || enable_break2_done)
+  if (enable_break2_done)
     return 1;
 
-  enable_break2_done = 1;
-
-  /* First, remove all the solib event breakpoints.  Their addresses
-     may have changed since the last time we ran the program.  */
-  remove_solib_event_breakpoints ();
-
   interp_text_sect_low = interp_text_sect_high = 0;
   interp_plt_sect_low = interp_plt_sect_high = 0;
 
@@ -771,6 +764,22 @@ enable_break2 (void)
 	}
       addr = extract_unsigned_integer (addr_buf, sizeof addr_buf, byte_order);
 
+      if (solib_frv_debug)
+	fprintf_unfiltered (gdb_stdlog,
+	                    "enable_break: _dl_debug_addr[0..3] = %s\n",
+	                    hex_string_custom (addr, 8));
+
+      /* If it's zero, then the ldso hasn't initialized yet, and so
+         there are no shared libs yet loaded.  */
+      if (addr == 0)
+	{
+	  if (solib_frv_debug)
+	    fprintf_unfiltered (gdb_stdlog,
+	                        "enable_break: ldso not yet initialized\n");
+	  /* Do not warn, but mark to run again.  */
+	  return 0;
+	}
+
       /* Fetch the r_brk field.  It's 8 bytes from the start of
          _dl_debug_addr.  */
       if (target_read_memory (addr + 8, addr_buf, sizeof addr_buf) != 0)
@@ -800,9 +809,15 @@ enable_break2 (void)
       /* We're also done with the loadmap.  */
       xfree (ldm);
 
+      /* First, remove all the solib event breakpoints.  Their addresses
+         may have changed since the last time we ran the program.  */
+      remove_solib_event_breakpoints ();
+
       /* Now (finally!) create the solib breakpoint.  */
       create_solib_event_breakpoint (target_gdbarch, addr);
 
+      enable_break2_done = 1;
+
       return 1;
     }
 
@@ -847,7 +862,6 @@ enable_break (void)
       return 0;
     }
 
-  enable_break1_done = 1;
   create_solib_event_breakpoint (target_gdbarch,
 				 symfile_objfile->ei.entry_point);
 
@@ -997,7 +1011,6 @@ static void
 frv_clear_solib (void)
 {
   lm_base_cache = 0;
-  enable_break1_done = 0;
   enable_break2_done = 0;
   main_lm_addr = 0;
   if (main_executable_lm_info != 0)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH]: gdb: fdpic/frv: fix shared library loading

[Andrew Stubbs]

On 14/04/10 07:59, Mike Frysinger wrote:
> While I have no way of testing the FRV, the Blackfin FDPIC code is using this
> same base in a 100% copy&  paste method since we implemented FDPIC the same
> way as the FRV guys (I'll address this in the future).  This fix was required
> in order to handle shared libraries with Blackfin FDPIC properly, and I see no
> reason why it wouldn't also work for FRV (since the uClibc ldso FDPIC code is
> the same too and that's really what this is poking).

I'm currently working on FDPIC support for SH-2A uClinux, so I have been 
poking at this same area myself.

The FRV code would not work for me out of the box - it's a little too 
FRV specific, so I started work on another copy and paste, but thought 
better of it.

What I've got now is the bones of a solib-fdpic.c which is intended to 
be architecture independent. This should be possible since, so far, all 
the FDPIC Linux implementations I've seen use the same basic ABI, and 
the SH FDPIC will follow suit.

The first major departure from solib-frv.c is that I've implemented a 
new qXfer remote protocol packet to retrieve the load maps (a pseudo 
register did not seem like a good plan). This should be target 
independent also - I believe FRV has the same ptrace interface as SH, so 
it should Just Work.

My implementation isn't complete, by any means, but I intend to submit 
the first patches soon. I have not attempted to interfere with the FRV 
support in any way.

Andrew

Re: [PATCH]: gdb: fdpic/frv: fix shared library loading

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1971 bytes --]

On Thursday 15 April 2010 11:24:49 Andrew Stubbs wrote:
> On 14/04/10 07:59, Mike Frysinger wrote:
> > While I have no way of testing the FRV, the Blackfin FDPIC code is using
> > this same base in a 100% copy&  paste method since we implemented FDPIC
> > the same way as the FRV guys (I'll address this in the future).  This
> > fix was required in order to handle shared libraries with Blackfin FDPIC
> > properly, and I see no reason why it wouldn't also work for FRV (since
> > the uClibc ldso FDPIC code is the same too and that's really what this
> > is poking).
> 
> I'm currently working on FDPIC support for SH-2A uClinux, so I have been
> poking at this same area myself.
> 
> The FRV code would not work for me out of the box - it's a little too
> FRV specific, so I started work on another copy and paste, but thought
> better of it.
> 
> What I've got now is the bones of a solib-fdpic.c which is intended to
> be architecture independent. This should be possible since, so far, all
> the FDPIC Linux implementations I've seen use the same basic ABI, and
> the SH FDPIC will follow suit.

sad, because ive already done the same thing and it's working for Blackfin 
FDPIC.  but we've implemented FDPIC the same was as FRV.

> The first major departure from solib-frv.c is that I've implemented a
> new qXfer remote protocol packet to retrieve the load maps (a pseudo
> register did not seem like a good plan). This should be target
> independent also - I believe FRV has the same ptrace interface as SH, so
> it should Just Work.

i posted a Linux patch a while ago to move the FDPIC ptrace defines out of the 
arch-specific code and into the common ptrace code.  it was acked by everyone, 
but i guess i need to push andrew to merge it.

i dont have a problem with a pseudo register for the loadmaps ... seems like a 
simple/straightforward way of getting the info rather than having to extend 
the gdb protocol.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH]: gdb: fdpic/frv: fix shared library loading

[Kevin Buettner]

On Wed, 14 Apr 2010 02:59:38 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> 2010-04-14  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* solib-frv.c (enable_break1_done): Delete.
> 	(enable_break2): Do not check enable_break1_done.  Move the
> 	enable_break2_done setting and call to
> 	remove_solib_event_breakpoints() to the end.  Return without
> 	warning when the contents of _dl_debug_addr are 0.
> 	(enable_break): Do not set enable_break1_done.
> 	(frv_clear_solib): Likewise.

Okay, except for...

> +      /* First, remove all the solib event breakpoints.  Their addresses
> +         may have changed since the last time we ran the program.  */
> +      remove_solib_event_breakpoints ();

...this comment.  Just change it to read "Remove all the solib...".  (It's
not happening first anymore.)

Feel free to commit it after revising that comment.

Kevin