Re: [PATCH] Add RTLD_RELOAD to dlopen

[Samuel Thibault <samuel dot thibault at ens-lyon dot org>]

Hello,

So, is it OK to add this? Considering that dlmopen() brings us far from
enough factorization for our needs, and dlopen() currently never reloads
unless a real fat copy of the file is done (hardlinks/symlinks don't
work, as it is based on st_ino).

Samuel

Samuel Thibault, on jeu. 20 juil. 2017 14:15:17 -0500, wrote:
> In our parallel programming projects, we would like to load some DSO
> several times within the same process, because we want to share the
> addresse space for passing data pointers between parallel executions,
> and the DSO has global variables and such which we want to see
> duplicated.
> 
> Unfortunately, dlopen() does not re-load the DSO when it is already
> loaded. One workaround is to cp the file under another name, but that's
> ugly and does not share the memory pages.
> 
> The patch proposed here simply adds an RTLD_RELOAD flag which disables
> checking for the DSO being already loaded, thus always loading the DSO
> again. There is no actual code modification, only the addition of two
> if()s and reindent.
> 
> Samuel
> 
> 2017-07-19  Samuel Thibault  <samuel.thibault@ens-lyon.org>
> 
> 	* bits/dlfcn.h (RTLD_RELOAD): New macro
> 	* sysdeps/mips/bits/dlfcn.h (RTLD_RELOAD): New macro
> 	* dlfcn/dlopen.c (dlopen_doit): Let args->mode contain RTLD_RELOAD.
> 	* elf/dl-load.c (_dl_map_object_from_fd): Do not match the file id
> 	when mode contains RTLD_RELOAD.
> 	(_dl_map_object): Do not match the file name when mode contains
> 	RTLD_RELOAD.
> 	* elf/tst-reload.c: New file.
> 
> diff --git a/bits/dlfcn.h b/bits/dlfcn.h
> index 7786d8f939..371e8a0e3c 100644
> --- a/bits/dlfcn.h
> +++ b/bits/dlfcn.h
> @@ -26,6 +26,7 @@
>  #define	RTLD_BINDING_MASK   0x3	/* Mask of binding time value.  */
>  #define RTLD_NOLOAD	0x00004	/* Do not load the object.  */
>  #define RTLD_DEEPBIND	0x00008	/* Use deep binding.  */
> +#define RTLD_RELOAD	0x00010	/* Reload the object.  */
>  
>  /* If the following bit is set in the MODE argument to `dlopen',
>     the symbols of the loaded object and its dependencies are made
> diff --git a/dlfcn/dlopen.c b/dlfcn/dlopen.c
> index 22120655d2..d317565b7f 100644
> --- a/dlfcn/dlopen.c
> +++ b/dlfcn/dlopen.c
> @@ -60,7 +60,7 @@ dlopen_doit (void *a)
>  
>    if (args->mode & ~(RTLD_BINDING_MASK | RTLD_NOLOAD | RTLD_DEEPBIND
>  		     | RTLD_GLOBAL | RTLD_LOCAL | RTLD_NODELETE
> -		     | __RTLD_SPROF))
> +		     | RTLD_RELOAD | __RTLD_SPROF))
>      _dl_signal_error (0, NULL, NULL, _("invalid mode parameter"));
>  
>    args->new = GLRO(dl_open) (args->file ?: "", args->mode | __RTLD_DLOPEN,
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index c1b6d4ba0f..6cd28dc15e 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -894,20 +894,23 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>      }
>  
>    /* Look again to see if the real name matched another already loaded.  */
> -  for (l = GL(dl_ns)[nsid]._ns_loaded; l != NULL; l = l->l_next)
> -    if (!l->l_removed && _dl_file_id_match_p (&l->l_file_id, &id))
> -      {
> -	/* The object is already loaded.
> -	   Just bump its reference count and return it.  */
> -	__close (fd);
> +  if ((mode & RTLD_RELOAD) == 0)
> +    {
> +      for (l = GL(dl_ns)[nsid]._ns_loaded; l != NULL; l = l->l_next)
> +	if (!l->l_removed && _dl_file_id_match_p (&l->l_file_id, &id))
> +	  {
> +	    /* The object is already loaded.
> +	       Just bump its reference count and return it.  */
> +	    __close (fd);
>  
> -	/* If the name is not in the list of names for this object add
> -	   it.  */
> -	free (realname);
> -	add_name_to_object (l, name);
> +	    /* If the name is not in the list of names for this object add
> +	       it.  */
> +	    free (realname);
> +	    add_name_to_object (l, name);
>  
> -	return l;
> -      }
> +	    return l;
> +	  }
> +    }
>  
>  #ifdef SHARED
>    /* When loading into a namespace other than the base one we must
> @@ -1902,33 +1905,36 @@ _dl_map_object (struct link_map *loader, const char *name,
>    assert (nsid < GL(dl_nns));
>  
>    /* Look for this name among those already loaded.  */
> -  for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next)
> +  if ((mode & RTLD_RELOAD) == 0)
>      {
> -      /* If the requested name matches the soname of a loaded object,
> -	 use that object.  Elide this check for names that have not
> -	 yet been opened.  */
> -      if (__glibc_unlikely ((l->l_faked | l->l_removed) != 0))
> -	continue;
> -      if (!_dl_name_match_p (name, l))
> +      for (l = GL(dl_ns)[nsid]._ns_loaded; l; l = l->l_next)
>  	{
> -	  const char *soname;
> -
> -	  if (__glibc_likely (l->l_soname_added)
> -	      || l->l_info[DT_SONAME] == NULL)
> +	  /* If the requested name matches the soname of a loaded object,
> +	     use that object.  Elide this check for names that have not
> +	     yet been opened.  */
> +	  if (__glibc_unlikely ((l->l_faked | l->l_removed) != 0))
>  	    continue;
> +	  if (!_dl_name_match_p (name, l))
> +	    {
> +	      const char *soname;
>  
> -	  soname = ((const char *) D_PTR (l, l_info[DT_STRTAB])
> -		    + l->l_info[DT_SONAME]->d_un.d_val);
> -	  if (strcmp (name, soname) != 0)
> -	    continue;
> +	      if (__glibc_likely (l->l_soname_added)
> +		  || l->l_info[DT_SONAME] == NULL)
> +		continue;
>  
> -	  /* We have a match on a new name -- cache it.  */
> -	  add_name_to_object (l, soname);
> -	  l->l_soname_added = 1;
> -	}
> +	      soname = ((const char *) D_PTR (l, l_info[DT_STRTAB])
> +			+ l->l_info[DT_SONAME]->d_un.d_val);
> +	      if (strcmp (name, soname) != 0)
> +		continue;
>  
> -      /* We have a match.  */
> -      return l;
> +	      /* We have a match on a new name -- cache it.  */
> +	      add_name_to_object (l, soname);
> +	      l->l_soname_added = 1;
> +	    }
> +
> +	  /* We have a match.  */
> +	  return l;
> +	}
>      }
>  
>    /* Display information if we are debugging.  */
> diff --git a/elf/tst-reload.c b/elf/tst-reload.c
> new file mode 100644
> index 0000000000..1fb25e7c97
> --- /dev/null
> +++ b/elf/tst-reload.c
> @@ -0,0 +1,83 @@
> +/* Verify that RTLD_NOLOAD works as expected.
> +
> +   Copyright (C) 2016-2017 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <dlfcn.h>
> +#include <stdio.h>
> +#include <gnu/lib-names.h>
> +
> +static int
> +do_test (void)
> +{
> +  /* Test that no object is loaded with RTLD_NOLOAD.  */
> +  void *h1 = dlopen (LIBM_SO, RTLD_LAZY | RTLD_NOLOAD);
> +  if (h1 != NULL)
> +    {
> +      printf ("h1: DSO has been loaded while it should have not\n");
> +      return 1;
> +    }
> +
> +  /* Test that loading an already loaded object returns the same handle.  */
> +  void *h2 = dlopen (LIBM_SO, RTLD_LAZY);
> +  if (h2 == NULL)
> +    {
> +      printf ("h2: failed to open DSO: %s\n", dlerror ());
> +      return 1;
> +    }
> +  void *h3 = dlopen (LIBM_SO, RTLD_LAZY);
> +  if (h3 == NULL)
> +    {
> +      printf ("h3: failed to open DSO: %s\n", dlerror ());
> +      return 1;
> +    }
> +  if (h3 != h2)
> +    {
> +      printf ("h3: should return the same object\n");
> +      return 1;
> +    }
> +
> +  /* Test that reloading an already loaded object returns a different handle.  */
> +  void *h4 = dlopen (LIBM_SO, RTLD_LAZY | RTLD_RELOAD);
> +  if (h4 == NULL)
> +    {
> +      printf ("h4: failed to open DSO: %s\n", dlerror ());
> +      return 1;
> +    }
> +  if (h4 == h2)
> +    {
> +      printf ("h4: should not return the same object\n");
> +      return 1;
> +    }
> +
> +  /* Cleanup */
> +  if (dlclose (h4) != 0)
> +    {
> +      printf ("h4: dlclose failed: %s\n", dlerror ());
> +      return 1;
> +    }
> +  if (dlclose (h2) != 0)
> +    {
> +      printf ("h2: dlclose failed: %s\n", dlerror ());
> +      return 1;
> +    }
> +
> +  return 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> diff --git a/sysdeps/mips/bits/dlfcn.h b/sysdeps/mips/bits/dlfcn.h
> index 95b2fa0973..c4bf5e149b 100644
> --- a/sysdeps/mips/bits/dlfcn.h
> +++ b/sysdeps/mips/bits/dlfcn.h
> @@ -26,6 +26,7 @@
>  #define RTLD_BINDING_MASK  0x3	/* Mask of binding time value.  */
>  #define RTLD_NOLOAD	0x00008	/* Do not load the object.  */
>  #define RTLD_DEEPBIND	0x00010	/* Use deep binding.  */
> +#define RTLD_RELOAD	0x00020	/* Reload the object.  */
>  
>  /* If the following bit is set in the MODE argument to `dlopen',
>     the symbols of the loaded object and its dependencies are made

-- 
Samuel
<N> M.  MIMRAM  Samuel Antonin
<N> en voila un qui etait predestiné
 -+- #ens-mim - Quelles gueules qu'ils ont les ptits nouveaux ? -+-