[patch] Fixed occasional failure to load a custom shared library

[patch] Fixed occasional failure to load a custom shared library

[Jan Kratochvil]

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

Hi,

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=146810

Inferior ENV{"LD_LIBRARY_PATH"} and/or ENV{"PATH"} may get ignored by
solib_open() resulting in
	Error while mapping shared library sections:
	libc++test4dl.so: Success.
and afterwards while calling a function from the shared library it crashes:
	(gdb) call randomthing()
	Program received signal SIGSEGV, Segmentation fault.
	randomthing () at c++test4dl.cxx:3
	3       void randomthing()
	The program being debugged was signaled while in a function called from GDB.

Trivia NULL vs. "" check issue.


Regards,
Jan Kratochvil

[-- Attachment #2: gdb-cvs20060621-solib_absolute_prefix_is_empty.patch --]
[-- Type: text/plain, Size: 1927 bytes --]

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=146810

Bug is still present in 2006-06-21 GDB CVS. The segfault is just a consequence of earlier failure to properly load the custom shared library:
Error while mapping shared library sections:
libc++test4dl.so: Success.


Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.84
diff -u -p -r1.84 solib.c
--- solib.c	25 Feb 2006 04:36:39 -0000	1.84
+++ solib.c	22 Jun 2006 11:24:40 -0000
@@ -146,13 +146,17 @@ solib_open (char *in_pathname, char **fo
   int found_file = -1;
   char *temp_pathname = NULL;
   char *p = in_pathname;
+  int solib_absolute_prefix_is_empty;
+
+  solib_absolute_prefix_is_empty = (!solib_absolute_prefix
+                                    || !*solib_absolute_prefix);
 
   while (*p && !IS_DIR_SEPARATOR (*p))
     p++;
 
   if (*p)
     {
-      if (! IS_ABSOLUTE_PATH (in_pathname) || solib_absolute_prefix == NULL)
+      if (! IS_ABSOLUTE_PATH (in_pathname) || solib_absolute_prefix_is_empty)
         temp_pathname = in_pathname;
       else
 	{
@@ -208,14 +212,14 @@ solib_open (char *in_pathname, char **fo
 					   &temp_pathname);
 
   /* If not found, next search the inferior's $PATH environment variable. */
-  if (found_file < 0 && solib_absolute_prefix == NULL)
+  if (found_file < 0 && solib_absolute_prefix_is_empty)
     found_file = openp (get_in_environ (inferior_environ, "PATH"),
 			OPF_TRY_CWD_FIRST, in_pathname, O_RDONLY | O_BINARY, 0,
 			&temp_pathname);
 
   /* If not found, next search the inferior's $LD_LIBRARY_PATH 
      environment variable. */
-  if (found_file < 0 && solib_absolute_prefix == NULL)
+  if (found_file < 0 && solib_absolute_prefix_is_empty)
     found_file = openp (get_in_environ (inferior_environ, "LD_LIBRARY_PATH"),
 			OPF_TRY_CWD_FIRST, in_pathname, O_RDONLY | O_BINARY, 0,
 			&temp_pathname);

Re: [patch] Fixed occasional failure to load a custom shared library

[Kevin Buettner]

On Thu, 22 Jun 2006 13:40:16 +0200
Jan Kratochvil <lace@jankratochvil.net> wrote:

Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.84
diff -u -p -r1.84 solib.c
--- solib.c	25 Feb 2006 04:36:39 -0000	1.84
+++ solib.c	22 Jun 2006 11:24:40 -0000
@@ -146,13 +146,17 @@ solib_open (char *in_pathname, char **fo
   int found_file = -1;
   char *temp_pathname = NULL;
   char *p = in_pathname;
+  int solib_absolute_prefix_is_empty;
+
+  solib_absolute_prefix_is_empty = (!solib_absolute_prefix
+                                    || !*solib_absolute_prefix);

I'd prefer to see this written as follows:

+  solib_absolute_prefix_is_empty = (solib_absolute_prefix == NULL
+                                    || *solib_absolute_prefix == 0);

The rest of your patch looks reasonable.  (You forgot to include a
ChangeLog entry though...)

Are you able to commit this yourself or do you need someone to commit
it for you?

Thanks for the patch!

Kevin

Re: [patch] Fixed occasional failure to load a custom shared library

[Jan Kratochvil]

Hi Kevin,

On Tue, 27 Jun 2006 03:00:51 +0200, Kevin Buettner wrote:
...
> +  solib_absolute_prefix_is_empty = (!solib_absolute_prefix
> +                                    || !*solib_absolute_prefix);
> 
> I'd prefer to see this written as follows:
> 
> +  solib_absolute_prefix_is_empty = (solib_absolute_prefix == NULL
> +                                    || *solib_absolute_prefix == 0);

OK, I tried to copy the style but apparently not perfectly.

...
> (You forgot to include a ChangeLog entry though...)

2006-06-22  Jan Kratochvil  <lace@jankratochvil.net>

	* solib.c (solib_open): Fixed NULL vs. "" refusal to load inferior
	shared libraries.


> Are you able to commit this yourself or do you need someone to commit
> it for you?

I do not have the commit rights, please commit it for me.
So far I do not have FSF copyright assignment for gdb, if it would be required
for this trivia patch.



Thanks,
Jan

Re: [patch] Fixed occasional failure to load a custom shared library

[Jan Kratochvil]

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

Hi Kevin,

I am now copyright assignment compliant (as Red Hat employee).

Could you please commit the updated patch?  Its ChangeLog entry:

2006-07-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* solib.c (solib_open): Fixed NULL vs. "" refusal to load inferior
	shared libraries.


Thanks,
Jan


On Tue, 27 Jun 2006 03:00:51 +0200, Kevin Buettner wrote:
> On Thu, 22 Jun 2006 13:40:16 +0200
> Jan Kratochvil <lace@jankratochvil.net> wrote:
> 
> Index: solib.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/solib.c,v
> retrieving revision 1.84
> diff -u -p -r1.84 solib.c
> --- solib.c	25 Feb 2006 04:36:39 -0000	1.84
> +++ solib.c	22 Jun 2006 11:24:40 -0000
> @@ -146,13 +146,17 @@ solib_open (char *in_pathname, char **fo
>    int found_file = -1;
>    char *temp_pathname = NULL;
>    char *p = in_pathname;
> +  int solib_absolute_prefix_is_empty;
> +
> +  solib_absolute_prefix_is_empty = (!solib_absolute_prefix
> +                                    || !*solib_absolute_prefix);
> 
> I'd prefer to see this written as follows:
> 
> +  solib_absolute_prefix_is_empty = (solib_absolute_prefix == NULL
> +                                    || *solib_absolute_prefix == 0);
> 
> The rest of your patch looks reasonable.  (You forgot to include a
> ChangeLog entry though...)
> 
> Are you able to commit this yourself or do you need someone to commit
> it for you?
> 
> Thanks for the patch!
> 
> Kevin

[-- Attachment #2: gdb-cvs20060721-solib_absolute_prefix_is_empty-v2.patch --]
[-- Type: text/plain, Size: 1938 bytes --]

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=146810

Bug is still present in 2006-07-21 GDB CVS. The segfault is just a consequence of earlier failure to properly load the custom shared library:
Error while mapping shared library sections:
libc++test4dl.so: Success.


Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.84
diff -u -p -r1.84 solib.c
--- solib.c	25 Feb 2006 04:36:39 -0000	1.84
+++ solib.c	21 Jul 2006 18:27:49 -0000
@@ -146,13 +146,17 @@ solib_open (char *in_pathname, char **fo
   int found_file = -1;
   char *temp_pathname = NULL;
   char *p = in_pathname;
+  int solib_absolute_prefix_is_empty;
+
+  solib_absolute_prefix_is_empty = (solib_absolute_prefix == NULL
+                                    || *solib_absolute_prefix == 0);
 
   while (*p && !IS_DIR_SEPARATOR (*p))
     p++;
 
   if (*p)
     {
-      if (! IS_ABSOLUTE_PATH (in_pathname) || solib_absolute_prefix == NULL)
+      if (! IS_ABSOLUTE_PATH (in_pathname) || solib_absolute_prefix_is_empty)
         temp_pathname = in_pathname;
       else
 	{
@@ -208,14 +212,14 @@ solib_open (char *in_pathname, char **fo
 					   &temp_pathname);
 
   /* If not found, next search the inferior's $PATH environment variable. */
-  if (found_file < 0 && solib_absolute_prefix == NULL)
+  if (found_file < 0 && solib_absolute_prefix_is_empty)
     found_file = openp (get_in_environ (inferior_environ, "PATH"),
 			OPF_TRY_CWD_FIRST, in_pathname, O_RDONLY | O_BINARY, 0,
 			&temp_pathname);
 
   /* If not found, next search the inferior's $LD_LIBRARY_PATH 
      environment variable. */
-  if (found_file < 0 && solib_absolute_prefix == NULL)
+  if (found_file < 0 && solib_absolute_prefix_is_empty)
     found_file = openp (get_in_environ (inferior_environ, "LD_LIBRARY_PATH"),
 			OPF_TRY_CWD_FIRST, in_pathname, O_RDONLY | O_BINARY, 0,
 			&temp_pathname);

Re: [patch] Fixed occasional failure to load a custom shared library

[Daniel Jacobowitz]

On Fri, Jul 21, 2006 at 08:38:29PM +0200, Jan Kratochvil wrote:
> Hi Kevin,
> 
> I am now copyright assignment compliant (as Red Hat employee).
> 
> Could you please commit the updated patch?  Its ChangeLog entry:
> 
> 2006-07-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* solib.c (solib_open): Fixed NULL vs. "" refusal to load inferior
> 	shared libraries.

I have committed this for you, after fixing separately the inferior
segfault.

I tried to reproduce the actual problem you were having, but I
couldn't.  No matter what I put in LD_LIBRARY_PATH or rpath, glibc
filled in the full path to the shared library.  Maybe I have a
newer runtime loader than you do, or something similar.  Anyway,
that's unimportant.


-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Fixed occasional failure to load a custom shared library

[Jan Kratochvil]

Hi Daniel,

On Mon, 09 Oct 2006 22:17:47 +0200, Daniel Jacobowitz wrote:
> On Fri, Jul 21, 2006 at 08:38:29PM +0200, Jan Kratochvil wrote:
...
> > 2006-07-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* solib.c (solib_open): Fixed NULL vs. "" refusal to load inferior
> > 	shared libraries.
> 
> I have committed this for you, after fixing separately the inferior
> segfault.
> 
> I tried to reproduce the actual problem you were having, but I
> couldn't.  No matter what I put in LD_LIBRARY_PATH or rpath, glibc
> filled in the full path to the shared library.  Maybe I have a
> newer runtime loader than you do, or something similar.  Anyway,
> that's unimportant.

If you would use `-Wl,-rpath,$ORIGIN' or relative pathname for $LD_LIBRARY_PATH.
The latter case was testcased in:
	http://sourceware.org/ml/gdb-patches/2006-09/msg00009.html


Thanks,
Jan

Re: [patch] Fixed occasional failure to load a custom shared library

[Daniel Jacobowitz]

On Tue, Oct 10, 2006 at 12:50:42AM +0200, Jan Kratochvil wrote:
> If you would use `-Wl,-rpath,$ORIGIN' or relative pathname for $LD_LIBRARY_PATH.
> The latter case was testcased in:
> 	http://sourceware.org/ml/gdb-patches/2006-09/msg00009.html

In my glibc, setting LD_LIBRARY_PATH to a relative path makes no
difference.  Things get canonicalized anyway.  I also tried with
$ORIGIN.  I wonder if the Fedora glibc behaves differently in this
respect.

-- 
Daniel Jacobowitz
CodeSourcery