Re: [PATCHv4][PING][BZ #17833] _dl_close_worker() does not release inconsistent objects.

[Pavel Kopyl <p dot kopyl at samsung dot com>]

On 05/28/2015 01:06 AM, H.J. Lu wrote:
> On Wed, May 27, 2015 at 2:38 PM, Pavel Kopyl <p.kopyl@samsung.com> wrote:
> > > > > > AFAIU DF_1_NODELETE change is necessary. Otherwise unique symbols force
> > > > > > library to remain loaded in half-initialized state (with missing
> > > > > > dependencies). This usecase is demonstrated by the testcase in patch.
> > > > > But unique symbols != DF_1_NODELETE:
> > > >
> > > > DF_1_NODELETE is set in do_lookup_unique at runtime (Paul, am I right?):
> > > >
> > > >         enter_unique_sym (entries, size,
> > > >                           new_hash, strtab + sym->st_name, sym, map);
> > > >
> > > >         if (map->l_type == lt_loaded)
> > > >           /* Make sure we don't unload this object by
> > > >              setting the appropriate flag.  */
> > > >           ((struct link_map *) map)->l_flags_1 |= DF_1_NODELETE;
> > > >
> > > > -Y
> > > Should we set DF_1_NODELETE when dlopen fails?  What happens
> > > when dlopen a  DF_1_NODELETE fails?  Do we keep it in memory
> > > even when dopen fails?
> > As I know there are following ways how to set DF_1_NODELETE flag for a
> > library.
> >
> > 1. Unique symbols.
> > 2, Load with RTLD_NODELETE flag.
> > 3. Link with "-z nodelete" option.
> >
> > As for RTLD_NODELETE, there is no problem: dlopen exits in case of error
> > before DF_1_NODELETE is set.
> > If dlopen fails both in first and third cases this leads to locking
> > libraries in half-initialized state. I think this is a bug.
> > Suggested patch fixes both usecases.
> Assume there is a problem with DF_1_NODELETE, which can be
> set explicitly as well as implicitly.  Your testcase only covers
> implicitly DF_1_NODELETE, not explicitly  DF_1_NODELETE.
> I think we should verify the status of explicitly  DF_1_NODELETE.
>
> If explicitly  DF_1_NODELETE works fine, then we only have a
> problem with implicitly DF_1_NODELETE.
>
> If explicitly  DF_1_NODELETE doesn't work, we need to fix it
> first and implicitly DF_1_NODELETE may just work.

Ok, I prepared the following testcase.

tst-nodelete.cc:
#include <stdio.h>
#include <stdlib.h>
#include <dlfcn.h>

int
main (void)
{
  int ret = 0;

  /* This is a test for correct handling of dlopen failures for library 
that
     is loaded with RTLD_NODELETE flag. The first dlopen should fail 
because
     of undefined symbols in shared library. The second dlopen then 
verifies
     that library was properly unloaded.  */
  if (dlopen ("./tst-nodeletelib.so", RTLD_NOW | RTLD_NODELETE) != NULL
      || dlopen ("./tst-nodeletelib.so", RTLD_LAZY | RTLD_NOLOAD) != NULL)
    {
      printf("RTLD_NOLOAD test failed\n");
      ret = 1;
    }

  /* This is a test for correct handling of dlopen failures for library 
that
     is linked with '-z nodelete' option and hence has DF_1_NODELETE flag.
     The first dlopen should fail because of undefined symbols in shared
     library. The second dlopen then verifies that library was properly
     unloaded.  */
  if (dlopen ("./tst-znodeletelib.so", RTLD_NOW) != NULL
      || dlopen ("./tst-znodeletelib.so", RTLD_LAZY | RTLD_NOLOAD) != NULL)
    {
      printf("-z \'nodelete\' test failed\n");
      ret = 1;
    }

   /* This is a test for correct handling of dlopen failures for library
     with unique symbols. The first dlopen should fail because of undefined
     symbols in shared library. The second dlopen then verifies that 
library
     was properly unloaded.  */
  if (dlopen ("./tst-unique5lib.so", RTLD_NOW) != NULL
      || dlopen ("./tst-unique5lib.so", RTLD_LAZY | RTLD_NOLOAD) != NULL)
    {
      printf("Unique symbols test failed\n");
      ret = 1;
    }

  if (!ret)
    printf ("SUCCESS\n");

  return ret;
}


tst-unique5lib.cc:
extern int not_exist (void);

inline int make_unique (void)
{
/* Static variables in inline functions and classes
   generate STB_GNU_UNIQUE symbols.  */
  static int unique;
  return ++unique;
}

int foo (void)
{
  return make_unique () + not_exist ();
}


tst-nodeletelib.cc:
extern int not_exist (void);

int foo (void)
{
  return  not_exist ();
}


g++ -o tst-nodelete tst-nodelete.cc -ldl
g++ -shared -fPIC -o tst-nodeletelib.so tst-nodeletelib.cc
g++ -shared -fPIC -Wl,-z,nodelete -o tst-znodeletelib.so tst-nodeletelib.cc
./tst-nodelete
-z 'nodelete' test failed
Unique symbols test failed


Thus, besides unique symbols we have a problem with explicitly 
DF_1_NODELETE when library is built with "-Wl,-z,nodelete".
I checked that patch fixes this usecase too. Now I'm going to update 
testcase in make check.


-Pavel