From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1031.google.com (mail-pj1-x1031.google.com [IPv6:2607:f8b0:4864:20::1031]) by sourceware.org (Postfix) with ESMTPS id 28ACD3861031 for ; Mon, 9 Aug 2021 16:48:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 28ACD3861031 Received: by mail-pj1-x1031.google.com with SMTP id fa24-20020a17090af0d8b0290178bfa69d97so925710pjb.0 for ; Mon, 09 Aug 2021 09:48:10 -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=iWR2rytjefqTbeebm2iebqy2w7mYKNI/0NDPuo+4d8c=; b=eFNMheo//y4EM1Ay9vBmi3MoSURYftDuz/afLzt3DNJw6u0o+QRuYL43cTEMeq203v D3xM2dSqlbhLT1JfIMijfHXIU3ynlMlugERRp5rmhIlNF9lRAyKWLqgW4cfgxRPIP9m5 WEJ00fBP7mGkn5yRx6WsZppj6nO5qGixTjnZ/4hUIS7c6EP7Uvpi6LojSXTXZ7Ah+NWq c7pUXbTZgiAsyRI33sLNnLBAZHc2QcELFIuBaWl05aqFhJqx4sWyl2jl/DPlhnLXqr9h IdXN+hEsBBzQoErScLuu+uxz0EiSYByTMTZrd72h165wOQDW/h/l3KZQ5hkM6oE7JjyP tnww== X-Gm-Message-State: AOAM531vLwTDeefgJ2gtKhjyGTQ0XWOYxrK6eGvOyX59RdxC6WtpRh1w Kvtef8jpVi1fGZMVDrK9N9NpDg== X-Google-Smtp-Source: ABdhPJy553QqBmuiLRc8p/j5ejGkIUiX4tquBmD+BI1COQqCUq5Bt2jXJ2Zj16dD4gZ0LRrxsMMMkw== X-Received: by 2002:a17:902:6b84:b029:ee:f966:1911 with SMTP id p4-20020a1709026b84b02900eef9661911mr1952356plk.69.1628527689213; Mon, 09 Aug 2021 09:48:09 -0700 (PDT) Received: from ?IPv6:2804:431:c7cb:9dce:a96c:8a7f:2ffc:f373? ([2804:431:c7cb:9dce:a96c:8a7f:2ffc:f373]) by smtp.gmail.com with ESMTPSA id o9sm22314487pfh.217.2021.08.09.09.48.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 09 Aug 2021 09:48:08 -0700 (PDT) Subject: Re: [RFC][PATCH v12 4/8] Add DT_GNU_FLAGS_1/DF_GNU_1_UNIQUE to glibc DSOs (bug 22745) To: libc-alpha@sourceware.org, Vivek Das Mohapatra References: <20210708163255.812-1-vivek@collabora.com> <20210708163255.812-5-vivek@collabora.com> From: Adhemerval Zanella Message-ID: <6209df75-2d78-b188-4041-fe77849e224c@linaro.org> Date: Mon, 9 Aug 2021 13:48:06 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210708163255.812-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.1 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.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Mon, 09 Aug 2021 16:48:21 -0000 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 > --- > 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 > + > +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 >