Libc-alpha Digest, Vol 56, Issue 243
Hnin12a 55
h42324663@gmail.com
Sat Nov 2 23:50:55 GMT 2024
On Thu, Oct 31, 2024, 11:33 PM <libc-alpha-request@sourceware.org> wrote:
> Send Libc-alpha mailing list submissions to
> libc-alpha@sourceware.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
> https://sourceware.org/mailman/listinfo/libc-alpha
> or, via email, send a message with subject or body 'help' to
> libc-alpha-request@sourceware.org
>
> You can reach the person managing the list at
> libc-alpha-owner@sourceware.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Libc-alpha digest..."
> Today's Topics:
>
> 1. [PATCH v2 2/4] elf: Do not define consider_profiling,
> consider_symbind as macros (Florian Weimer)
> 2. [PATCH v2 3/4] elf: Introduce _dl_relocate_object_no_relro
> (Florian Weimer)
> 3. [PATCH v2 4/4] elf: Switch to main malloc after final ld.so
> self-relocation (Florian Weimer)
> 4. [PATCH v3] malloc: send freed small chunks to smallbin (k4lizen)
>
>
>
> ---------- Forwarded message ----------
> From: Florian Weimer <fweimer@redhat.com>
> To: libc-alpha@sourceware.org
> Cc:
> Bcc:
> Date: Thu, 31 Oct 2024 17:09:26 +0100
> Subject: [PATCH v2 2/4] elf: Do not define consider_profiling,
> consider_symbind as macros
> This avoids surprises when refactoring the code if these identifiers
> are re-used later in the file.
> ---
> elf/dl-reloc.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
> index 4bf7aec88b..b2c1627ceb 100644
> --- a/elf/dl-reloc.c
> +++ b/elf/dl-reloc.c
> @@ -220,8 +220,8 @@ _dl_relocate_object (struct link_map *l, struct
> r_scope_elem *scope[],
> int lazy = reloc_mode & RTLD_LAZY;
> int skip_ifunc = reloc_mode & __RTLD_NOIFUNC;
>
> -#ifdef SHARED
> bool consider_symbind = false;
> +#ifdef SHARED
> /* If we are auditing, install the same handlers we need for
> profiling. */
> if ((reloc_mode & __RTLD_AUDIT) == 0)
> {
> @@ -240,9 +240,7 @@ _dl_relocate_object (struct link_map *l, struct
> r_scope_elem *scope[],
> }
> #elif defined PROF
> /* Never use dynamic linker profiling for gprof profiling code. */
> -# define consider_profiling 0
> -#else
> -# define consider_symbind 0
> + consider_profiling = 0;
> #endif
>
> /* If DT_BIND_NOW is set relocate all references in this object. We
> @@ -300,7 +298,6 @@ _dl_relocate_object (struct link_map *l, struct
> r_scope_elem *scope[],
>
> ELF_DYNAMIC_RELOCATE (l, scope, lazy, consider_profiling, skip_ifunc);
>
> -#ifndef PROF
> if ((consider_profiling || consider_symbind)
> && l->l_info[DT_PLTRELSZ] != NULL)
> {
> @@ -321,7 +318,6 @@ _dl_relocate_object (struct link_map *l, struct
> r_scope_elem *scope[],
> _dl_fatal_printf (errstring, RTLD_PROGNAME, l->l_name);
> }
> }
> -#endif
> }
>
> /* Mark the object so we know this work has been done. */
> --
> 2.47.0
>
>
>
>
>
>
> ---------- Forwarded message ----------
> From: Florian Weimer <fweimer@redhat.com>
> To: libc-alpha@sourceware.org
> Cc:
> Bcc:
> Date: Thu, 31 Oct 2024 17:09:34 +0100
> Subject: [PATCH v2 3/4] elf: Introduce _dl_relocate_object_no_relro
> And make _dl_protect_relro apply RELRO conditionally.
>
> Reviewed-by: DJ Delorie <dj@redhat.com>
> ---
> elf/dl-reloc.c | 24 ++++++++++++++----------
> sysdeps/generic/ldsodefs.h | 7 +++++++
> 2 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
> index b2c1627ceb..76d14830dd 100644
> --- a/elf/dl-reloc.c
> +++ b/elf/dl-reloc.c
> @@ -202,12 +202,9 @@ resolve_map (lookup_t l, struct r_scope_elem
> *scope[], const ElfW(Sym) **ref,
> #include "dynamic-link.h"
>
> void
> -_dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
> - int reloc_mode, int consider_profiling)
> +_dl_relocate_object_no_relro (struct link_map *l, struct r_scope_elem
> *scope[],
> + int reloc_mode, int consider_profiling)
> {
> - if (l->l_relocated)
> - return;
> -
> struct textrels
> {
> caddr_t start;
> @@ -338,17 +335,24 @@ _dl_relocate_object (struct link_map *l, struct
> r_scope_elem *scope[],
>
> textrels = textrels->next;
> }
> -
> - /* In case we can protect the data now that the relocations are
> - done, do it. */
> - if (l->l_relro_size != 0)
> - _dl_protect_relro (l);
> }
>
> +void
> +_dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
> + int reloc_mode, int consider_profiling)
> +{
> + if (l->l_relocated)
> + return;
> + _dl_relocate_object_no_relro (l, scope, reloc_mode, consider_profiling);
> + _dl_protect_relro (l);
> +}
>
> void
> _dl_protect_relro (struct link_map *l)
> {
> + if (l->l_relro_size == 0)
> + return;
> +
> ElfW(Addr) start = ALIGN_DOWN((l->l_addr
> + l->l_relro_addr),
> GLRO(dl_pagesize));
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 259ce2e7d6..91447a5e77 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -1014,6 +1014,13 @@ extern void _dl_relocate_object (struct link_map
> *map,
> int reloc_mode, int consider_profiling)
> attribute_hidden;
>
> +/* Perform relocation, but do not apply RELRO. Does not check
> + L->relocated. Otherwise the same as _dl_relocate_object. */
> +void _dl_relocate_object_no_relro (struct link_map *map,
> + struct r_scope_elem *scope[],
> + int reloc_mode, int consider_profiling)
> + attribute_hidden;
> +
> /* Protect PT_GNU_RELRO area. */
> extern void _dl_protect_relro (struct link_map *map) attribute_hidden;
>
> --
> 2.47.0
>
>
>
>
>
>
> ---------- Forwarded message ----------
> From: Florian Weimer <fweimer@redhat.com>
> To: libc-alpha@sourceware.org
> Cc:
> Bcc:
> Date: Thu, 31 Oct 2024 17:09:45 +0100
> Subject: [PATCH v2 4/4] elf: Switch to main malloc after final ld.so
> self-relocation
> Before commit ee1ada1bdb8074de6e1bdc956ab19aef7b6a7872
> ("elf: Rework exception handling in the dynamic loader
> [BZ #25486]"), the previous order called the main calloc
> to allocate a shadow GOT/PLT array for auditing support.
> This happened before libc.so.6 ELF constructors were run, so
> a user malloc could run without libc.so.6 having been
> initialized fully. One observable effect was that
> environ was NULL at this point.
>
> It does not seem to be possible at present to trigger such
> an allocation, but it seems more robust to delay switching
> to main malloc after ld.so self-relocation is complete.
> The elf/tst-rtld-no-malloc-audit test case fails with a
> 2.34-era glibc that does not have this fix.
> ---
> elf/Makefile | 9 ++++
> elf/dl-support.c | 3 +-
> elf/rtld.c | 25 +++++------
> elf/tst-rtld-no-malloc-audit.c | 1 +
> elf/tst-rtld-no-malloc-preload.c | 1 +
> elf/tst-rtld-no-malloc.c | 75 ++++++++++++++++++++++++++++++++
> 6 files changed, 98 insertions(+), 16 deletions(-)
> create mode 100644 elf/tst-rtld-no-malloc-audit.c
> create mode 100644 elf/tst-rtld-no-malloc-preload.c
> create mode 100644 elf/tst-rtld-no-malloc.c
>
> diff --git a/elf/Makefile b/elf/Makefile
> index fda796f6d5..3a1cb72955 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -453,6 +453,9 @@ tests += \
> tst-recursive-tls \
> tst-relsort1 \
> tst-ro-dynamic \
> + tst-rtld-no-malloc \
> + tst-rtld-no-malloc-audit \
> + tst-rtld-no-malloc-preload \
> tst-rtld-run-static \
> tst-single_threaded \
> tst-single_threaded-pthread \
> @@ -3160,3 +3163,9 @@ tst-dlopen-tlsreinit4-ENV =
> LD_AUDIT=$(objpfx)tst-auditmod1.so
> tst-dlopen-auditdup-ENV =
> LD_AUDIT=$(objpfx)tst-dlopen-auditdup-auditmod.so
> $(objpfx)tst-dlopen-auditdup.out: \
> $(objpfx)tst-dlopen-auditdupmod.so
> $(objpfx)tst-dlopen-auditdup-auditmod.so
> +
> +# Reuse an audit module which provides ample debug logging.
> +tst-rtld-no-malloc-audit-ENV = LD_AUDIT=$(objpfx)tst-auditmod1.so
> +
> +# Any shared object should do.
> +tst-rtld-no-malloc-preload-ENV = LD_PRELOAD=$(objpfx)tst-auditmod1.so
> diff --git a/elf/dl-support.c b/elf/dl-support.c
> index 451932dd03..ee590edf93 100644
> --- a/elf/dl-support.c
> +++ b/elf/dl-support.c
> @@ -338,8 +338,7 @@ _dl_non_dynamic_init (void)
> call_function_static_weak (_dl_find_object_init);
>
> /* Setup relro on the binary itself. */
> - if (_dl_main_map.l_relro_size != 0)
> - _dl_protect_relro (&_dl_main_map);
> + _dl_protect_relro (&_dl_main_map);
> }
>
> #ifdef DL_SYSINFO_IMPLEMENTATION
> diff --git a/elf/rtld.c b/elf/rtld.c
> index dcd0f4cdc6..b8cc3f605f 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -2321,30 +2321,27 @@ dl_main (const ElfW(Phdr) *phdr,
> /* Make sure no new search directories have been added. */
> assert (GLRO(dl_init_all_dirs) == GL(dl_all_dirs));
>
> - /* Re-relocate ourselves with user-controlled symbol definitions.
> -
> - We must do this after TLS initialization in case after this
> - re-relocation, we might call a user-supplied function
> - (e.g. calloc from _dl_relocate_object) that uses TLS data. */
> -
> /* Set up the object lookup structures. */
> _dl_find_object_init ();
>
> - /* The malloc implementation has been relocated, so resolving
> - its symbols (and potentially calling IFUNC resolvers) is safe
> - at this point. */
> - __rtld_malloc_init_real (main_map);
> -
> /* Likewise for the locking implementation. */
> __rtld_mutex_init ();
>
> + /* Re-relocate ourselves with user-controlled symbol definitions. */
> +
> {
> RTLD_TIMING_VAR (start);
> rtld_timer_start (&start);
>
> - /* Mark the link map as not yet relocated again. */
> - GL(dl_rtld_map).l_relocated = 0;
> - _dl_relocate_object (&GL(dl_rtld_map), main_map->l_scope, 0, 0);
> + _dl_relocate_object_no_relro (&GL(dl_rtld_map), main_map->l_scope, 0,
> 0);
> +
> + /* The malloc implementation has been relocated, so resolving
> + its symbols (and potentially calling IFUNC resolvers) is safe
> + at this point. */
> + __rtld_malloc_init_real (main_map);
> +
> + if (GL(dl_rtld_map).l_relro_size != 0)
> + _dl_protect_relro (&GL(dl_rtld_map));
>
> rtld_timer_accum (&relocate_time, start);
> }
> diff --git a/elf/tst-rtld-no-malloc-audit.c
> b/elf/tst-rtld-no-malloc-audit.c
> new file mode 100644
> index 0000000000..a028377ad1
> --- /dev/null
> +++ b/elf/tst-rtld-no-malloc-audit.c
> @@ -0,0 +1 @@
> +#include "tst-rtld-no-malloc.c"
> diff --git a/elf/tst-rtld-no-malloc-preload.c
> b/elf/tst-rtld-no-malloc-preload.c
> new file mode 100644
> index 0000000000..a028377ad1
> --- /dev/null
> +++ b/elf/tst-rtld-no-malloc-preload.c
> @@ -0,0 +1 @@
> +#include "tst-rtld-no-malloc.c"
> diff --git a/elf/tst-rtld-no-malloc.c b/elf/tst-rtld-no-malloc.c
> new file mode 100644
> index 0000000000..a78f1ba727
> --- /dev/null
> +++ b/elf/tst-rtld-no-malloc.c
> @@ -0,0 +1,75 @@
> +/* Test that program loading does not call malloc.
> + Copyright (C) 2024 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +
> +#include <string.h>
> +#include <unistd.h>
> +
> +static void
> +print (const char *s)
> +{
> + const char *end = s + strlen (s);
> + while (s < end)
> + {
> + ssize_t ret = write (STDOUT_FILENO, s, end - s);
> + if (ret <= 0)
> + _exit (2);
> + s += ret;
> + }
> +}
> +
> +static void __attribute__ ((noreturn))
> +unexpected_call (const char *function)
> +{
> + print ("error: unexpected call to ");
> + print (function);
> + print ("\n");
> + _exit (1);
> +}
> +
> +/* These are the malloc functions implement in elf/dl-minimal.c. */
> +
> +void
> +free (void *ignored)
> +{
> + unexpected_call ("free");
> +}
> +
> +void *
> +calloc (size_t ignored1, size_t ignored2)
> +{
> + unexpected_call ("calloc");
> +}
> +
> +void *
> +malloc (size_t ignored)
> +{
> + unexpected_call ("malloc");
> +}
> +
> +void *
> +realloc (void *ignored1, size_t ignored2)
> +{
> + unexpected_call ("realloc");
> +}
> +
> +int
> +main (void)
> +{
> + /* Do not use the test wrapper, to avoid spurious malloc calls from
> it. */
> +}
> --
> 2.47.0
>
>
>
>
>
> ---------- Forwarded message ----------
> From: k4lizen <k4lizen@proton.me>
> To: "libc-alpha@sourceware.org" <libc-alpha@sourceware.org>, Wilco
> Dijkstra <Wilco.Dijkstra@arm.com>, DJ Delorie <dj@redhat.com>, "
> fweimer@redhat.com" <fweimer@redhat.com>
> Cc:
> Bcc:
> Date: Thu, 31 Oct 2024 17:00:08 +0000
> Subject: [PATCH v3] malloc: send freed small chunks to smallbin
> > In theory, it should reduce RSS by giving more time to coalesce
> > chunks and thus possibly either reuse those or shrink the top chunk.
>
> As Florian and Wilco already mentioned, the gist is:
> > Small, large and unsorted bins get immediately coalesced. So placing
> something
> > in a smallbin rather than unsorted has no effect on coalescing and thus
> RSS.
>
> The only possible effect I could think of is the fact that the patch
> affects
> the order in which small chunks are (re)allocated. Previously, recently
> freed chunks had a chance to get realloacted (due to the unsorted)
> quickly, while
> with the patch they would always follow the LIFO that smallbin enforces.
>
> So possibly heuristics like "chunks that were freed close time-wise should
> be
> close memory-location wise" could come into play. Following that logic it
> makes
> sense to me that LIFO is benefitial, and if it was in reality bad for
> fragmentation, that would denote a bigger problem in the current
> implementation
> of malloc.
>
> In any case if there was any effect, positive or negative, it would be
> negligable.
>
> There are benchmarks for RSS in the attachment I provided in the initial
> benchmark
> email (https://sourceware.org/pipermail/libc-alpha/2024-July/158130.html)
> though
> they aren't plotted/visualized properly, and are not of "real world
> applications".
>
> There's this project for benchmarking malloc implementations:
> https://github.com/daanx/mimalloc-bench . It's pretty cool and has neat
> tests
> like running redis, fraction factorization etc. You can read about them in
> the
> "Current benchmarks" section of the README if you want.
>
> The results are in the attachment. My interpretation is that RSS isn't
> affected.
> Time is also bearly affected (both positively and negatively) except in
> the gs
> test case where the patch sees a significant positive impact (yay!).
> That's the
> "gs: have ghostscript process the entire Intel Software Developer’s Manual
> PDF, which is around 5000 pages." test.
>
> I wanted to reproduce Wilco's benchmark but it seems xalancbmk is
> proprietary :(
>
> Wilco:
> > My only outstanding issue is code licensing.
>
> What? Could you elaborate?
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20241103/0d5cf442/attachment-0001.htm>
More information about the Libc-alpha
mailing list