[PATCH] elf: Allow vDSO as a direct dependency (BZ 33335)

[Florian Weimer]

* Adhemerval Zanella:

> The vDSO is treated as a special link map, and using it as a direct
> dependency shows two issues:
>
> 1. Binding uses the implicit vDSO provided by the kernel (even though a
>    library is explicitly provided);
> 2. And depending on how the binary is linked, it can cause loops that
>    trigger an inconsistency during process startup.

What are the expected semantics if a library is provided?  Would we use
it for clock_gettime etc. instead of the kernel-provided object?

> diff --git a/elf/Makefile b/elf/Makefile
> index 07b456f4f5..b2ed018b10 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1447,7 +1447,18 @@ CFLAGS-tst-_dl_addr_inside_object.c += $(PIE-ccflag)
>  endif
>  
>  ifeq ($(run-built-tests),yes)
> -tests-special += $(objpfx)tst-origin.out
> +tests-special += \
> +  $(objpfx)tst-origin.out \
> +  $(objpfx)tst-vdso-2.out \
> +  # tests-special
> +endif
> +
> +ifndef (,$(vdso-name))
> +tests += tst-vdso-1

Confusion between ifndef and ifneq?
(There are multiple occurrences of this.)

If this lives in sysdeps/unix/sysv/linux/x86_64/Makefile, the check
would be unnecessary.

> diff --git a/elf/dl-object.c b/elf/dl-object.c
> index 51d3704edc..8325258c2f 100644
> --- a/elf/dl-object.c
> +++ b/elf/dl-object.c
> @@ -98,6 +98,7 @@ _dl_new_object (char *realname, const char *libname, int type,
>    new->l_real = new;
>    new->l_symbolic_searchlist.r_list = (struct link_map **) ((char *) (new + 1)
>  							    + audit_space);
> +  new->l_vdso = !!(mode & __RTLD_VDSO);
>  
>    new->l_libname = newname
>      = (struct libname_list *) (new->l_symbolic_searchlist.r_list + 1);
> diff --git a/elf/linux-vdso.c b/elf/linux-vdso.c
> new file mode 100644
> index 0000000000..abbd88e3fb

I believe this should go under sysdeps/unix/sysv/linux.

> diff --git a/include/link.h b/include/link.h
> index 78fa288b9d..78ce1edb26 100644
> --- a/include/link.h
> +++ b/include/link.h
> @@ -213,6 +213,7 @@ struct link_map
>  					       needs to process this
>  					       lt_library map.  */
>      unsigned int l_tls_in_slotinfo:1; /* TLS slotinfo updated in dlopen.  */
> +    unsigned int l_vdso:1;  /* Nonzero if object refers to a vDSO.  */
>  
>      /* NODELETE status of the map.  Only valid for maps of type
>         lt_loaded.  Lazy binding sets l_nodelete_active directly,

This starts a new byte.  I don't think this matters unless we want to
backport this because we should not change the layout of struct link_map
in a backport.

Looking at the existing bits, I think we could replace l_link_map with
an lt_executable check if we ever need to backport this.  Or add a new
lt_vdso type.

> diff --git a/sysdeps/unix/sysv/linux/aarch64/Makefile b/sysdeps/unix/sysv/linux/aarch64/Makefile
> index 6741f8ec00..569f840e1e 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/Makefile
> +++ b/sysdeps/unix/sysv/linux/aarch64/Makefile
> @@ -158,3 +158,5 @@ endif
>  
>  abi-lp64-condition := !defined __AARCH64EB__
>  abi-lp64_be-condition := defined __AARCH64EB__
> +
> +vdso-name := linux-vdso

Wouldn't this go into shlib-versions, something like this:

linux-vdso=linux-vdso.1

And then you'd use $(linux-vdso.so-version).  This may end up in
<gnu/lib-names.h>, but I don't think that's necessarily a bad thing?
It's ABI, after all.

Thanks,
Florian