Re: [PATCH] Bug 11941: Improper assert map->l_init_called in dlclose

[Florian Weimer <fweimer at redhat dot com>]

On 12/22/2016 06:36 AM, Carlos O'Donell wrote:

> +  /* We must take the lock to examine the contents of map whose
> +     l_flags_1 or l_direct_opencount may be modified by concurrent
> +     dlopen calls.  */
> +  __rtld_lock_lock_recursive (GL(dl_load_lock));
> +
>    /* First see whether we can remove the object at all.  */
>    if (__glibc_unlikely (map->l_flags_1 & DF_1_NODELETE))
>      {
> -      assert (map->l_init_called);
>        /* Nope.  Do nothing.  */
> +      __rtld_lock_unlock_recursive (GL(dl_load_lock));
>        return;
>      }
>
>    if (__builtin_expect (map->l_direct_opencount, 1) == 0)
>      _dl_signal_error (0, map->l_name, NULL, N_("shared object not open"));

Missing unlock before non-local exit.

The plugin should have some reference to a symbol defined by the other 
DSO, so that if ld applies --as-needed by default for some reason, the 
test still has the expected behavior.

> diff --git a/elf/tst-nodelete-dlclose-dso.c b/elf/tst-nodelete-dlclose-dso.c

> +void (*plugin_func)(void);

Missing space before paramter list.  Those variables could be static.

> +#define LIB_PLUGIN "tst-nodelete-dlclose-plugin.so"
> +
> +void
> +primary(void)

Likewise.

> +{
> +  char *error;
> +
> +  plugin_lib = dlopen (LIB_PLUGIN, RTLD_NOW | RTLD_LOCAL | RTLD_NODELETE);
> +  if (!plugin_lib)

Comparison against NULL is required by the style guide.

> +__attribute__((destructor))
> +void
> +primary_dtor(void)

Missing space before parameter list.  Function could be static.

> diff --git a/elf/tst-nodelete-dlclose-plugin.c b/elf/tst-nodelete-dlclose-plugin.c

> +plugin(void)

Likewise.

> +{
> +  printf("INFO: Calling plugin function.\n");

Likewise.

> +}
> +
> +__attribute__((destructor))

Likewise.

> +static void
> +plugin_dtor(void)

Likewise.

> +{
> +  printf("INFO: Calling plugin destructor.\n");

Likewise.

> diff --git a/elf/tst-nodelete-dlclose.c b/elf/tst-nodelete-dlclose.c

> +extern void primary(void);

Likewise.

> +
> +static int
> +do_test(void)

Likewise.

> +{
> +  printf ("INFO: Starting applicaiton.\n");

Typo: application

Thanks,
Florian