[PATCH v3] Set the retain attribute on _elf_set_element if CC supports [BZ #27492]
Fāng-ruì Sòng
maskray@google.com
Wed Mar 31 04:34:13 GMT 2021
On 2021-03-30, H.J. Lu wrote:
>On Tue, Mar 30, 2021 at 12:17 PM Fāng-ruì Sòng <maskray@google.com> wrote:
>>
>> On Tue, Mar 30, 2021 at 11:17 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>> >
>> > On Mon, Mar 29, 2021 at 4:24 PM Fangrui Song <maskray@google.com> wrote:
>> > >
>> > > So that text_set_element/data_set_element/bss_set_element defined
>> > > variables will be retained by the linker.
>> > >
>> > > Note: 'used' and 'retain' are orthogonal: 'used' makes sure the variable
>> > > will not be optimized out; 'retain' prevents section garbage collection
>> > > if the linker support SHF_GNU_RETAIN.
>> > >
>> > > GNU ld 2.37 and LLD 13 will support -z start-stop-gc which allow C
>> > > identifier name sections to be GCed even if there are live
>> > > __start_/__stop_ references.
>> > >
>> > > Without the change, there are some static linking problems, e.g.
>> > > _IO_cleanup (libio/genops.c) may be discarded by ld --gc-sections, so
>> > > stdout is not flushed on exit.
>> > >
>> > > Note: GCC may warning ‘retain’ attribute ignored while __has_attribute(retain) is 1
>> > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99587).
>> > > ---
>> > > Changes in v1 -> v2:
>> > > * Define attribute_used_retain_section
>> > > Changes in v2 -> v3:
>> > > * Use attribute_used_retain instead attribute_used_retain_section
>> > > ---
>> > > config.h.in | 3 +++
>> > > configure | 23 +++++++++++++++++++++++
>> > > configure.ac | 16 ++++++++++++++++
>> > > include/libc-symbols.h | 14 ++++++++++----
>> > > 4 files changed, 52 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/config.h.in b/config.h.in
>> > > index ca1547ae67..95af1ef229 100644
>> > > --- a/config.h.in
>> > > +++ b/config.h.in
>> > > @@ -187,6 +187,9 @@
>> > > /* Define if gcc supports attribute ifunc. */
>> > > #undef HAVE_GCC_IFUNC
>> > >
>> > > +/* Define if gcc supports attribute retain. */
>> > > +#undef HAVE_GCC_RETAIN
>> > > +
>> > > /* Define if the linker defines __ehdr_start. */
>> > > #undef HAVE_EHDR_START
>> > >
>> > > diff --git a/configure b/configure
>> > > index fcf43bf7de..6a6b8ac264 100755
>> > > --- a/configure
>> > > +++ b/configure
>> > > @@ -4105,6 +4105,29 @@ fi
>> > > $as_echo "$libc_cv_textrel_ifunc" >&6; }
>> > >
>> > >
>> > > +# Check if gcc supports attribute ifunc as it is used in libc_ifunc macro.
>> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for gcc attribute retain support" >&5
>> > > +$as_echo_n "checking for gcc attribute retain support... " >&6; }
>> > > +if ${libc_cv_gcc_retain+:} false; then :
>> > > + $as_echo_n "(cached) " >&6
>> > > +else
>> > > + cat > conftest.c <<EOF
>> > > +static int var __attribute__ ((used, retain, section ("__libc_atexit")));
>> > > +EOF
>> > > +libc_cv_gcc_retain=no
>> > > +if ${CC-cc} -Werror -c conftest.c -o /dev/null 1>&5 \
>> > > + 2>&5 ; then
>> > > + libc_cv_gcc_retain=yes
>> > > +fi
>> > > +rm -f conftest*
>> > > +fi
>> > > +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_gcc_retain" >&5
>> > > +$as_echo "$libc_cv_gcc_retain" >&6; }
>> > > +if test $libc_cv_gcc_retain = yes; then
>> > > + $as_echo "#define HAVE_GCC_RETAIN 1" >>confdefs.h
>> > > +
>> > > +fi
>> > > +
>> > > # Check if gcc warns about alias for function with incompatible types.
>> > > { $as_echo "$as_me:${as_lineno-$LINENO}: checking if compiler warns about alias for function with incompatible types" >&5
>> > > $as_echo_n "checking if compiler warns about alias for function with incompatible types... " >&6; }
>> > > diff --git a/configure.ac b/configure.ac
>> > > index fce967f2c2..031b8c3fb3 100644
>> > > --- a/configure.ac
>> > > +++ b/configure.ac
>> > > @@ -707,6 +707,22 @@ fi
>> > > rm -f conftest*])
>> > > AC_SUBST(libc_cv_textrel_ifunc)
>> > >
>> > > +# Check if gcc supports attribute ifunc as it is used in libc_ifunc macro.
>> > > +AC_CACHE_CHECK([for gcc attribute retain support],
>> > > + libc_cv_gcc_retain, [dnl
>> > > +cat > conftest.c <<EOF
>> > > +static int var __attribute__ ((used, retain, section ("__libc_atexit")));
>> > > +EOF
>> > > +libc_cv_gcc_retain=no
>> > > +if ${CC-cc} -Werror -c conftest.c -o /dev/null 1>&AS_MESSAGE_LOG_FD \
>> > > + 2>&AS_MESSAGE_LOG_FD ; then
>> > > + libc_cv_gcc_retain=yes
>> > > +fi
>> > > +rm -f conftest*])
>> > > +if test $libc_cv_gcc_retain = yes; then
>> > > + AC_DEFINE(HAVE_GCC_RETAIN)
>> > > +fi
>> > > +
>> > > # Check if gcc warns about alias for function with incompatible types.
>> > > AC_CACHE_CHECK([if compiler warns about alias for function with incompatible types],
>> > > libc_cv_gcc_incompatible_alias, [dnl
>> > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h
>> > > index 546fc26a7b..b311c90f0c 100644
>> > > --- a/include/libc-symbols.h
>> > > +++ b/include/libc-symbols.h
>> > > @@ -352,6 +352,12 @@ for linking")
>> > >
>> > > */
>> > >
>> > > +#ifdef HAVE_GCC_RETAIN
>> > > +# define attribute_used_retain __attribute__ ((__used__, __retain__))
>> > > +#else
>> > > +# define attribute_used_retain __attribute__ ((__used__))
>> > > +#endif
>> > > +
>> > > /* Symbol set support macros. */
>> > >
>> > > /* Make SYMBOL, which is in the text segment, an element of SET. */
>> > > @@ -367,12 +373,12 @@ for linking")
>> > > /* When building a shared library, make the set section writable,
>> > > because it will need to be relocated at run time anyway. */
>> > > # define _elf_set_element(set, symbol) \
>> > > - static const void *__elf_set_##set##_element_##symbol##__ \
>> > > - __attribute__ ((used, section (#set))) = &(symbol)
>> > > + static const void *__elf_set_##set##_element_##symbol##__ \
>> > > + attribute_used_retain __attribute__ ((section (#set))) = &(symbol)
>> > > #else
>> > > # define _elf_set_element(set, symbol) \
>> > > - static const void *const __elf_set_##set##_element_##symbol##__ \
>> > > - __attribute__ ((used, section (#set))) = &(symbol)
>> > > + static const void *const __elf_set_##set##_element_##symbol##__ \
>> > > + attribute_used_retain __attribute__ ((section (#set))) = &(symbol)
>> > > #endif
>> > >
>> > > /* Define SET as a symbol set. This may be required (it is in a.out) to
>> > > --
>> > > 2.31.0.291.g576ba9dcdaf-goog
>> > >
>> >
>> > Need gc-sections tests for linkers with and without -z start-stop-gc.
>> >
>> > --
>> > H.J.
>>
>> I don't know how to add such a test... It needs to check flush at ext
>> time but I cannot find a template in libio/tst-* ...
>
>You can add a special rule to check for linker output with
>--print-gc-sections.
I can come up runtime tests. ( https://github.com/MaskRay/glibc/tree/retain)
Runtime tests are direct.
(Unsure how to test --print-gc-sections with tests-static...)
diff --git a/libio/Makefile b/libio/Makefile
index 12ce41038f..5e0e7f00cd 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -195,6 +195,13 @@ ifeq (yes,$(build-shared))
tests-special += $(objpfx)tst-fopenloc-cmp.out $(objpfx)tst-fopenloc-mem.out \
$(objpfx)tst-bz24228-mem.out
endif
+
+tests += tst-cleanup-start-stop-gc tst-cleanup-nostart-stop-gc
+tests-static += tst-cleanup-start-stop-gc tst-cleanup-nostart-stop-gc
+tests-special += $(objpfx)tst-cleanup-start-stop-gc.out \
+ $(objpfx)tst-cleanup-nostart-stop-gc.out
+LDFLAGS-tst-cleanup-start-stop-gc = -Wl,--gc-sections,-z,start-stop-gc
+LDFLAGS-tst-cleanup-nostart-stop-gc = -Wl,--gc-sections,-z,nostart-stop-gc
endif
include ../Rules
@@ -224,6 +231,20 @@ $(objpfx)tst_wprintf2.out: $(gen-locales)
$(objpfx)tst-wfile-sync.out: $(gen-locales)
endif
+$(objpfx)tst-cleanup-start-stop-gc.out: $(objpfx)tst-cleanup-start-stop-gc
+ $< > $@T
+ grep hello $@T; $(evaluate-test)
+ rm -f $@T
+$(objpfx)tst-cleanup-start-stop-gc.o: tst-cleanup.c
+ $(compile.c) -o $@
+
+$(objpfx)tst-cleanup-nostart-stop-gc.out: $(objpfx)tst-cleanup-nostart-stop-gc
+ $< > $@T
+ grep hello $@T; $(evaluate-test)
+ rm -f $@T
+$(objpfx)tst-cleanup-nostart-stop-gc.o: tst-cleanup.c
+ $(compile.c) -o $@
+
$(objpfx)test-freopen.out: test-freopen.sh $(objpfx)test-freopen
$(SHELL) $< $(common-objpfx) '$(test-program-prefix)' \
$(common-objpfx)libio/; \
diff --git a/libio/tst-cleanup.c b/libio/tst-cleanup.c
new file mode 100644
index 0000000000..de8130a482
--- /dev/null
+++ b/libio/tst-cleanup.c
@@ -0,0 +1,14 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+void
+hook (void)
+{
+ puts ("hello");
+}
+
+int
+main (void)
+{
+ atexit (hook);
+}
More information about the Libc-alpha
mailing list