[rfa] Shared object matching for solib-som.c

[rfa] Shared object matching for solib-som.c

[Randolph Chung]

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

Dave Anglin brought this problem to my attention -

Sometimes gdb does not properly relocate symbols in shared objects, 
leading to wrong backtraces and all sorts of problems. The problem seems 
to be that the SOM solib code builds section offsets by comparing object 
filenames between objfile->name so so_list->so_name, and because the two 
filenames could have gone through different types of postprocessing, 
they can differ subtly that makes the matching fail.

In the case I debugged, gdb was comparing
/mnt/gnu/gcc-3.3/objdir/hppa2.0w-hp-hpux11.11/libjava/.libs/libgcj.sl.7
against
/mnt/gnu/gcc-3.3/objdir/hppa2.0w-hp-hpux11.11/./libjava/.libs/libgcj.sl.7

The attached patch changes the logic so that only the last component in 
the filename is compared. This of course assumes that you have not 
linked two shared objects with the same filename (hopefully not likely). 
This is not very robust either, but I think this is better than what we 
have before.

I notice that most of the solib variants do not need to do this type of 
processing; unfortunately I haven't figured out a better way to handle 
the SOM case. Perhaps somebody more familiar with the solib code/SOM can 
comment?

Meanwhile, is this patch ok?

randolph

[-- Attachment #2: solib.diff --]
[-- Type: text/x-patch, Size: 1407 bytes --]

2006-04-19  Randolph Chung  <tausq@debian.org>

	* solib-som.c (som_relocate_section_addresses): Cleanup formatting.
	(som_solib_section_offsets): Only compare filename to match shared
	object.

Index: solib-som.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-som.c,v
retrieving revision 1.9
diff -u -p -r1.9 solib-som.c
--- solib-som.c	24 Mar 2006 23:49:56 -0000	1.9
+++ solib-som.c	19 Apr 2006 00:57:21 -0000
@@ -127,11 +127,9 @@ som_relocate_section_addresses (struct s
     }
   else if (aflag & SEC_DATA)
     {
-      sec->addr    += so->lm_info->data_start; 
+      sec->addr    += so->lm_info->data_start;
       sec->endaddr += so->lm_info->data_start;
     }
-  else
-    ;
 }
 
 /* This hook gets called just before the first instruction in the
@@ -787,7 +794,21 @@ som_solib_section_offsets (struct objfil
     {
       /* Oh what a pain!  We need the offsets before so_list->objfile
          is valid.  The BFDs will never match.  Make a best guess.  */
-      if (strstr (objfile->name, so_list->so_name))
+      char *p1, *p2;
+
+      p1 = strrchr(objfile->name, '/');
+      p2 = strrchr(so_list->so_name, '/');
+
+      if (p1) 
+        p1++; 
+      else 
+        p1 = objfile->name;
+      if (p2)
+        p2++;
+      else
+        p2 = so_list->so_name;
+
+      if (strcmp (p1, p2) == 0)
 	{
 	  asection *private_section;
 

Re: [rfa] Shared object matching for solib-som.c

[Eli Zaretskii]

> Date: Wed, 19 Apr 2006 09:07:09 +0800
> From: Randolph Chung <randolph@tausq.org>
> 
> @@ -787,7 +794,21 @@ som_solib_section_offsets (struct objfil
>      {
>        /* Oh what a pain!  We need the offsets before so_list->objfile
>           is valid.  The BFDs will never match.  Make a best guess.  */
> -      if (strstr (objfile->name, so_list->so_name))
> +      char *p1, *p2;
> +
> +      p1 = strrchr(objfile->name, '/');
> +      p2 = strrchr(so_list->so_name, '/');
> +
> +      if (p1) 
> +        p1++; 
> +      else 
> +        p1 = objfile->name;
> +      if (p2)
> +        p2++;
> +      else
> +        p2 = so_list->so_name;
> +
> +      if (strcmp (p1, p2) == 0)
>  	{
>  	  asection *private_section;

Please don't use literal slash characters in GDB sources: they are
non-portable.  AFAICS, you simply need to use lbasename here, on both
file names.

Re: [rfa] Shared object matching for solib-som.c

[Randolph Chung]

> Please don't use literal slash characters in GDB sources: they are
> non-portable.  AFAICS, you simply need to use lbasename here, on both
> file names.

I guess you mean "basename" instead of "lbasename". I could, and I
started with that, although basename allocates memory so it's slightly
more messy.

While I agree with your portability comment, I hope nobody is crazy
enough to use SOM outside of hpux :)

Anyway, I'll redo this using basename.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [rfa] Shared object matching for solib-som.c

[Eli Zaretskii]

> Date: Wed, 19 Apr 2006 16:29:09 +0800
> From: Randolph Chung <randolph@tausq.org>
> CC:  gdb-patches@sources.redhat.com
> 
> > Please don't use literal slash characters in GDB sources: they are
> > non-portable.  AFAICS, you simply need to use lbasename here, on both
> > file names.
> 
> I guess you mean "basename" instead of "lbasename".

No, I mean lbasename, the one from libiberty.  That solves a problem
of basename being not portable enough (I think we never use basename
in GDB, only lbasename, but I might be wrong).

> While I agree with your portability comment, I hope nobody is crazy
> enough to use SOM outside of hpux :)

Probably not, but people frequently lift code by copy/paste.  Anyway,
these are our coding standards, AFAIK.

> Anyway, I'll redo this using basename.

Thanks, but please use lbasename.

Re: [rfa] Shared object matching for solib-som.c

[Daniel Jacobowitz]

On Wed, Apr 19, 2006 at 09:07:09AM +0800, Randolph Chung wrote:
> I notice that most of the solib variants do not need to do this type of 
> processing; unfortunately I haven't figured out a better way to handle 
> the SOM case. Perhaps somebody more familiar with the solib code/SOM can 
> comment?

No one else needs to do this, in fact.  This is from the symfile
section_offsets callback; but why can't you do this in the
solib relocate_section_addresses callback instead?  In fact, you
already do, so it is not at all clear to me why you need to adjust the
offsets here.  Other targets only need to adjust the one copy in the
section table.

Ah, I think this normally works via
build_section_addr_info_from_section_table.  Which produces the second
parameter for som_symfile_offsets.  Which is now valid for shared
libraries.  It's just that the way that function _uses_ it is bogus for
shared libraries.  Instead of finding the .text offset you'd have to
honor the section indexes.  See default_symfile_offsets.

Also the two solib-som.c functions setting offsets use different data
offsets, so beware that.

-- 
Daniel Jacobowitz
CodeSourcery