From: Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org>
To: libc-alpha@sourceware.org, Vivek Das Mohapatra <vivek@collabora.com>
Subject: Re: [RFC][PATCH v12 4/8] Add DT_GNU_FLAGS_1/DF_GNU_1_UNIQUE to glibc DSOs (bug 22745)
Date: Mon, 9 Aug 2021 13:48:06 -0300 [thread overview]
Message-ID: <6209df75-2d78-b188-4041-fe77849e224c@linaro.org> (raw)
In-Reply-To: <20210708163255.812-5-vivek@collabora.com>
On 08/07/2021 13:32, Vivek Das Mohapatra via Libc-alpha wrote:
> libc.so, libpthread.so etc should have the new unique-dso-by-default
> flag set to allow dlmopen to work better (libc et al instance shared
> by default when DSOs dlmopened into a new namespace).
The patch looks ok, some comments below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> Makeconfig | 3 +++
> Makerules | 18 ++++++++++++++++-
> config.make.in | 1 +
> configure | 42 ++++++++++++++++++++++++++++++++++++---
> configure.ac | 31 ++++++++++++++++++++++++++---
> elf/Makefile | 3 +++
> elf/dynamic-notes.c | 4 ++++
> extra-lib.mk | 3 +++
> htl/Makefile | 3 +++
> iconvdata/Makefile | 3 +++
> iconvdata/extra-module.mk | 4 ++++
> nptl/Makefile | 7 ++++++-
> 12 files changed, 114 insertions(+), 8 deletions(-)
> create mode 100644 elf/dynamic-notes.c
>
> diff --git a/Makeconfig b/Makeconfig
> index efc7351d71..baa893840e 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -398,6 +398,9 @@ LDFLAGS-lib.so += -Wl,-z,now
> # Extra flags for dynamically linked non-test main programs.
> link-extra-flags += -Wl,-z,now
> endif
> +ifeq ($(ld-zunique),yes)
> +LDFLAGS-lib.so += -Wl,-z,unique
> +endif
>
> # Command to run after every final link (executable or shared object).
> # This is invoked with $(call after-link,...), so it should operate on
Ok.
> diff --git a/Makerules b/Makerules
> index 596fa68376..66c1251754 100644
> --- a/Makerules
> +++ b/Makerules
> @@ -581,7 +581,8 @@ $(common-objpfx)shlib.lds: $(common-objpfx)config.make $(..)Makerules
> PROVIDE(__start___libc_IO_vtables = .);\
> __libc_IO_vtables : { *(__libc_IO_vtables) }\
> PROVIDE(__stop___libc_IO_vtables = .);\
> - /DISCARD/ : { *(.gnu.glibc-stub.*) }@'
> + /DISCARD/ : { *(.gnu.glibc-stub.*) }@' \
> + -e 's/^.*\*(\.dynamic).*$$/ .dynamic : { *dynamic-notes.os(.dynamic) *(.dynamic) }/'
> test -s $@T
> mv -f $@T $@
> common-generated += shlib.lds
Ok.
> @@ -636,6 +637,16 @@ build-shlib-objlist = $(build-module-helper-objlist) \
> # Also omits crti.o and crtn.o, which we do not want
> # since we define our own `.init' section specially.
> LDFLAGS-c.so = -nostdlib -nostartfiles
> +
> +# The stub dynamic-notes.os should not have weak/undefined symbol magic in it.
> +# It's not really part of the internals, rather it is a vector for linker magic
> +# which we need when the linker isn't new enough:
> +%/dynamic-notes.os: CPPFLAGS += -UMODULE_NAME
> +
> +ifeq ($(ld-zunique),yes)
> +LDFLAGS-c.so += -Wl,-z,unique
> +endif
> +
> # But we still want to link libc.so against $(libc.so-gnulib).
> LDLIBS-c.so += $(libc.so-gnulib)
> # Give libc.so an entry point and make it directly runnable itself.
Ok.
> @@ -706,6 +717,11 @@ $(common-objpfx)linkobj/libc.so: $(common-objpfx)linkobj/libc_pic.a \
> $(build-shlib)
> $(call after-link,$@)
>
> +ifneq ($(ld-zunique),yes)
> +$(common-objpfx)libc.so: $(common-objpfx)elf/dynamic-notes.os
> +$(common-objpfx)linkobj/libc.so: $(common-objpfx)elf/dynamic-notes.os
> +endif
> +
> ifeq ($(build-shared),yes)
> $(common-objpfx)libc.so: $(common-objpfx)libc.map
> endif
OK.
> diff --git a/config.make.in b/config.make.in
> index cbf59114b0..8755490c13 100644
> --- a/config.make.in
> +++ b/config.make.in
> @@ -72,6 +72,7 @@ have-cc-with-libunwind = @libc_cv_cc_with_libunwind@
> fno-unit-at-a-time = @fno_unit_at_a_time@
> bind-now = @bindnow@
> have-hash-style = @libc_cv_hashstyle@
> +ld-zunique = @ld_zunique@
> use-default-link = @use_default_link@
> have-cxx-thread_local = @libc_cv_cxx_thread_local@
> have-loop-to-function = @libc_cv_cc_loop_to_function@
Ok.
> diff --git a/configure b/configure
> index 9619c10991..a53243a178 100755
> --- a/configure
> +++ b/configure
> @@ -625,6 +625,7 @@ libc_cv_cc_nofma
> libc_cv_mtls_dialect_gnu2
> fno_unit_at_a_time
> libc_cv_has_glob_dat
> +ld_zunique
> libc_cv_hashstyle
> libc_cv_fpie
> libc_cv_z_execstack
> @@ -6074,9 +6075,38 @@ fi
> $as_echo "$libc_cv_hashstyle" >&6; }
>
>
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for linker DT_GNU_FLAGS_1/ DF_GNU_1_UNIQUE support" >&5
> +$as_echo_n "checking for linker DT_GNU_FLAGS_1/ DF_GNU_1_UNIQUE support... " >&6; }
> +if ${libc_cv_ld_zunique+:} false; then :
> + $as_echo_n "(cached) " >&6
> +else
> + cat > conftest.ld.c <<\EOF
> +int x (void) { return 0; }
> +EOF
> +
> +libc_cv_ld_zunique=no;
> +
> +if { ac_try='${CC-cc} -Wl,--fatal-warnings -Wl,-z,unique -shared -o conftest.ld.so conftest.ld.c'
> + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> + (eval $ac_try) 2>&5
> + ac_status=$?
> + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> + test $ac_status = 0; }; }
> +then
> + libc_cv_ld_zunique=yes
> +fi;
> +ld_zunique=$libc_cv_ld_zunique
> +rm -f conftest*
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_ld_zunique" >&5
> +$as_echo "$libc_cv_ld_zunique" >&6; }
> +
> +
> # The linker's default -shared behavior is good enough if it
> # does these things that our custom linker scripts ensure that
> -# all allocated NOTE sections come first.
> +# all allocated NOTE sections come first AND it understands
> +# -z unique should result in DT_GNU_FLAGS_1/DF_GNU_1_UNIQUE
> +# sections in its output.
> if test "$use_default_link" = default; then
> { $as_echo "$as_me:${as_lineno-$LINENO}: checking for sufficient default -shared layout" >&5
> $as_echo_n "checking for sufficient default -shared layout... " >&6; }
> @@ -6119,10 +6149,16 @@ EOF
> esac
> shift 2
> done
> - case "$libc_seen_a$libc_seen_b" in
> - yesyes)
> + case "$libc_seen_a$libc_seen_b$libc_cv_ld_zunique" in
> + yesyesyes)
> libc_cv_use_default_link=yes
> ;;
> + yesyesno)
> + echo >&5 "\
> +shared layout from:
> +$ac_try
> +is OK but -Wl,-z,unique is unsupported so linker scripts are required"
> + ;;
> *)
> echo >&5 "\
> $libc_seen_a$libc_seen_b from:
Ok.
> diff --git a/configure.ac b/configure.ac
> index 34ecbba540..9369bcbebe 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1372,9 +1372,28 @@ fi
> rm -f conftest*])
> AC_SUBST(libc_cv_hashstyle)
>
> +AC_CACHE_CHECK([for linker DT_GNU_FLAGS_1/ DF_GNU_1_UNIQUE support],
Maybe a space before ' /'.
> + libc_cv_ld_zunique, [dnl
> +cat > conftest.ld.c <<\EOF
> +int x (void) { return 0; }
> +EOF
> +
> +libc_cv_ld_zunique=no;
> +
> +if AC_TRY_COMMAND([dnl
> +${CC-cc} -Wl,--fatal-warnings -Wl,-z,unique -shared -o conftest.ld.so conftest.ld.c])
> +then
> + libc_cv_ld_zunique=yes
> +fi;
> +ld_zunique=$libc_cv_ld_zunique
> +rm -f conftest*])
> +AC_SUBST(ld_zunique)
> +
I think we can simplify it with:
old_LDFLAGS="$LDFLAGS"
LDFLAGS="-Wl,--fatal-warnings -Wl,-z,unique -shared"
AC_CACHE_CHECK([for linker DT_GNU_FLAGS_1 / DF_GNU_1_UNIQUE support],
AC_LINK_IFELSE([AC_LANG_SOURCE([void foo (void) { }])],
libc_cv_ld_zunique=yes,
libc_cv_ld_zunique=no))
LDFLAGS="$old_LDFLAGS"
AC_SUBST(ld_zunique)
> # The linker's default -shared behavior is good enough if it
> # does these things that our custom linker scripts ensure that
> -# all allocated NOTE sections come first.
> +# all allocated NOTE sections come first AND it understands
> +# -z unique should result in DT_GNU_FLAGS_1/DF_GNU_1_UNIQUE
> +# sections in its output.
> if test "$use_default_link" = default; then
> AC_CACHE_CHECK([for sufficient default -shared layout],
> libc_cv_use_default_link, [dnl
Ok.
> @@ -1410,10 +1429,16 @@ EOF
> esac
> shift 2
> done
> - case "$libc_seen_a$libc_seen_b" in
> - yesyes)
> + case "$libc_seen_a$libc_seen_b$libc_cv_ld_zunique" in
> + yesyesyes)
> libc_cv_use_default_link=yes
> ;;
> + yesyesno)
> + echo >&AS_MESSAGE_LOG_FD "\
> +shared layout from:
> +$ac_try
> +is OK but -Wl,-z,unique is unsupported so linker scripts are required"
> + ;;
> *)
> echo >&AS_MESSAGE_LOG_FD "\
> $libc_seen_a$libc_seen_b from:
Ok.
> diff --git a/elf/Makefile b/elf/Makefile
> index 698a6ab985..9734030c23 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -103,6 +103,9 @@ endif
>
> ifeq (yes,$(build-shared))
> extra-objs = $(all-rtld-routines:%=%.os) sofini.os interp.os
> +ifneq (yes,$(ld-zunique))
> +extra-objs += dynamic-notes.os
> +endif
> generated += librtld.os dl-allobjs.os ld.so ldd
> install-others = $(inst_rtlddir)/$(rtld-installed-name)
> install-bin-script = ldd
Ok.
> diff --git a/elf/dynamic-notes.c b/elf/dynamic-notes.c
> new file mode 100644
> index 0000000000..d0b487e970
> --- /dev/null
> +++ b/elf/dynamic-notes.c
> @@ -0,0 +1,4 @@
> +#include <link.h>
> +
> +const ElfW(Dyn) __dynamic_note __attribute__ ((section (".dynamic"))) =
> + { .d_tag = DT_GNU_FLAGS_1, .d_un.d_val = DF_GNU_1_UNIQUE };
Ok.
> diff --git a/extra-lib.mk b/extra-lib.mk
> index 72f8d2e1df..9051958ec0 100644
> --- a/extra-lib.mk
> +++ b/extra-lib.mk
> @@ -101,6 +101,9 @@ $(objpfx)$(lib).so: $(firstword $($(lib)-map) \
> $(addprefix $(common-objpfx), \
> $(filter $(lib).map, \
> $(version-maps))))
> +ifneq ($(ld-zunique),yes)
> +$(objpfx)$(lib).so: $(common-objpfx)/elf/dynamic-notes.os
> +endif
> endif
>
> endif
Ok.
> diff --git a/htl/Makefile b/htl/Makefile
> index cf9d12fc12..c2a25dcd79 100644
> --- a/htl/Makefile
> +++ b/htl/Makefile
> @@ -203,6 +203,9 @@ libc-link.so = $(common-objpfx)libc.so
>
> extra-B-pthread.so = -B$(common-objpfx)htl/
> LDFLAGS-pthread.so = -Wl,--enable-new-dtags,-z,nodelete,-z,initfirst
> +ifeq ($(ld-zunique),yes)
> +LDFLAGS-pthread.so += -Wl,-z,unique
> +endif
>
> include ../Rules
>
Ok.
> diff --git a/iconvdata/Makefile b/iconvdata/Makefile
> index bb3f621b49..4170005322 100644
> --- a/iconvdata/Makefile
> +++ b/iconvdata/Makefile
> @@ -67,6 +67,9 @@ modules := ISO8859-1 ISO8859-2 ISO8859-3 ISO8859-4 ISO8859-5 \
> ifeq ($(bind-now),yes)
> LDFLAGS.so += -Wl,-z,now
> endif
> +ifeq ($(ld-zunique),yes)
> +LDFLAGS.so += -Wl,-z,unique
> +endif
>
> modules.so := $(addsuffix .so, $(modules))
>
Ok.
> diff --git a/iconvdata/extra-module.mk b/iconvdata/extra-module.mk
> index ecaf507624..92c3ab4d7b 100644
> --- a/iconvdata/extra-module.mk
> +++ b/iconvdata/extra-module.mk
> @@ -7,6 +7,10 @@ $(objpfx)$(mod).so: $(addprefix $(objpfx),$(addsuffix .os,$($(mod)-routines)))\
> $(shlib-lds) $(link-libc-deps)
> $(build-module-asneeded)
>
> +ifneq ($(ld-zunique),yes)
> +$(objpfx)$(mod).so: $(common-objpfx)/elf/dynamic-notes.os
> +endif
> +
> ifneq (,$(extra-modules-left))
> include extra-module.mk
> endif
Ok.
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 9b94bfcd31..81353b9455 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -375,7 +375,12 @@ CPPFLAGS-tst-pthread-gdb-attach-static.c := \
> # were launched with an explicit ld.so invocation.
> tst-pthread-gdb-attach-no-pie = yes
>
> -
> +LDFLAGS-pthread.so = -Wl,--enable-new-dtags,-z,nodelete,-z,initfirst
> +ifeq ($(ld-zunique),yes)
> +LDFLAGS-pthread.so += -Wl,-z,unique
> +endif
> +
I think now that we move all symbols to libpthread, there is no need to mark it
as -z,unique (as you do on a subsequent patch in this set).
> +tests += tst-cancelx7 tst-cancelx17 tst-cleanupx4
I think the extra tst-cleanupx4 is a rebase mistake here.
>
> ifeq ($(build-shared),yes)
> tests += tst-compat-forwarder tst-audit-threads
>
next prev parent reply other threads:[~2021-08-09 16:48 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-08 16:32 [RFC][PATCH v12 0/8] Implementation of RTLD_SHARED for dlmopen Vivek Das Mohapatra via Libc-alpha
2021-07-08 16:32 ` [RFC][PATCH v12 1/8] Define a new dynamic section tag - DT_GNU_FLAGS_1 (bug 22745) Vivek Das Mohapatra via Libc-alpha
2021-08-09 12:27 ` Adhemerval Zanella via Libc-alpha
2021-07-08 16:32 ` [RFC][PATCH v12 2/8] Abstract loaded-DSO search code into a helper function Vivek Das Mohapatra via Libc-alpha
2021-08-09 13:04 ` Adhemerval Zanella via Libc-alpha
2021-07-08 16:32 ` [RFC][PATCH v12 3/8] Use the new DSO finder " Vivek Das Mohapatra via Libc-alpha
2021-08-09 12:59 ` Adhemerval Zanella via Libc-alpha
2021-07-08 16:32 ` [RFC][PATCH v12 4/8] Add DT_GNU_FLAGS_1/DF_GNU_1_UNIQUE to glibc DSOs (bug 22745) Vivek Das Mohapatra via Libc-alpha
2021-08-09 16:48 ` Adhemerval Zanella via Libc-alpha [this message]
2021-08-10 13:21 ` Adhemerval Zanella via Libc-alpha
2021-08-10 13:32 ` H.J. Lu via Libc-alpha
2021-07-08 16:32 ` [RFC][PATCH v12 5/8] Implement dlmopen RTLD_SHARED flag " Vivek Das Mohapatra via Libc-alpha
2021-08-09 19:07 ` Adhemerval Zanella via Libc-alpha
2021-08-09 20:24 ` Adhemerval Zanella via Libc-alpha
2021-08-09 20:30 ` Adhemerval Zanella via Libc-alpha
2021-07-08 16:32 ` [RFC][PATCH v12 6/8] Add dlmopen / RTLD_SHARED tests Vivek Das Mohapatra via Libc-alpha
2021-08-09 20:09 ` Adhemerval Zanella via Libc-alpha
2021-07-08 16:32 ` [RFC][PATCH v12 7/8] Restore separate libc loading for the TLS/namespace storage test Vivek Das Mohapatra via Libc-alpha
2021-08-09 19:08 ` Adhemerval Zanella via Libc-alpha
2021-07-08 16:32 ` [RFC][PATCH v12 8/8] Drop DT_GNU_FLAGS_1/DF_GNU_1_UNIQUE from the libpthread DSO Vivek Das Mohapatra via Libc-alpha
2021-08-09 20:10 ` Adhemerval Zanella via Libc-alpha
2021-08-09 21:24 ` Florian Weimer via Libc-alpha
2021-07-13 13:08 ` [External] : [RFC][PATCH v12 0/8] Implementation of RTLD_SHARED for dlmopen Alfonso Alfonso Peterssen via Libc-alpha
2021-08-09 20:34 ` Adhemerval Zanella via Libc-alpha
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/libc/involved.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6209df75-2d78-b188-4041-fe77849e224c@linaro.org \
--to=libc-alpha@sourceware.org \
--cc=adhemerval.zanella@linaro.org \
--cc=vivek@collabora.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).