From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf30.google.com (mail-qv1-xf30.google.com [IPv6:2607:f8b0:4864:20::f30]) by sourceware.org (Postfix) with ESMTPS id 3C7C0398B17F for ; Fri, 28 May 2021 14:53:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3C7C0398B17F Received: by mail-qv1-xf30.google.com with SMTP id 5so2026943qvk.0 for ; Fri, 28 May 2021 07:53:13 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=gs+t4CnZdZYu19o+kbaDXo7R8qru+YJ5Td5MCq7ts30=; b=Tt/Hv8ur6UZrZ4G61I8EokxxXsYRkN0a1kNcuufojNQR8ATM1AoCosSYSFpkQWNm07 cam6WqyT24cFeiS2h7F10cTsWQywlJKjVwAnjgQyiMxdTQht08uhXMzqeMtin4erkIAv tULji+xEU4xgdAA0cozFAhrHe855jkcp32LhtiDP++9NFwNUtTeiZvS0S4ZDuLiDppYj Hn/32yf83YPZuroIW4GGlFX9uzqFxxkMDdiE2/HUhgSBXW26g5YzYKzIrsKNzPRvq7C3 /v/RRlDZc/dFw6yYivmS2obuJm9umdEomlXf6iZWh0oetvjufeD5C3Cp+XtJ/S8yb4wA cL8g== X-Gm-Message-State: AOAM530u4k/uAoKUHOgxOhkceuwuWjZAwxum24G2QmJ80Y4l3jaUA4+j fbmJ+p+kzPiGnfBqDHJf0D3TkSjr3ViqcQ== X-Google-Smtp-Source: ABdhPJx2GVqQY8CmxbMhNTDn8Gj0lwTjvYxvWllkNuMcWlUwq9l3bMphuLPJB+lcZ3c0/XEoMw7p/w== X-Received: by 2002:ad4:5f07:: with SMTP id fo7mr4255954qvb.54.1622213592560; Fri, 28 May 2021 07:53:12 -0700 (PDT) Received: from [192.168.1.4] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id f19sm3772841qkg.70.2021.05.28.07.53.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 28 May 2021 07:53:12 -0700 (PDT) Subject: Re: [RFC][PATCH v10 4/7] Add DT_GNU_FLAGS_1/DF_GNU_1_UNIQUE to glibc DSOs (bug 22745) To: Vivek Das Mohapatra , libc-alpha@sourceware.org References: <20210322154111.24798-1-vivek@collabora.com> <20210322154111.24798-5-vivek@collabora.com> From: Adhemerval Zanella Message-ID: Date: Fri, 28 May 2021 11:53:10 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210322154111.24798-5-vivek@collabora.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 May 2021 14:53:17 -0000 On 22/03/2021 12:41, 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). Some comments below. > --- > Makeconfig | 3 +++ > Makerules | 19 +++++++++++++- > config.make.in | 1 + > configure | 54 ++++++++++++++++++++++++++++++++++++--- > configure.ac | 32 +++++++++++++++++++++-- > dynamic-notes.c | 4 +++ > elf/Makefile | 3 +++ > extra-lib.mk | 3 +++ > htl/Makefile | 3 +++ > iconvdata/Makefile | 3 +++ > iconvdata/extra-module.mk | 4 +++ > nptl/Makefile | 3 +++ > 12 files changed, 126 insertions(+), 6 deletions(-) > create mode 100644 dynamic-notes.c > > diff --git a/Makeconfig b/Makeconfig > index 01f8638c2e..7e1116163f 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 ca9885436e..d54767f3a6 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,17 @@ 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 > +%/dynamic-notes.os: dynamic-notes.c > + > +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. > @@ -706,6 +718,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)dynamic-notes.os > +$(common-objpfx)linkobj/libc.so: $(common-objpfx)dynamic-notes.os > +endif > + > ifeq ($(build-shared),yes) > $(common-objpfx)libc.so: $(common-objpfx)libc.map > endif > diff --git a/config.make.in b/config.make.in > index 7f47f0caa4..c2d818681b 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@ > diff --git a/configure b/configure > index 37cef37413..1040130d6c 100755 > --- a/configure > +++ b/configure > @@ -685,6 +685,7 @@ hardcoded_path_in_tests > enable_timezone_tools > extra_nonshared_cflags > use_default_link > +ld_zunique > sysheaders > ac_ct_CXX > CXXFLAGS > @@ -731,6 +732,7 @@ infodir > docdir > oldincludedir > includedir > +runstatedir > localstatedir > sharedstatedir > sysconfdir Although it being autogenerated by autoconf, the runstatedir should be added in a different patch. So I think it should be manually removed for now. > @@ -843,6 +845,7 @@ datadir='${datarootdir}' > sysconfdir='${prefix}/etc' > sharedstatedir='${prefix}/com' > localstatedir='${prefix}/var' > +runstatedir='${localstatedir}/run' > includedir='${prefix}/include' > oldincludedir='/usr/include' > docdir='${datarootdir}/doc/${PACKAGE_TARNAME}' > @@ -1095,6 +1098,15 @@ do > | -silent | --silent | --silen | --sile | --sil) > silent=yes ;; > > + -runstatedir | --runstatedir | --runstatedi | --runstated \ > + | --runstate | --runstat | --runsta | --runst | --runs \ > + | --run | --ru | --r) > + ac_prev=runstatedir ;; > + -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \ > + | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \ > + | --run=* | --ru=* | --r=*) > + runstatedir=$ac_optarg ;; > + > -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb) > ac_prev=sbindir ;; > -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \ Ditto. > @@ -1232,7 +1244,7 @@ fi > for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \ > datadir sysconfdir sharedstatedir localstatedir includedir \ > oldincludedir docdir infodir htmldir dvidir pdfdir psdir \ > - libdir localedir mandir > + libdir localedir mandir runstatedir > do > eval ac_val=\$$ac_var > # Remove trailing slashes. > @@ -1385,6 +1397,7 @@ Fine tuning of the installation directories: > --sysconfdir=DIR read-only single-machine data [PREFIX/etc] > --sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com] > --localstatedir=DIR modifiable single-machine data [PREFIX/var] > + --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run] > --libdir=DIR object code libraries [EPREFIX/lib] > --includedir=DIR C header files [PREFIX/include] > --oldincludedir=DIR C header files for non-gcc [/usr/include] > @@ -3331,6 +3344,7 @@ fi > > > > + > # Check whether --with-default-link was given. > if test "${with_default_link+set}" = set; then : > withval=$with_default_link; use_default_link=$withval > @@ -5976,6 +5990,34 @@ 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 > +else > + echo >&5 "\ > +${CC-cc} does not support -Wl,-z,unique from: > +$ac_try" > +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. > @@ -6021,10 +6063,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 anyway" > + ;; > *) > echo >&5 "\ > $libc_seen_a$libc_seen_b from: > diff --git a/configure.ac b/configure.ac > index 16b15b6f90..178874aa12 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -148,6 +148,7 @@ AC_ARG_WITH([headers], > [sysheaders='']) > AC_SUBST(sysheaders) > > +AC_SUBST(ld_zunique) Move it to where it is actually set below. > AC_SUBST(use_default_link) > AC_ARG_WITH([default-link], > AS_HELP_STRING([--with-default-link], > @@ -1352,6 +1353,27 @@ fi > rm -f conftest*]) > AC_SUBST(libc_cv_hashstyle) > > +AC_CACHE_CHECK([for linker DT_GNU_FLAGS_1/ DF_GNU_1_UNIQUE support], > + libc_cv_ld_zunique, [dnl > +cat > conftest.ld.c <<\EOF > +int x (void) { return 0; } > +EOF > +libc_cv_ld_zunique=no; Move it to the else below, as other tests do. > +dnl FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME > +dnl THIS IS DELIBERATELY WRONG SO WE CAN TEST BUILDING WITH A TOO-OLD LINKER > +dnl FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME FIXME I think this is leftover from development. > +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 > +else > + echo >&AS_MESSAGE_LOG_FD "\ > +${CC-cc} does not support -Wl,-z,unique from: > +$ac_try" > +fi; > +ld_zunique=$libc_cv_ld_zunique > +rm -f conftest*]) > + > # 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. > @@ -1390,10 +1412,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 anyway" > + ;; > *) > echo >&AS_MESSAGE_LOG_FD "\ > $libc_seen_a$libc_seen_b from: I am not sure this message here add much extra diagnosit . > diff --git a/dynamic-notes.c b/dynamic-notes.c > new file mode 100644 > index 0000000000..d0b487e970 > --- /dev/null > +++ b/dynamic-notes.c > @@ -0,0 +1,4 @@ > +#include > + > +const ElfW(Dyn) __dynamic_note __attribute__ ((section (".dynamic"))) = > + { .d_tag = DT_GNU_FLAGS_1, .d_un.d_val = DF_GNU_1_UNIQUE }; I think we should put the note on the elf directory, even though it is used on mutiples modules. > diff --git a/elf/Makefile b/elf/Makefile > index 3b8e13e066..22f2a87fcb 100644 > --- a/elf/Makefile > +++ b/elf/Makefile > @@ -102,6 +102,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, although I think this implementation should be moved to elf/ > diff --git a/extra-lib.mk b/extra-lib.mk > index 72f8d2e1df..4e4a447d5e 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)dynamic-notes.os > +endif > endif > > endif Ditto. > diff --git a/htl/Makefile b/htl/Makefile > index 895a6f777c..65ddd202a1 100644 > --- a/htl/Makefile > +++ b/htl/Makefile > @@ -205,6 +205,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 55c527a5f7..2fe2404ade 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, it does make sense to make the iconvdata unique. > diff --git a/iconvdata/extra-module.mk b/iconvdata/extra-module.mk > index ecaf507624..4d81358f19 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)dynamic-notes.os > +endif > + > ifneq (,$(extra-modules-left)) > include extra-module.mk > endif Ok. > diff --git a/nptl/Makefile b/nptl/Makefile > index b1948cc47a..dedfe389c0 100644 > --- a/nptl/Makefile > +++ b/nptl/Makefile > @@ -396,6 +396,9 @@ tests-printers-libs := $(static-thread-library) > endif > > LDFLAGS-pthread.so = -Wl,--enable-new-dtags,-z,nodelete,-z,initfirst > +ifeq ($(ld-zunique),yes) > +LDFLAGS-pthread.so += -Wl,-z,unique > +endif > > tests += tst-cancelx7 tst-cancelx17 tst-cleanupx4 > > With the libpthread symbol move I think there is no need to add 'unique' on libpthread.so anylonger.