The branches may be better optimized since mtag_enabled is widely used.
Granule size larger than a chunk header is not supported since then we
cannot have both the chunk header and user area granule aligned. To
fix that for targets with large granule, the chunk layout has to change.
So code that attempted to handle the granule mask generally was changed.
This simplified CHUNK_AVAILABLE_SIZE and the logic in malloc_usable_size.
Reviewed-by: DJ Delorie <dj@redhat.com>
A flag check can be faster than function pointers because of how
branch prediction and speculation works and it can also remove a layer
of indirection when there is a mismatch between the malloc internal
tag_* api and __libc_mtag_* target hooks.
Memory tagging wrapper functions are moved to malloc.c from arena.c and
the logic now checks mmap_enabled. The definition of tag_new_usable is
moved after chunk related definitions.
This refactoring also allows using mtag_enabled checks instead of
USE_MTAG ifdefs when memory tagging support only changes code logic
when memory tagging is enabled at runtime. Note: an "if (false)" code
block is optimized away even at -O0 by gcc.
Reviewed-by: DJ Delorie <dj@redhat.com>
This does not change behaviour, just removes one layer of indirection
in the internal memory tagging logic.
Use tag_ and mtag_ prefixes instead of __tag_ and __mtag_ since these
are all symbols with internal linkage, private to malloc.c, so there
is no user namespace pollution issue.
Reviewed-by: DJ Delorie <dj@redhat.com>
The chunk cannot be a dumped one here. The only non-obvious cases
are free and realloc which may be called on a dumped area chunk,
but in both cases it can be verified that tagging is already
avoided for dumped area chunks.
Reviewed-by: DJ Delorie <dj@redhat.com>
I used these shell commands:
../glibc/scripts/update-copyrights $PWD/../gnulib/build-aux/update-copyright
(cd ../glibc && git commit -am"[this commit message]")
and then ignored the output, which consisted lines saying "FOO: warning:
copyright statement not found" for each of 6694 files FOO.
I then removed trailing white space from benchtests/bench-pthread-locks.c
and iconvdata/tst-iconv-big5-hkscs-to-2ucs4.c, to work around this
diagnostic from Savannah:
remote: *** pre-commit check failed ...
remote: *** error: lines with trailing whitespace found
remote: error: hook declined to update refs/heads/master
This patch adds the basic support for memory tagging.
Various flavours are supported, particularly being able to turn on
tagged memory at run-time: this allows the same code to be used on
systems where memory tagging support is not present without neededing
a separate build of glibc. Also, depending on whether the kernel
supports it, the code will use mmap for the default arena if morecore
does not, or cannot support tagged memory (on AArch64 it is not
available).
All the hooks use function pointers to allow this to work without
needing ifuncs.
Reviewed-by: DJ Delorie <dj@redhat.com>
The secondary/non-primary/inner libc (loaded via dlmopen, LD_AUDIT,
static dlopen) must not use sbrk to allocate member because that would
interfere with allocations in the outer libc. On Linux, this does not
matter because sbrk itself was changed to fail in secondary libcs.
_dl_addr occasionally shows up in profiles, but had to be used before
because __libc_multiple_libs was unreliable. So this change achieves
a slight reduction in startup time.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
malloc debug: fix compile error when enable macro MALLOC_DEBUG > 1.
this is because commit e9c4fe93b3 has change the struct malloc_chunk's member "size" to "mchunk_size".
the reproduction is like that:
setp1: modify related Makefile.
vim ../glibc/malloc/Makefile
CPPFLAGS-malloc.o += -DMALLOC_DEBUG=2
step2: ../configure --prefix=/usr
make -j32
this will cause the compile error:
/home/liqingqing/glibc_upstream/buildglibc/malloc/malloc.o
In file included from malloc.c:1899:0:
arena.c: In function 'dump_heap':
arena.c:422:58: error: 'struct malloc_chunk' has no member named 'size'
fprintf (stderr, "chunk %p size %10lx", p, (long) p->size);
^~
arena.c:428:17: error: 'struct malloc_chunk' has no member named 'size'
else if (p->size == (0 | PREV_INUSE))
Reviewed-by: DJ Delorie <dj@redhat.com>
Remove do_set_mallopt_check prototype since it is unused.
* malloc/arena.c (do_set_mallopt_check): Removed.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: DJ Delorie <dj@redhat.com>
This commit is in preparation of turning the macro into a proper
function. The output arguments of the macro were in fact unused.
Also clean up uses of __builtin_expect.
It does not make sense to register separate cleanup functions for arena
and tcache since they're always going to be called together. Call the
tcache cleanup function from within arena_thread_freeres since it at
least makes the order of those cleanups clear in the code.
Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
The current malloc initialization is quite convoluted. Instead of
sometimes calling malloc_consolidate from ptmalloc_init, call
malloc_init_state early so that the main_arena is always initialized.
The special initialization can now be removed from malloc_consolidate.
This also fixes BZ #22159.
Check all calls to malloc_consolidate and remove calls that are
redundant initialization after ptmalloc_init, like in int_mallinfo
and __libc_mallopt (but keep the latter as consolidation is required for
set_max_fast). Update comments to improve clarity.
Remove impossible initialization check from _int_malloc, fix assert
in do_check_malloc_state to ensure arena->top != 0. Fix the obvious bugs
in do_check_free_chunk and do_check_remalloced_chunk to enable single
threaded malloc debugging (do_check_malloc_state is not thread safe!).
[BZ #22159]
* malloc/arena.c (ptmalloc_init): Call malloc_init_state.
* malloc/malloc.c (do_check_free_chunk): Fix build bug.
(do_check_remalloced_chunk): Fix build bug.
(do_check_malloc_state): Add assert that checks arena->top.
(malloc_consolidate): Remove initialization.
(int_mallinfo): Remove call to malloc_consolidate.
(__libc_mallopt): Clarify why malloc_consolidate is needed.
Clean up calls to malloc_printerr and trim its argument list.
This also removes a few bits of work done before calling
malloc_printerr (such as unlocking operations).
The tunable/environment variable still enables the lightweight
additional malloc checking, but mallopt (M_CHECK_ACTION)
no longer has any effect.
The TUNABLE_SET_VALUE and family of macros (and my later attempt to
add a TUNABLE_GET) never quite went together very well because the
overall interface was not clearly defined. This patch is an attempt
to do just that.
This patch consolidates the API to two simple sets of macros,
TUNABLE_GET* and TUNABLE_SET*. If TUNABLE_NAMESPACE is defined,
TUNABLE_GET takes just the tunable name, type and a (optionally NULL)
callback function to get the value of the tunable. The callback
function, if non-NULL, is called if the tunable was externally set
(i.e. via GLIBC_TUNABLES or any future mechanism). For example:
val = TUNABLE_GET (check, int32_t, check_callback)
returns the value of the glibc.malloc.check tunable (assuming
TUNABLE_NAMESPACE is set to malloc) as an int32_t into VAL after
calling check_callback.
Likewise, TUNABLE_SET can be used to set the value of the tunable,
although this is currently possible only in the dynamic linker before
it relocates itself. For example:
TUNABLE_SET (check, int32_t, 2)
will set glibc.malloc.check to 2. Of course, this is not possible
since we set (or read) glibc.malloc.check long after it is relocated.
To access or set a tunable outside of TUNABLE_NAMESPACE, use the
TUNABLE_GET_FULL and TUNABLE_SET_FULL macros, which have the following
prototype:
TUNABLE_GET_FULL (glibc, tune, hwcap_mask, uint64_t, NULL)
TUNABLE_SET_FULL (glibc, tune, hwcap_mask, uint64_t, 0xffff)
In future the tunable list may get split into mutable and immutable
tunables where mutable tunables can be modified by the library and
userspace after relocation as well and TUNABLE_SET will be more useful
than it currently is. However whenever we actually do that split, we
will have to ensure that the mutable tunables are protected with
locks.
* elf/Versions (__tunable_set_val): Rename to __tunable_get_val.
* elf/dl-tunables.c: Likewise.
(do_tunable_update_val): New function.
(__tunable_set_val): New function.
(__tunable_get_val): Call CB only if the tunable was externally
initialized.
(tunables_strtoul): Replace strval with initialized.
* elf/dl-tunables.h (strval): Replace with a bool initialized.
(TUNABLE_ENUM_NAME, TUNABLE_ENUM_NAME1): Adjust names to
prevent collision.
(__tunable_set_val): New function.
(TUNABLE_GET, TUNABLE_GET_FULL): New macros.
(TUNABLE_SET, TUNABLE_SET_FULL): Likewise.
(TUNABLE_SET_VAL): Remove.
(TUNABLE_SET_VAL_WITH_CALLBACK): Likewise.
* README.tunables: Document the new macros.
* malloc/arena.c (ptmalloc_init): Adjust.
The code to set value passed a tunable_val_t, which when cast to
int32_t on big-endian gives the wrong value. Instead, use
tunable_val_t.numval instead, which can then be safely cast into
int32_t.
The tunables framework allows us to uniformly manage and expose global
variables inside glibc as switches to users. tunables/README has
instructions for glibc developers to add new tunables.
Tunables support can be enabled by passing the --enable-tunables
configure flag to the configure script. This patch only adds a
framework and does not pose any limitations on how tunable values are
read from the user. It also adds environment variables used in malloc
behaviour tweaking to the tunables framework as a PoC of the
compatibility interface.
* manual/install.texi: Add --enable-tunables option.
* INSTALL: Regenerate.
* README.tunables: New file.
* Makeconfig (CPPFLAGS): Define TOP_NAMESPACE.
(before-compile): Generate dl-tunable-list.h early.
* config.h.in: Add HAVE_TUNABLES.
* config.make.in: Add have-tunables.
* configure.ac: Add --enable-tunables option.
* configure: Regenerate.
* csu/init-first.c (__libc_init_first): Move
__libc_init_secure earlier...
* csu/init-first.c (LIBC_START_MAIN):... to here.
Include dl-tunables.h, libc-internal.h.
(LIBC_START_MAIN) [!SHARED]: Initialize tunables for static
binaries.
* elf/Makefile (dl-routines): Add dl-tunables.
* elf/Versions (ld): Add __tunable_set_val to GLIBC_PRIVATE
namespace.
* elf/dl-support (_dl_nondynamic_init): Unset MALLOC_CHECK_
only when !HAVE_TUNABLES.
* elf/rtld.c (process_envvars): Likewise.
* elf/dl-sysdep.c [HAVE_TUNABLES]: Include dl-tunables.h
(_dl_sysdep_start): Call __tunables_init.
* elf/dl-tunable-types.h: New file.
* elf/dl-tunables.c: New file.
* elf/dl-tunables.h: New file.
* elf/dl-tunables.list: New file.
* malloc/tst-malloc-usable-static.c: New test case.
* malloc/Makefile (tests-static): Add it.
* malloc/arena.c [HAVE_TUNABLES]: Include dl-tunables.h.
Define TUNABLE_NAMESPACE.
(DL_TUNABLE_CALLBACK (set_mallopt_check)): New function.
(DL_TUNABLE_CALLBACK_FNDECL): New macro. Use it to define
callback functions.
(ptmalloc_init): Set tunable values.
* scripts/gen-tunables.awk: New file.
* sysdeps/mach/hurd/dl-sysdep.c: Include dl-tunables.h.
(_dl_sysdep_start): Call __tunables_init.
It is necessary to preserve the invariant that if an arena is
on the free list, it has thread attach count zero. Otherwise,
when arena_thread_freeres sees the zero attach count, it will
add it, and without the invariant, an arena could get pushed
to the list twice, resulting in a cycle.
One possible execution trace looks like this:
Thread 1 examines free list and observes it as empty.
Thread 2 exits and adds its arena to the free list,
with attached_threads == 0).
Thread 1 selects this arena in reused_arena (not from the free list).
Thread 1 increments attached_threads and attaches itself.
(The arena remains on the free list.)
Thread 1 exits, decrements attached_threads,
and adds the arena to the free list.
The final step creates a cycle in the usual way (by overwriting the
next_free member with the former list head, while there is another
list item pointing to the arena structure).
tst-malloc-thread-exit exhibits this issue, but it was only visible
with a debugger because the incorrect fix in bug 19243 removed
the assert from get_free_list.
Before this change, the while loop in reused_arena which avoids
returning a corrupt arena would never execute its body if the selected
arena were not corrupt. As a result, result == begin after the loop,
and the function returns NULL, triggering fallback to mmap.
__malloc_initialize_hook is interposed by application code, so
the usual approach to define a compatibility symbol does not work.
This commit adds a new mechanism based on #pragma GCC poison in
<stdc-predef.h>.
The fork handler now runs so late that there is no risk anymore that
other fork handlers in the same thread use malloc, so it is no
longer necessary to install malloc hooks which made a subset
of malloc functionality available to the thread that called fork.
Previously, a thread M invoking fork would acquire locks in this order:
(M1) malloc arena locks (in the registered fork handler)
(M2) libio list lock
A thread F invoking flush (NULL) would acquire locks in this order:
(F1) libio list lock
(F2) individual _IO_FILE locks
A thread G running getdelim would use this order:
(G1) _IO_FILE lock
(G2) malloc arena lock
After executing (M1), (F1), (G1), none of the threads can make progress.
This commit changes the fork lock order to:
(M'1) libio list lock
(M'2) malloc arena locks
It explicitly encodes the lock order in the implementations of fork,
and does not rely on the registration order, thus avoiding the deadlock.
* malloc/arena.c (list_lock): Document lock ordering requirements.
(free_list_lock): New lock.
(ptmalloc_lock_all): Comment on free_list_lock.
(ptmalloc_unlock_all2): Reinitialize free_list_lock.
(detach_arena): Update comment. free_list_lock is now needed.
(_int_new_arena): Use free_list_lock around detach_arena call.
Acquire arena lock after list_lock. Add comment, including FIXME
about incorrect synchronization.
(get_free_list): Switch to free_list_lock.
(reused_arena): Acquire free_list_lock around detach_arena call
and attached threads counter update. Add two FIXMEs about
incorrect synchronization.
(arena_thread_freeres): Switch to free_list_lock.
* malloc/malloc.c (struct malloc_state): Update comments to
mention free_list_lock.
reused_arena can increase the attached thread count of arenas on the
free list. This means that the assertion that the reference count is
zero is incorrect. In this case, the reference count initialization
is incorrect as well and could cause arenas to be put on the free
list too early (while they still have attached threads).
* malloc/arena.c (get_free_list): Remove assert and adjust
reference count handling. Add comment about reused_arena
interaction.
(reused_arena): Add comments abount get_free_list interaction.
* malloc/tst-malloc-thread-exit.c: New file.
* malloc/Makefile (tests): Add tst-malloc-thread-exit.
(tst-malloc-thread-exit): Link against libpthread.
In the per-thread arenas we apply trim_threshold-based checks
to the extra space between the pad and the top_area. This isn't
quite accurate and instead we should be harmonizing with the way
in which trim_treshold is applied everywhere else like sysrtim
and _int_free. The trimming check should be based on the size of
the top chunk and only the size of the top chunk. The following
patch harmonizes the trimming and make it consistent for the main
arena and thread arenas.
In the old code a large padding request might have meant that
trimming was not triggered. Now trimming is considered first based
on the chunk, then the pad is subtracted, and the remainder trimmed.
This is how all the other trimmings operate. I didn't measure the
performance difference of this change because it corrects what I
consider to be a behavioural anomaly. We'll need some profile driven
optimization to make this code better, and even there Ondrej and
others have better ideas on how to speedup malloc.
Tested on x86_64 with no regressions. Already reviewed by Siddhesh
Poyarekar and Mel Gorman here and discussed here:
https://sourceware.org/ml/libc-alpha/2015-05/msg00002.html
If allocation on a non-main arena fails, the main arena is used
without checking to see if it is corrupt. Add a check that avoids the
main arena if it is corrupt.
* malloc/arena.c (arena_get_retry): Don't use main_arena if it is
corrupt.
The arena pointer in the first argument to arena_get2 was used in the
old days before per-thread arenas. They're unused now and hence can
be dropped.
ChangeLog:
* malloc/arena.c (arena_get2): Drop unused argument.
(arena_lock): Adjust.
(arena_get_retry): Likewise.
mksquashfs was reported in openSUSE to be causing segmentation faults when
creating installation images. Testing showed that mksquashfs sometimes
failed and could be reproduced within 10 attempts. The core dump looked
like the heap top was corrupted and was pointing to an unmapped area. In
other cases, this has been due to an application corrupting glibc structures
but mksquashfs appears to be fine in this regard.
The problem is that heap_trim is "growing" the top into unmapped space.
If the top chunk == MINSIZE then top_area is -1 and this check does not
behave as expected due to a signed/unsigned comparison
if (top_area <= pad)
return 0;
The next calculation extra = ALIGN_DOWN(top_area - pad, pagesz) calculates
extra as a negative number which also is unnoticed due to a signed/unsigned
comparison. We then call shrink_heap(heap, negative_number) which crashes
later. This patch adds a simple check against MINSIZE to make sure extra
does not become negative. It adds a cast to hint to the reader that this
is a signed vs unsigned issue.
Without the patch, mksquash fails within 10 attempts. With it applied, it
completed 1000 times without error. The standard test suite "make check"
showed no changes in the summary of test results.
