While doing code review I converted another bespoke round down, and
corrected a comment.
The comment spoke about keeping at least one page allocated even
during systrim, which is not correct. The code does nothing to keep
a page allocated. The code does attempt to keep PAD padding as
documented in comments and MINSIZE as required by design.
Historically in 2002 when Ulrich wrote the code (fa8d436c) the math
was inlined into one statement which did reserve an extra page:
extra = ((top_size - pad - MINSIZE + (pagesz-1)) / pagesz - 1) * pagesz;
There is no reason given for this extra page.
In 2010 Anton Branchard's change (b9b42ee0) from division
to shifts removed the extra page by dropping the "+ (pagesiz-1), which
mean we might have attempted to return -0 via MORECORE. The fix by Will
Newton in 2014 added a check for extra being zero (51a7380b).
From first principles I see no reason why we should keep an extra
page of memory from being trimmed back to the OS. The only sensible
interface is to honour PAD padding as the function is documented,
with the caveat the MINSIZE is maintained for the top chunk.
Given that we've been using this code for 5+ years with no extra
page allocated is sufficient evidence that the comment should be changed
to match the code that I'm touching.
Tested on x86_64 and i686, no regressions.
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.
[BZ #17581] The checking chain of unused chunks was terminated by a hash of
the block pointer, which was sometimes confused with the chunk length byte.
We now avoid using a length byte equal to the magic byte.
When the malloc subsystem detects some kind of memory corruption,
depending on the configuration it prints the error, a backtrace, a
memory map and then aborts the process. In this process, the
backtrace() call may result in a call to malloc, resulting in
various kinds of problematic behavior.
In one case, the malloc it calls may detect a corruption and call
backtrace again, and a stack overflow may result due to the infinite
recursion. In another case, the malloc it calls may deadlock on an
arena lock with the malloc (or free, realloc, etc.) that detected the
corruption. In yet another case, if the program is linked with
pthreads, backtrace may do a pthread_once initialization, which
deadlocks on itself.
In all these cases, the program exit is not as intended. This is
avoidable by marking the arena that malloc detected a corruption on,
as unusable. The following patch does that. Features of this patch
are as follows:
- A flag is added to the mstate struct of the arena to indicate if the
arena is corrupt.
- The flag is checked whenever malloc functions try to get a lock on
an arena. If the arena is unusable, a NULL is returned, causing the
malloc to use mmap or try the next arena.
- malloc_printerr sets the corrupt flag on the arena when it detects a
corruption
- free does not concern itself with the flag at all. It is not
important since the backtrace workflow does not need free. A free
in a parallel thread may cause another corruption, but that's not
new
- The flag check and set are not atomic and may race. This is fine
since we don't care about contention during the flag check. We want
to make sure that the malloc call in the backtrace does not trip on
itself and all that action happens in the same thread and not across
threads.
I verified that the test case does not show any regressions due to
this patch. I also ran the malloc benchmarks and found an
insignificant difference in timings (< 2%).
* malloc/Makefile (tests): New test case tst-malloc-backtrace.
* malloc/arena.c (arena_lock): Check if arena is corrupt.
(reused_arena): Find a non-corrupt arena.
(heap_trim): Pass arena to unlink.
* malloc/hooks.c (malloc_check_get_size): Pass arena to
malloc_printerr.
(top_check): Likewise.
(free_check): Likewise.
(realloc_check): Likewise.
* malloc/malloc.c (malloc_printerr): Add arena argument.
(unlink): Likewise.
(munmap_chunk): Adjust.
(ARENA_CORRUPTION_BIT): New macro.
(arena_is_corrupt): Likewise.
(set_arena_corrupt): Likewise.
(sysmalloc): Use mmap if there are no usable arenas.
(_int_malloc): Likewise.
(__libc_malloc): Don't fail if arena_get returns NULL.
(_mid_memalign): Likewise.
(__libc_calloc): Likewise.
(__libc_realloc): Adjust for additional argument to
malloc_printerr.
(_int_free): Likewise.
(malloc_consolidate): Likewise.
(_int_realloc): Likewise.
(_int_memalign): Don't touch corrupt arenas.
* malloc/tst-malloc-backtrace.c: New test case.
Trimming heaps is a balance between saving memory and the system overhead
required to update page tables and discard allocated pages. The malloc
option M_TRIM_THRESHOLD is a tunable that users are meant to use to decide
where this balance point is but it is only applied to the main arena.
For scalability reasons, glibc malloc has per-thread heaps but these are
shrunk with madvise() if there is one page free at the top of the heap.
In some circumstances this can lead to high system overhead if a thread
has a control flow like
while (data_to_process) {
buf = malloc(large_size);
do_stuff();
free(buf);
}
For a large size, the free() will call madvise (pagetable teardown, page
free and TLB flush) every time followed immediately by a malloc (fault,
kernel page alloc, zeroing and charge accounting). The kernel overhead
can dominate such a workload.
This patch allows the user to tune when madvise gets called by applying
the trim threshold to the per-thread heaps and using similar logic to the
main arena when deciding whether to shrink. Alternatively if the dynamic
brk/mmap threshold gets adjusted then the new values will be obeyed by
the per-thread heaps.
Bug 17195 was a test case motivated by a problem encountered in scientific
applications written in python that performance badly due to high page fault
overhead. The basic operation of such a program was posted by Julian Taylor
https://sourceware.org/ml/libc-alpha/2015-02/msg00373.html
With this patch applied, the overhead is eliminated. All numbers in this
report are in seconds and were recorded by running Julian's program 30
times.
pyarray
glibc madvise
2.21 v2
System min 1.81 ( 0.00%) 0.00 (100.00%)
System mean 1.93 ( 0.00%) 0.02 ( 99.20%)
System stddev 0.06 ( 0.00%) 0.01 ( 88.99%)
System max 2.06 ( 0.00%) 0.03 ( 98.54%)
Elapsed min 3.26 ( 0.00%) 2.37 ( 27.30%)
Elapsed mean 3.39 ( 0.00%) 2.41 ( 28.84%)
Elapsed stddev 0.14 ( 0.00%) 0.02 ( 82.73%)
Elapsed max 4.05 ( 0.00%) 2.47 ( 39.01%)
glibc madvise
2.21 v2
User 141.86 142.28
System 57.94 0.60
Elapsed 102.02 72.66
Note that almost a minutes worth of system time is eliminted and the
program completes 28% faster on average.
To illustrate the problem without python this is a basic test-case for
the worst case scenario where every free is a madvise followed by a an alloc
/* gcc bench-free.c -lpthread -o bench-free */
static int num = 1024;
void __attribute__((noinline,noclone)) dostuff (void *p)
{
}
void *worker (void *data)
{
int i;
for (i = num; i--;)
{
void *m = malloc (48*4096);
dostuff (m);
free (m);
}
return NULL;
}
int main()
{
int i;
pthread_t t;
void *ret;
if (pthread_create (&t, NULL, worker, NULL))
exit (2);
if (pthread_join (t, &ret))
exit (3);
return 0;
}
Before the patch, this resulted in 1024 calls to madvise. With the patch applied,
madvise is called twice because the default trim threshold is high enough to avoid
this.
This a more complex case where there is a mix of frees. It's simply a different worker
function for the test case above
void *worker (void *data)
{
int i;
int j = 0;
void *free_index[num];
for (i = num; i--;)
{
void *m = malloc ((i % 58) *4096);
dostuff (m);
if (i % 2 == 0) {
free (m);
} else {
free_index[j++] = m;
}
}
for (; j >= 0; j--)
{
free(free_index[j]);
}
return NULL;
}
glibc 2.21 calls malloc 90305 times but with the patch applied, it's
called 13438. Increasing the trim threshold will decrease the number of
times it's called with the option of eliminating the overhead.
ebizzy is meant to generate a workload resembling common web application
server workloads. It is threaded with a large working set that at its core
has an allocation, do_stuff, free loop that also hits this case. The primary
metric of the benchmark is records processed per second. This is running on
my desktop which is a single socket machine with an I7-4770 and 8 cores.
Each thread count was run for 30 seconds. It was only run once as the
performance difference is so high that the variation is insignificant.
glibc 2.21 patch
threads 1 10230 44114
threads 2 19153 84925
threads 4 34295 134569
threads 8 51007 183387
Note that the saving happens to be a concidence as the size allocated
by ebizzy was less than the default threshold. If a different number of
chunks were specified then it may also be necessary to tune the threshold
to compensate
This is roughly quadrupling the performance of this benchmark. The difference in
system CPU usage illustrates why.
ebizzy running 1 thread with glibc 2.21
10230 records/s 306904
real 30.00 s
user 7.47 s
sys 22.49 s
22.49 seconds was spent in the kernel for a workload runinng 30 seconds. With the
patch applied
ebizzy running 1 thread with patch applied
44126 records/s 1323792
real 30.00 s
user 29.97 s
sys 0.00 s
system CPU usage was zero with the patch applied. strace shows that glibc
running this workload calls madvise approximately 9000 times a second. With
the patch applied madvise was called twice during the workload (or 0.06
times per second).
2015-02-10 Mel Gorman <mgorman@suse.de>
[BZ #17195]
* malloc/arena.c (free): Apply trim threshold to per-thread heaps
as well as the main arena.
This seems to have been left behind as an artifact of some old changes
and can now be merged. Verified that the only generated code change
on x86_64 is that of line numbers in asserts, like so:
@@ -27253,7 +27253,7 @@ Disassembly of section .text:
416f09: 48 89 42 20 mov %rax,0x20(%rdx)
416f0d: e9 7e f6 ff ff jmpq 416590 <_int_free+0x230>
416f12: b9 3f 9f 4a 00 mov $0x4a9f3f,%ecx
- 416f17: ba d5 0f 00 00 mov $0xfd5,%edx
+ 416f17: ba d6 0f 00 00 mov $0xfd6,%edx
416f1c: be a8 9b 4a 00 mov $0x4a9ba8,%esi
416f21: bf 6a 9c 4a 00 mov $0x4a9c6a,%edi
416f26: e8 45 e8 ff ff callq 415770 <__malloc_assert>
We are replacing all of the bespoke alignment code with
ALIGN_UP, ALIGN_DOWN, PTR_ALIGN_UP, and PTR_ALIGN_DOWN.
This cleans up malloc/malloc.c, malloc/arena.c, and
elf/dl-reloc.c. It also makes all the code consistently
use pagesize, and powerof2 as required.
Code size is reduced with the removal of precomputed
pagemask, and use of pagesize instead. No measurable
difference in performance.
No regressions on x86_64.
[BZ #17581] The checking chain of unused chunks was terminated by a hash of
the block pointer, which was sometimes confused with the chunk length byte.
The chain is now terminated by a NULL byte.
The current scheme to identify which module a translation unit is
built in depends on defining multiple macros IS_IN_* and also defining
NOT_IN_libc if we're building a non-libc module. In addition, there
is an IN_LIB macro that does effectively the same thing, but for
different modules (notably the systemtap probes). This macro scheme
unifies both ideas to use just one macro IN_MODULE and assign it a
value depending on the module it is being built into. If the module
is not defined, it defaults to MODULE_libc.
Patches that follow will replace uses of IS_IN_* variables with the
IS_IN() macro. libc-symbols.h has been converted already to give an
example of how such a transition will look.
Verified that there are no relevant binary changes. One source change
that will crop up repeatedly is that of nscd_stat, since it uses the
build timestamp as a constant in its logic.
* Makeconfig (in-module): Get value of libof set for the
translation unit.
(CPPFLAGS): Use $(in-module).
* Makerules: Don't suffix routine names for nonlib.
* include/libc-modules.h: New file.
* include/libc-symbols.h: Include libc-modules.h
(IS_IN): New macro to replace IS_IN_* macros.
* elf/Makefile: Set libof-* for each routine.
* elf/rtld-Rules: Likewise.
* extra-modules.mk: Likewise.
* iconv/Makefile: Likewise.
* iconvdata/Makefile: Likewise.
* locale/Makefile: Likewise.
* malloc/Makefile: Likewise.
* nss/Makefile: Likewise.
* sysdeps/gnu/Makefile: Likewise.
* sysdeps/ieee754/ldbl-opt/Makefile: Likewise.
* sysdeps/unix/sysv/linux/Makefile: Likewise.
* sysdeps/s390/s390-64/Makefile: Likewise.
* nscd/Makefile: Set libof-* for each routine. Set CFLAGS and
CPPFLAGS for nscd instead of nonlib.
malloc_info is defined in the same file as malloc and free, but is not
an ISO C function, so should be a weak symbol. This patch makes it
so.
Tested for x86_64 (testsuite, and that disassembly of installed shared
libraries is unchanged by the patch).
[BZ #17570]
* malloc/malloc.c (malloc_info): Rename to __malloc_info and
define as weak alias of __malloc_info.
Merge the latest version of the obstack.c and obstack.h files
from gnulib. The majority of this change is coding style and
cosmetic comment changes but it also fixes a -Wundef warning
in the build as a side effect.
2014-07-02 Will Newton <will.newton@linaro.org>
* malloc/obstack.c: Merge from gnulib master.
[HAVE_CONFIG_H]: Remove conditional code.
[!_LIBC]: Include config.h.
[!ELIDE_CODE]: Don't include inttypes.h, include
stdint.h unconditionally.
(print_and_abort): Mark as _Noreturn.
(_obstack_allocated_p): Mark as __attribute_pure__.
(obstack_free): Rename to __obstack_free.
[!__attribute__]: Remove conditional code.
* malloc/obstack.h: Merge from gnulib master.
[__cplusplus]: Move conditional down.
[!__attribute_pure__]: Define __attribute_pure__ here
if it is not already defined.
(_obstack_memory_used): Mark as __attribute_pure__.
[!__obstack_free]: Define as obstack_free.
[__GNUC__]: Remove check for ancient NeXT gcc.
Due to my bad review suggestion for the fix for BZ #15089 a check
was removed from systrim to prevent sbrk being called with a zero
argument. Add the check back to avoid this useless work.
ChangeLog:
2014-06-19 Will Newton <will.newton@linaro.org>
* malloc/malloc.c (systrim): If extra is zero then return
early.
One wart in the original support for test wrappers for cross testing,
as noted in
<https://sourceware.org/ml/libc-alpha/2012-10/msg00722.html>, is the
requirement for test wrappers to pass a poorly-defined set of
environment variables from the build system to the system running the
glibc under test. Although some variables are passed explicitly via
$(test-wrapper-env), including LD_* variables that simply can't be
passed implicitly because of the side effects they'd have on the build
system's dynamic linker, others are passed implicitly, including
variables such as GCONV_PATH and LOCPATH that could potentially affect
the build system's libc (so effectively relying on any such effects
not breaking the wrappers). In addition, the code in
cross-test-ssh.sh for preserving environment variables is fragile (it
depends on how bash formats a list of exported variables, and could
well break for multi-line variable definitions where the contents
contain things looking like other variable definitions).
This patch moves to explicitly passing environment variables via
$(test-wrapper-env). Makefile variables that previously used
$(test-wrapper) are split up into -before-env and -after-env parts
that can be passed separately to the various .sh files used in
testing, so those files can then insert environment settings between
the two parts.
The common default environment settings in make-test-out are made into
a separate makefile variable that can also be passed to scripts,
rather than many scripts duplicating those settings (for testing an
installed glibc, it is desirable to have the GCONV_PATH setting on
just one place, so just that one place needs to support it pointing to
an installed sysroot instead of the build tree). The default settings
are included in the variables such as $(test-program-prefix), so that
if tests do not need any non-default settings they can continue to use
single variables rather than the split-up variables.
Although this patch cleans up LC_ALL=C settings (that being part of
the common defaults), various LANG=C and LANGUAGE=C settings remain.
Those are generally unnecessary and I propose a subsequent cleanup to
remove them. LC_ALL takes precedence over LANG, and while LANGUAGE
takes precedence over LC_ALL, it only does so for settings other than
LC_ALL=C. So LC_ALL=C on its own is sufficient to ensure the C
locale, and anything that gets LC_ALL=C does not need the other
settings.
While preparing this patch I noticed some tests with .sh files that
appeared to do nothing beyond what the generic makefile support for
tests can do (localedata/tst-wctype.sh - the makefiles support -ENV
variables and .input files - and localedata/tst-mbswcs.sh - just runs
five tests that could be run individually from the makefile). So I
propose another subsequent cleanup to move those to using the generic
support instead of special .sh files.
Tested x86_64 (native) and powerpc32 (cross).
* Makeconfig (run-program-env): New variable.
(run-program-prefix-before-env): Likewise.
(run-program-prefix-after-env): Likewise.
(run-program-prefix): Define in terms of new variables.
(built-program-cmd-before-env): New variable.
(built-program-cmd-after-env): Likewise.
(built-program-cmd): Define in terms of new variables.
(test-program-prefix-before-env): New variable.
(test-program-prefix-after-env): Likewise.
(test-program-prefix): Define in terms of new variables.
(test-program-cmd-before-env): New variable.
(test-program-cmd-after-env): Likewise.
(test-program-cmd): Define in terms of new variables.
* Rules (make-test-out): Use $(run-program-env).
* scripts/cross-test-ssh.sh (env_blacklist): Remove variable.
(help): Do not mention environment variables. Mention
--timeoutfactor option.
(timeoutfactor): New variable.
(blacklist_exports): Remove function.
(exports): Remove variable.
(command): Do not include ${exports}.
* manual/install.texi (Configuring and compiling): Do not mention
test wrappers preserving environment variables. Mention that last
assignment to a variable must take precedence.
* INSTALL: Regenerated.
* benchtests/Makefile (run-bench): Use $(run-program-env).
* catgets/Makefile ($(objpfx)test1.cat): Use
$(built-program-cmd-before-env), $(run-program-env) and
$(built-program-cmd-after-env).
($(objpfx)test2.cat): Do not specify environment variables
explicitly.
($(objpfx)de/libc.cat): Use $(built-program-cmd-before-env),
$(run-program-env) and $(built-program-cmd-after-env).
($(objpfx)test-gencat.out): Use $(test-program-cmd-before-env),
$(run-program-env) and $(test-program-cmd-after-env).
($(objpfx)sample.SJIS.cat): Do not specify environment variables
explicitly.
* catgets/test-gencat.sh: Use test_program_cmd_before_env,
run_program_env and test_program_cmd_after_env arguments.
* elf/Makefile ($(objpfx)tst-pathopt.out): Use $(run-program-env).
* elf/tst-pathopt.sh: Use run_program_env argument.
* iconvdata/Makefile ($(objpfx)iconv-test.out): Use
$(test-wrapper-env) and $(run-program-env).
* iconvdata/run-iconv-test.sh: Use test_wrapper_env and
run_program_env arguments.
* iconvdata/tst-table.sh: Do not set GCONV_PATH explicitly.
* intl/Makefile ($(objpfx)tst-gettext.out): Use
$(test-program-prefix-before-env), $(run-program-env) and
$(test-program-prefix-after-env).
($(objpfx)tst-gettext2.out): Likewise.
* intl/tst-gettext.sh: Use test_program_prefix_before_env,
run_program_env and test_program_prefix_after_env arguments.
* intl/tst-gettext2.sh: Likewise.
* intl/tst-gettext4.sh: Do not set environment variables
explicitly.
* intl/tst-gettext6.sh: Likewise.
* intl/tst-translit.sh: Likewise.
* malloc/Makefile ($(objpfx)tst-mtrace.out): Use
$(test-program-prefix-before-env), $(run-program-env) and
$(test-program-prefix-after-env).
* malloc/tst-mtrace.sh: Use test_program_prefix_before_env,
run_program_env and test_program_prefix_after_env arguments.
* math/Makefile (run-regen-ulps): Use $(run-program-env).
* nptl/Makefile ($(objpfx)tst-tls6.out): Use $(run-program-env).
* nptl/tst-tls6.sh: Use run_program_env argument. Set LANG=C
explicitly with each use of ${test_wrapper_env}.
* posix/Makefile ($(objpfx)wordexp-tst.out): Use
$(test-program-prefix-before-env), $(run-program-env) and
$(test-program-prefix-after-env).
* posix/tst-getconf.sh: Do not set environment variables
explicitly.
* posix/wordexp-tst.sh: Use test_program_prefix_before_env,
run_program_env and test_program_prefix_after_env arguments.
* stdio-common/tst-printf.sh: Do not set environment variables
explicitly.
* stdlib/Makefile ($(objpfx)tst-fmtmsg.out): Use
$(test-program-prefix-before-env), $(run-program-env) and
$(test-program-prefix-after-env).
* stdlib/tst-fmtmsg.sh: Use test_program_prefix_before_env,
run_program_env and test_program_prefix_after_env arguments.
Split $test calls into $test_pre and $test.
* timezone/Makefile (build-testdata): Use
$(built-program-cmd-before-env), $(run-program-env) and
$(built-program-cmd-after-env).
localedata/ChangeLog:
* Makefile ($(addprefix $(objpfx),$(CTYPE_FILES))): Use
$(built-program-cmd-before-env), $(run-program-env) and
$(built-program-cmd-after-env).
($(objpfx)sort-test.out): Use $(test-program-prefix-before-env),
$(run-program-env) and $(test-program-prefix-after-env).
($(objpfx)tst-fmon.out): Use $(run-program-prefix-before-env),
$(run-program-env) and $(run-program-prefix-after-env).
($(objpfx)tst-locale.out): Use $(built-program-cmd-before-env),
$(run-program-env) and $(built-program-cmd-after-env).
($(objpfx)tst-trans.out): Use $(run-program-prefix-before-env),
$(run-program-env), $(run-program-prefix-after-env),
$(test-program-prefix-before-env) and
$(test-program-prefix-after-env).
($(objpfx)tst-ctype.out): Use $(test-program-cmd-before-env),
$(run-program-env) and $(test-program-cmd-after-env).
($(objpfx)tst-wctype.out): Likewise.
($(objpfx)tst-langinfo.out): Likewise.
($(objpfx)tst-langinfo-static.out): Likewise.
* gen-locale.sh: Use localedef_before_env, run_program_env and
localedef_after_env arguments.
* sort-test.sh: Use test_program_prefix_before_env,
run_program_env and test_program_prefix_after_env arguments.
* tst-ctype.sh: Use tst_ctype_before_env, run_program_env and
tst_ctype_after_env arguments.
* tst-fmon.sh: Use run_program_prefix_before_env, run_program_env
and run_program_prefix_after_env arguments.
* tst-langinfo.sh: Use tst_langinfo_before_env, run_program_env
and tst_langinfo_after_env arguments.
* tst-locale.sh: Use localedef_before_env, run_program_env and
localedef_after_env arguments.
* tst-mbswcs.sh: Do not set environment variables explicitly.
* tst-numeric.sh: Likewise.
* tst-rpmatch.sh: Likewise.
* tst-trans.sh: Use run_program_prefix_before_env,
run_program_env, run_program_prefix_after_env,
test_program_prefix_before_env and test_program_prefix_after_env
arguments.
* tst-wctype.sh: Use tst_wctype_before_env, run_program_env and
tst_wctype_after_env arguments.
The current malloc_info xml output only has information about
allocations on the heap. Display information about number of mappings
and total mmapped size to this to complete the picture.
The nested function mi_arena was removed from malloc_info
and made into a non-nested static inline function of the same
name with the correct set of arguments passed from malloc_info.
This enables building glibc with compilers that don't support
nested functions. Future work on malloc_info should remove these
functions entirely to support JSON format output. Therefore we
do the minimum required to remove the nested function.
glibc's Makeconfig defines some variables such as $(libm) and $(libdl)
for linking with libraries built by glibc, and nptl/Makeconfig
(included by the toplevel Makeconfig) defines others such as
$(shared-thread-library).
In some places glibc's Makefiles use those variables when linking
against the relevant libraries, but in other places they hardcode the
location of the libraries in the build tree. This patch cleans up
various places to use the variables that already exist (in the case of
libm, replacing several duplicate definitions of a $(link-libm)
variable in subdirectory Makefiles). (It's not necessarily exactly
equivalent to what the existing code does - in particular,
$(shared-thread-library) includes libpthread_nonshared, but is
replacing places that just referred to libpthread.so. But I think
that change is desirable on the general principle of linking things as
close as possible to the way in which they would be linked with an
installed library, unless there is a clear reason not to do so.)
To support running tests with an installed copy of glibc without
needing the full build tree from when that copy was built, I think it
will be useful to use such variables more generally and systematically
- every time the rules for building a test refer to some file from the
build tree that's also installed by glibc, use a makefile variable so
that the installed-testing case can point those variables to installed
copies of the files. This patch just deals with straightforward cases
where such variables already exist.
It's quite possible some uses of $(shared-thread-library) should
actually be a new $(thread-library) variable that's set appropriately
in the --disable-shared case, if those uses would in fact work without
shared libraries. I didn't change the status quo that those cases
hardcode use of a shared library whether or not it's actually needed
(but other uses such as $(libm) and $(libdl) would now get the static
library if the shared library isn't built, when some previously
hardcoded use of the shared library - if they actually need shared
libraries, the test itself needs an enable-shared conditional anyway).
Tested x86_64.
* benchtests/Makefile
($(addprefix $(objpfx)bench-,$(bench-math))): Depend on $(libm),
not $(common-objpfx)math/libm.so.
($(addprefix $(objpfx)bench-,$(bench-pthread))): Depend on
$(shared-thread-library), not $(common-objpfx)nptl/libpthread.so.
* elf/Makefile ($(objpfx)noload): Depend on $(libdl), not
$(common-objpfx)dlfcn/libdl.so.
($(objpfx)tst-audit8): Depend on $(libm), not
$(common-objpfx)math/libm.so.
* malloc/Makefile ($(objpfx)libmemusage.so): Depend on $(libdl),
not $(common-objpfx)dlfcn/libdl.so.
* math/Makefile
($(addprefix $(objpfx),$(filter-out $(tests-static),$(tests)))):
Depend on $(libm), not $(objpfx)libm.so. Do not condition on
[$(build-shared) = yes].
($(objpfx)test-fenv-tls): Depend on $(shared-thread-library), not
$(common-objpfx)nptl/libpthread.so.
* misc/Makefile ($(objpfx)tst-tsearch): Depend on $(libm), not
$(common-objpfx)math/libm.so$(libm.so-version) or
$(common-objpfx)math/libm.a depending on [$(build-shared) = yes].
* nptl/Makefile ($(objpfx)tst-unload): Depend on $(libdl), not
$(common-objpfx)dlfcn/libdl.so.
* setjmp/Makefile (link-libm): Remove variable.
($(objpfx)tst-setjmp-fp): Depend on $(libm), not $(link-libm).
* stdio-common/Makefile (link-libm): Remove variable.
($(objpfx)tst-printf-round): Depend on $(libm), not $(link-libm).
* stdlib/Makefile (link-libm): Remove variable.
($(objpfx)bug-getcontext): Depend on $(libm), not $(link-libm).
($(objpfx)tst-strtod-round): Likewise.
($(objpfx)tst-tininess): Likewise.
($(objpfx)tst-strtod-underflow): Likewise.
($(objpfx)tst-strtod6): Likewise.
($(objpfx)tst-tls-atexit): Depend on $(shared-thread-library) and
$(libdl), not $(common-objpfx)nptl/libpthread.so and
$(common-objpfx)dlfcn/libdl.so.
MALLOC_DEBUG is set optionally on the command line. Default the value
to zero if it is not set on the command line, and test its value
with #if rather than #ifdef. Verified the code is identical before
and after this change apart from line numbers.
ChangeLog:
2014-04-11 Will Newton <will.newton@linaro.org>
* malloc/malloc.c [!MALLOC_DEBUG]: #define MALLOC_DEBUG
to zero if it is not defined elsewhere. (mtrim): Test
the value of MALLOC_DEBUG with #if rather than #ifdef.
This patch is a revised and updated version of
<https://sourceware.org/ml/libc-alpha/2014-01/msg00196.html>.
In order to generate overall summaries of the results of all tests in
the glibc testsuite, we need to identify and concatenate the files
with the results of individual tests.
Tomas Dohnalek's patch used $(common-objpfx)*/*.test-result for this.
However, the normal glibc approach is explicit enumeration of the
expected set of files with a given property, rather than all files
matching some pattern like that. Furthermore, we would like to be
able to mark tests as UNRESOLVED if the file with their results is for
some reason missing, and in future we would like to be able to mark
tests as UNSUPPORTED if they are disabled for a particular
configuration (rather than simply having them missing from the list of
tests as at present). Such handling of tests that were not run or did
not record results requires an explicit enumeration of tests.
For the tests following the default makefile rules, $(tests) (and
$(xtests)) provides such an enumeration. Others, however, are added
directly as dependencies of the "tests" and "xtests" makefile
targets. This patch changes the makefiles to put them in variables
tests-special and xtests-special, with appropriate dependencies on the
tests listed there then being added centrally.
Those variables are used in Rules and so need to be set before Rules
is included in a subdirectory makefile, which is often earlier in the
makefile than the dependencies were present before. We previously
discussed the question of where to include Rules; see the question at
<https://sourceware.org/ml/libc-alpha/2012-11/msg00798.html>, and a
discussion in
<https://sourceware.org/ml/libc-alpha/2013-01/msg00337.html> of why
Rules is included early rather than late in subdirectory makefiles.
It was necessary to avoid an indirection through the check-abi target
and get the check-abi-* targets for individual libraries into the
tests-special variable. The intl/ test $(objpfx)tst-gettext.out,
previously built only because of dependencies from other tests, was
also added to tests-special for the same reason.
The entries in tests-special are the full makefile targets, complete
with $(objpfx) and .out. If a future change causes tests to be named
consistently with a .out suffix, this can be changed to include just
the path relative to $(objpfx), without .out.
Tested x86_64, including that the same set of files is generated in
the build directory by a build and testsuite run both before and after
the patch (except for changes to the
elf/tst-null-argv.debug.out.<number> file name), and a build with
run-built-tests=no to verify there aren't any more obvious instances
of the issue Marcus Shawcroft reported with a previous version in
<https://sourceware.org/ml/libc-alpha/2014-01/msg00462.html>.
* Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
(tests): Depend on $(tests-special).
* Makerules (check-abi-list): New variable.
(check-abi): Depend on $(check-abi-list).
[$(subdir) = elf] (tests-special): Add
$(objpfx)check-abi-libc.out.
[$(build-shared) = yes && subdir] (tests-special): Add
$(check-abi-list).
[$(build-shared) = yes && subdir] (tests): Do not depend on
check-abi.
* Rules (tests): Depend on $(tests-special).
(xtests): Depend on $(xtests-special).
* catgets/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* conform/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* elf/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* grp/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* iconv/Makefile (xtests): Change dependencies to ....
(xtests-special): ... additions to this variable.
* iconvdata/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* intl/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable. Also add
$(objpfx)tst-gettext.out.
* io/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* libio/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* malloc/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* misc/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* nptl/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* nptl_db/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* posix/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
(xtests): Change dependencies to ....
(xtests-special): ... additions to this variable.
* resolv/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
(xtests): Change dependencies to ....
(xtests-special): ... additions to this variable.
* stdio-common/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
(do-tst-unbputc): Remove target.
(do-tst-printf): Likewise.
* stdlib/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* string/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
* sysdeps/x86/Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
localedata:
* Makefile (tests): Change dependencies to ....
(tests-special): ... additions to this variable.
Objections were raised surrounding the calloc simplification
and it is better to revert the patch, continue discussions
and then submit a new patch for inclusion with all issues
fully addressed.
In <https://sourceware.org/ml/libc-alpha/2014-01/msg00196.html> I
noted it was necessary to add includes of Makeconfig early in various
subdirectory makefiles for the tests-special variable settings added
by that patch to be conditional on configuration information. No-one
commented on the general question there of whether Makeconfig should
always be included immediately after the definition of subdir.
This patch implements that early inclusion of Makeconfig in each
directory (which is a lot easier than consistent placement of includes
of Rules). Includes are added if needed, or moved up if already
present. Subdirectory "all:" targets are removed, since Makeconfig
provides one.
There is potential for further cleanups I haven't done. Rules and
Makerules have code such as
ifneq "$(findstring env,$(origin headers))" ""
headers :=
endif
to override to empty any value of various variables that came from the
environment. I think there is a case for Makeconfig setting all the
subdirectory variables (other than subdir) to empty to ensure no
outside value is going to take effect if a subdirectory fails to
define a variable. (A list of such variables, possibly out of date
and incomplete, is in manual/maint.texi.) Rules and Makerules would
give errors if Makeconfig hadn't already been included, instead of
including it themselves. The special code to override values coming
from the environment would then be obsolete and could be removed.
Tested x86_64, including that installed binaries are identical before
and after the patch.
* argp/Makefile: Include Makeconfig immediately after defining
subdir.
* assert/Makefile: Likewise.
* benchtests/Makefile: Likewise.
* catgets/Makefile: Likewise.
* conform/Makefile: Likewise.
* crypt/Makefile: Likewise.
* csu/Makefile: Likewise.
(all): Remove target.
* ctype/Makefile: Include Makeconfig immediately after defining
subdir.
* debug/Makefile: Likewise.
* dirent/Makefile: Likewise.
* dlfcn/Makefile: Likewise.
* gmon/Makefile: Likewise.
* gnulib/Makefile: Likewise.
* grp/Makefile: Likewise.
* gshadow/Makefile: Likewise.
* hesiod/Makefile: Likewise.
* hurd/Makefile: Likewise.
(all): Remove target.
* iconvdata/Makefile: Include Makeconfig immediately after
defining subdir.
* inet/Makefile: Likewise.
* intl/Makefile: Likewise.
* io/Makefile: Likewise.
* libio/Makefile: Likewise.
(all): Remove target.
* locale/Makefile: Include Makeconfig immediately after defining
subdir.
* login/Makefile: Likewise.
* mach/Makefile: Likewise.
(all): Remove target.
* malloc/Makefile: Include Makeconfig immediately after defining
subdir.
(all): Remove target.
* manual/Makefile: Include Makeconfig immediately after defining
subdir.
* math/Makefile: Likewise.
* misc/Makefile: Likewise.
* nis/Makefile: Likewise.
* nss/Makefile: Likewise.
* po/Makefile: Likewise.
(all): Remove target.
* posix/Makefile: Include Makeconfig immediately after defining
subdir.
* pwd/Makefile: Likewise.
* resolv/Makefile: Likewise.
* resource/Makefile: Likewise.
* rt/Makefile: Likewise.
* setjmp/Makefile: Likewise.
* shadow/Makefile: Likewise.
* signal/Makefile: Likewise.
* socket/Makefile: Likewise.
* soft-fp/Makefile: Likewise.
* stdio-common/Makefile: Likewise.
* stdlib/Makefile: Likewise.
* streams/Makefile: Likewise.
* string/Makefile: Likewise.
* sunrpc/Makefile: Likewise.
(all): Remove target.
* sysvipc/Makefile: Include Makeconfig immediately after defining
subdir.
* termios/Makefile: Likewise.
* time/Makefile: Likewise.
* timezone/Makefile: Likewise.
(all): Remove target.
* wcsmbs/Makefile: Include Makeconfig immediately after defining
subdir.
* wctype/Makefile: Likewise.
libidn/ChangeLog:
* Makefile: Include Makeconfig immediately after defining subdir.
localedata/ChangeLog:
* Makefile: Include Makeconfig immediately after defining subdir.
(all): Remove target.
nptl/ChangeLog:
* Makefile: Include Makeconfig immediately after defining subdir.
nptl_db/ChangeLog:
* Makefile: Include Makeconfig immediately after defining subdir.
To make future improvements of allocator simpler we could for now calloc
just call malloc and memset. With that we could omit a changes that
would duplicate malloc changes anyway.
Remove an unused #define and use ANSI prototypes.
Generated code identical on x86_64-unknown-linux-gnu.
ChangeLog:
2014-02-11 Will Newton <will.newton@linaro.org>
* malloc/mtrace.c (attribute_hidden): Remove unused macro
definition. (tr_where, tr_freehook, tr_mallochook,
tr_reallochook, tr_memalignhook): Use ANSI protoype.
