* Tulio Magno Quites Machado Filho:
> I suspect this patch doesn't address all the comments from v1.
> However, I believe some of the open questions/comments may not be
> necessary anymore after the latest changes.
>
> I've decided to not add the new test to xtests, because it executes in
> less than 3s in most of my tests. There is just a single case that
> takes up to 30s.
>
> Changes since v1:
>
> - Fixed the coding style issues.
> - Replaced atomic loads/store with memory fences.
> - Added a test.
I don't think the fences are correct, they still need to be combined
with relaxed MO loads and stores.
Does the issue that Carlos mentioned really show up in cross-builds?
> diff --git a/nptl/tst-audit-threads-mod1.c b/nptl/tst-audit-threads-mod1.c
> new file mode 100644
> index 0000000000..e2d3f78bae
> --- /dev/null
> +++ b/nptl/tst-audit-threads-mod1.c
> +la_version(unsigned int ver)
> +la_objopen(struct link_map *map, Lmid_t lmid, uintptr_t *cookie)
Style: missing space before (.
> diff --git a/nptl/tst-audit-threads.c b/nptl/tst-audit-threads.c
> new file mode 100644
> index 0000000000..93ddebaecb
> --- /dev/null
> +++ b/nptl/tst-audit-threads.c
> + /* Used to synchronize all the threads after calling each retNumN. */
> + pthread_barrier_init (&barrier, NULL, num_threads);
xpthread_barrier_init.
> + threads = (pthread_t *) malloc (num_threads * sizeof(pthread_t));
> + bzero (threads, num_threads * sizeof(pthread_t));
xcalloc or xmalloc. But bzero does not appear to be required.
> + for (i = 0; i < num_threads; i++)
> + pthread_create(threads + i, NULL, thread_main, NULL);
xpthread_create.
> + for (i = 0; i < num_threads; i++)
> + pthread_join(threads[i], NULL);
xpthread_join.
Rest looks okay.
Thanks,
Florian
@@ -183,9 +183,18 @@ _dl_profile_fixup (
/* This is the address in the array where we store the result of previous
relocations. */
struct reloc_result *reloc_result = &l->l_reloc_result[reloc_index];
- DL_FIXUP_VALUE_TYPE *resultp = &reloc_result->addr;
+ /* CONCURRENCY NOTES:
+
+ The following code uses DL_FIXUP_VALUE_CODE_ADDR to access a potential
+ member of reloc_result->addr to indicate if it is the first time this
+ object is being relocated.
+ Reading/Writing from/to reloc_result->addr must not happen before previous
+ writes to reloc_result complete as they could end-up with an incomplete
+ struct. */
+ DL_FIXUP_VALUE_TYPE *resultp = &reloc_result->addr;
DL_FIXUP_VALUE_TYPE value = *resultp;
+ atomic_thread_fence_acquire ();
if (DL_FIXUP_VALUE_CODE_ADDR (value) == 0)
{
/* This is the first time we have to relocate this object. */
@@ -346,7 +355,13 @@ _dl_profile_fixup (
/* Store the result for later runs. */
if (__glibc_likely (! GLRO(dl_bind_not)))
- *resultp = value;
+ {
+ /* Guarantee all previous writes complete before
+ resultp (aka. reloc_result->addr) is updated. See CONCURRENCY
+ NOTES earlier */
+ atomic_thread_fence_release ();
+ *resultp = value;
+ }
}
/* By default we do not call the pltexit function. */
@@ -382,7 +382,8 @@ tests += tst-cancelx2 tst-cancelx3 tst-cancelx4 tst-cancelx5 \
tst-cleanupx0 tst-cleanupx1 tst-cleanupx2 tst-cleanupx3 tst-cleanupx4 \
tst-oncex3 tst-oncex4
ifeq ($(build-shared),yes)
-tests += tst-atfork2 tst-tls4 tst-_res1 tst-fini1 tst-compat-forwarder
+tests += tst-atfork2 tst-tls4 tst-_res1 tst-fini1 tst-compat-forwarder \
+ tst-audit-threads
tests-internal += tst-tls3 tst-tls3-malloc tst-tls5 tst-stackguard1
tests-nolibpthread += tst-fini1
ifeq ($(have-z-execstack),yes)
@@ -394,7 +395,8 @@ modules-names = tst-atfork2mod tst-tls3mod tst-tls4moda tst-tls4modb \
tst-tls5mod tst-tls5moda tst-tls5modb tst-tls5modc \
tst-tls5modd tst-tls5mode tst-tls5modf tst-stack4mod \
tst-_res1mod1 tst-_res1mod2 tst-execstack-mod tst-fini1mod \
- tst-join7mod tst-compat-forwarder-mod
+ tst-join7mod tst-compat-forwarder-mod tst-audit-threads-mod1 \
+ tst-audit-threads-mod2
extra-test-objs += $(addsuffix .os,$(strip $(modules-names))) \
tst-cleanup4aux.o tst-cleanupx4aux.o
test-extras += tst-cleanup4aux tst-cleanupx4aux
@@ -709,6 +711,14 @@ endif
$(objpfx)tst-compat-forwarder: $(objpfx)tst-compat-forwarder-mod.so
+ifeq ($(run-built-tests),yes)
+ifeq (yes,$(build-shared))
+$(objpfx)tst-audit-threads.out: $(objpfx)tst-audit-threads-mod1.so
+$(objpfx)tst-audit-threads: $(objpfx)tst-audit-threads-mod2.so
+tst-audit-threads-ENV = LD_AUDIT=$(objpfx)tst-audit-threads-mod1.so
+endif
+endif
+
# The tests here better do not run in parallel
ifneq ($(filter %tests,$(MAKECMDGOALS)),)
.NOTPARALLEL:
new file mode 100644
@@ -0,0 +1,38 @@
+/* Dummy audit library for test-audit-threads.
+
+ Copyright (C) 2018 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
+ <http://www.gnu.org/licenses/>. */
+
+#include <elf.h>
+#include <link.h>
+#include <stdio.h>
+#include <assert.h>
+#include <string.h>
+
+volatile int count = 0;
+
+unsigned int
+la_version(unsigned int ver)
+{
+ return 1;
+}
+
+unsigned int
+la_objopen(struct link_map *map, Lmid_t lmid, uintptr_t *cookie)
+{
+ return LA_FLG_BINDTO | LA_FLG_BINDFROM;
+}
new file mode 100644
@@ -0,0 +1,22 @@
+/* Shared object with a huge number of functions for test-audit-threads.
+
+ Copyright (C) 2018 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
+ <http://www.gnu.org/licenses/>. */
+
+/* Define all the retNumN functions. */
+#define definenum
+#include "tst-audit-threads.h"
new file mode 100644
@@ -0,0 +1,92 @@
+/* Test multi-threading using LD_AUDIT.
+
+ Copyright (C) 2018 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
+ <http://www.gnu.org/licenses/>. */
+
+/* This test uses a dummy LD_AUDIT library (test-audit-threads-mod1) and a
+ library with a huge number of functions in order to validate lazy symbol
+ binding with an audit library. */
+
+#include <pthread.h>
+#include <strings.h>
+#include <stdlib.h>
+#include <sys/sysinfo.h>
+
+static int do_test (void);
+
+/* This test usually takes less than 3s to run. However, there are cases that
+ take up to 30s. */
+#define TIMEOUT 60
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
+#define externnum
+#include "tst-audit-threads.h"
+#undef externnum
+
+int num_threads;
+pthread_barrier_t barrier;
+
+void
+sync_all (int num)
+{
+ pthread_barrier_wait (&barrier);
+}
+
+void
+call_all_ret_nums (void)
+{
+#define callnum
+#include "tst-audit-threads.h"
+#undef callnum
+}
+
+void *
+thread_main (void *unused)
+{
+ call_all_ret_nums ();
+ return NULL;
+}
+
+#define STR2(X) #X
+#define STR(X) STR2(X)
+
+static int
+do_test (void)
+{
+ int i;
+ pthread_t *threads;
+
+ num_threads = get_nprocs ();
+ if (num_threads <= 1)
+ num_threads = 2;
+
+ /* Used to synchronize all the threads after calling each retNumN. */
+ pthread_barrier_init (&barrier, NULL, num_threads);
+
+ threads = (pthread_t *) malloc (num_threads * sizeof(pthread_t));
+ bzero (threads, num_threads * sizeof(pthread_t));
+ for (i = 0; i < num_threads; i++)
+ pthread_create(threads + i, NULL, thread_main, NULL);
+
+ for (i = 0; i < num_threads; i++)
+ pthread_join(threads[i], NULL);
+
+ free (threads);
+
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,86 @@
+/* Helper header for test-audit-threads.
+
+ Copyright (C) 2018 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
+ <http://www.gnu.org/licenses/>. */
+
+#define CONCAT(a, b) a ## b
+#define NUM(x, y) CONCAT (x, y)
+
+#define FUNC10(x) \
+ FUNC (NUM (x, 0)); \
+ FUNC (NUM (x, 1)); \
+ FUNC (NUM (x, 2)); \
+ FUNC (NUM (x, 3)); \
+ FUNC (NUM (x, 4)); \
+ FUNC (NUM (x, 5)); \
+ FUNC (NUM (x, 6)); \
+ FUNC (NUM (x, 7)); \
+ FUNC (NUM (x, 8)); \
+ FUNC (NUM (x, 9))
+
+#define FUNC100(x) \
+ FUNC10 (NUM (x, 0)); \
+ FUNC10 (NUM (x, 1)); \
+ FUNC10 (NUM (x, 2)); \
+ FUNC10 (NUM (x, 3)); \
+ FUNC10 (NUM (x, 4)); \
+ FUNC10 (NUM (x, 5)); \
+ FUNC10 (NUM (x, 6)); \
+ FUNC10 (NUM (x, 7)); \
+ FUNC10 (NUM (x, 8)); \
+ FUNC10 (NUM (x, 9))
+
+#define FUNC1000(x) \
+ FUNC100 (NUM (x, 0)); \
+ FUNC100 (NUM (x, 1)); \
+ FUNC100 (NUM (x, 2)); \
+ FUNC100 (NUM (x, 3)); \
+ FUNC100 (NUM (x, 4)); \
+ FUNC100 (NUM (x, 5)); \
+ FUNC100 (NUM (x, 6)); \
+ FUNC100 (NUM (x, 7)); \
+ FUNC100 (NUM (x, 8)); \
+ FUNC100 (NUM (x, 9))
+
+#define FUNC10000() \
+ FUNC1000 (1); \
+ FUNC1000 (2); \
+ FUNC1000 (3); \
+ FUNC1000 (4); \
+ FUNC1000 (5); \
+ FUNC1000 (6); \
+ FUNC1000 (7); \
+ FUNC1000 (8); \
+ FUNC1000 (9)
+
+#ifdef FUNC
+# undef FUNC
+#endif
+
+#ifdef externnum
+# define FUNC(x) extern int CONCAT (retNum, x) (void)
+#endif
+
+#ifdef definenum
+# define FUNC(x) int CONCAT (retNum, x) (void) { return x; }
+#endif
+
+#ifdef callnum
+# define FUNC(x) CONCAT (retNum, x) (); sync_all (x)
+#endif
+
+FUNC10000 ();