[PATCH] Test case for BZ 19329

[PATCH] Test case for BZ 19329

[Szabolcs Nagy]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

Test concurrent dlopen and pthread_create when the loaded
modules have TLS.  This triggers dl-tls assertion failures
more reliably than the tst-stack4 test.

The dlopened module has 100 DT_NEEDED dependencies and
for me about 4000 concurrent thread creations are needed
to see failures on x86_64.

2016-11-24  Szabolcs Nagy  <szabolcs.nagy@arm.com>

	[BZ #19329]
	* nptl/Makefile (tests): Add tst-tls7.
	(modules-names): Add tst-tls7mod, tst-tls7mod-dep.
	* nptl/tst-tls7.c: New file.
	* nptl/tst-tls7mod-dep.c: New file.
	* nptl/tst-tls7mod.c: New file.

[-- Attachment #2: tls7.diff --]
[-- Type: text/x-patch, Size: 3983 bytes --]

diff --git a/nptl/Makefile b/nptl/Makefile
index 11588fe..90152d2 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -327,7 +327,7 @@ tests += tst-cancelx2 tst-cancelx3 tst-cancelx4 tst-cancelx5 \
 	 tst-oncex3 tst-oncex4
 ifeq ($(build-shared),yes)
 tests += tst-atfork2 tst-tls3 tst-tls3-malloc tst-tls4 tst-tls5 tst-_res1 \
-	 tst-fini1 tst-stackguard1
+	 tst-fini1 tst-stackguard1 tst-tls7
 tests-nolibpthread += tst-fini1
 ifeq ($(have-z-execstack),yes)
 tests += tst-execstack
@@ -338,7 +338,7 @@ 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-join7mod tst-tls7mod tst-tls7mod-dep
 extra-test-objs += $(addsuffix .os,$(strip $(modules-names))) \
 		   tst-cleanup4aux.o tst-cleanupx4aux.o
 test-extras += $(modules-names) tst-cleanup4aux tst-cleanupx4aux
@@ -545,6 +545,18 @@ $(objpfx)tst-tls5: $(objpfx)tst-tls5mod.so $(shared-thread-library)
 LDFLAGS-tst-tls5 = $(no-as-needed)
 LDFLAGS-tst-tls5mod.so = -Wl,-soname,tst-tls5mod.so
 
+$(objpfx)tst-tls7: $(libdl) $(shared-thread-library)
+tst-tls7mod-deps = $(shell for i in 0 1 2 3 4 5 6 7 8 9; do \
+			    for j in 0 1 2 3 4 5 6 7 8 9; do \
+			      echo $(objpfx)tst-tls7mod-dep-$$i-$$j.so; \
+			    done; done)
+$(objpfx)tst-tls7.out: $(objpfx)tst-tls7mod.so
+$(objpfx)tst-tls7mod.so: $(tst-tls7mod-deps)
+$(tst-tls7mod-deps): $(objpfx)tst-tls7mod-dep.so
+	cp -f $< $@
+clean:
+	rm -f $(tst-tls7mod-deps)
+
 ifeq ($(build-shared),yes)
 $(objpfx)tst-tls6.out: tst-tls6.sh $(objpfx)tst-tls5 \
 		       $(objpfx)tst-tls5moda.so $(objpfx)tst-tls5modb.so \
diff --git a/nptl/tst-tls7.c b/nptl/tst-tls7.c
new file mode 100644
index 0000000..e4c7db2
--- /dev/null
+++ b/nptl/tst-tls7.c
@@ -0,0 +1,62 @@
+/* Test concurrent dlopen and pthread_create: BZ 19329.
+   Copyright (C) 2016 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 <assert.h>
+#include <dlfcn.h>
+#include <pthread.h>
+#include <limits.h>
+
+#define THREADS 4000
+
+static void *
+start (void *a)
+{
+	assert (dlopen ("tst-tls7mod.so", RTLD_LAZY));
+	return 0;
+}
+
+static void *
+nop (void *a)
+{
+	return 0;
+}
+
+static int
+do_test (void)
+{
+	pthread_t td;
+	pthread_attr_t attr;
+
+	assert (pthread_attr_init (&attr) == 0);
+	assert (pthread_attr_setstacksize (&attr, PTHREAD_STACK_MIN) == 0);
+
+	/* Load a module with lots of dependencies and TLS.  */
+	assert (pthread_create (&td, 0, start, 0) == 0);
+
+	/* Concurrently create lots of threads.  */
+	for (int i = 0; i < THREADS; i++)
+		assert (pthread_create (&(pthread_t){0}, &attr, nop, 0) == 0);
+
+	/* Wait for the loading to finish, ignore other threads.  */
+	assert (pthread_join (td, 0) == 0);
+
+	return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/nptl/tst-tls7mod-dep.c b/nptl/tst-tls7mod-dep.c
new file mode 100644
index 0000000..206ece4
--- /dev/null
+++ b/nptl/tst-tls7mod-dep.c
@@ -0,0 +1 @@
+int __thread x;
diff --git a/nptl/tst-tls7mod.c b/nptl/tst-tls7mod.c
new file mode 100644
index 0000000..206ece4
--- /dev/null
+++ b/nptl/tst-tls7mod.c
@@ -0,0 +1 @@
+int __thread x;

Re: [PATCH] Test case for BZ 19329

[Florian Weimer]

On 11/24/2016 12:18 PM, Szabolcs Nagy wrote:
> Test concurrent dlopen and pthread_create when the loaded
> modules have TLS.  This triggers dl-tls assertion failures
> more reliably than the tst-stack4 test.
>
> The dlopened module has 100 DT_NEEDED dependencies and
> for me about 4000 concurrent thread creations are needed
> to see failures on x86_64.
>
> 2016-11-24  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>
> 	[BZ #19329]
> 	* nptl/Makefile (tests): Add tst-tls7.
> 	(modules-names): Add tst-tls7mod, tst-tls7mod-dep.
> 	* nptl/tst-tls7.c: New file.
> 	* nptl/tst-tls7mod-dep.c: New file.
> 	* nptl/tst-tls7mod.c: New file.
>

Please do not use assert in tests.  Use the xpthread_* functions in 
test-skeleton.c instead.

This test will fail on large SMP machines due to timeouts because thread 
creation is rather expensive there.  A less resource-intense way of 
writing this test would be to iterate multiple times and try to hit the 
race each time, with a smaller number of threads created in parallel.

Florian

Re: [PATCH] Test case for BZ 19329

[Szabolcs Nagy]

On 24/11/16 11:40, Florian Weimer wrote:
> On 11/24/2016 12:18 PM, Szabolcs Nagy wrote:
>> Test concurrent dlopen and pthread_create when the loaded
>> modules have TLS.  This triggers dl-tls assertion failures
>> more reliably than the tst-stack4 test.
>>
>> The dlopened module has 100 DT_NEEDED dependencies and
>> for me about 4000 concurrent thread creations are needed
>> to see failures on x86_64.
>>
>> 2016-11-24  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>>
>>     [BZ #19329]
>>     * nptl/Makefile (tests): Add tst-tls7.
>>     (modules-names): Add tst-tls7mod, tst-tls7mod-dep.
>>     * nptl/tst-tls7.c: New file.
>>     * nptl/tst-tls7mod-dep.c: New file.
>>     * nptl/tst-tls7mod.c: New file.
>>
> 
> Please do not use assert in tests.  Use the xpthread_* functions in test-skeleton.c instead.
> 

i can do that but it looks ugly.

we are looking for an assertion failure anyway
so it would be better to make the test system
deal with that..

> This test will fail on large SMP machines due to timeouts because thread creation is rather expensive there.  A
> less resource-intense way of writing this test would be to iterate multiple times and try to hit the race each
> time, with a smaller number of threads created in parallel.
> 

i can try, but iterating is not easy: there
is dynamic linker internal state that cannot
be reset (generation counter, max dtv idx,
allocated slotininfo entries) so i'd need to
start a new process for every iteration.

Re: [PATCH] Test case for BZ 19329

[Florian Weimer]

On 11/24/2016 01:31 PM, Szabolcs Nagy wrote:

>> Please do not use assert in tests.  Use the xpthread_* functions in test-skeleton.c instead.
>>
>
> i can do that but it looks ugly.

Well.  The asserts are incorrect because you rely on their side effects.

>> This test will fail on large SMP machines due to timeouts because thread creation is rather expensive there.  A
>> less resource-intense way of writing this test would be to iterate multiple times and try to hit the race each
>> time, with a smaller number of threads created in parallel.
>>
>
> i can try, but iterating is not easy: there
> is dynamic linker internal state that cannot
> be reset (generation counter, max dtv idx,
> allocated slotininfo entries) so i'd need to
> start a new process for every iteration.

Can you use fork before each round?

Thanks,
Florian

Re: [PATCH] Test case for BZ 19329

[Szabolcs Nagy]

On 24/11/16 12:34, Florian Weimer wrote:
> On 11/24/2016 01:31 PM, Szabolcs Nagy wrote:
> 
>>> Please do not use assert in tests.  Use the xpthread_* functions in test-skeleton.c instead.
>>>
>>
>> i can do that but it looks ugly.
> 
> Well.  The asserts are incorrect because you rely on their side effects.
> 

ah i thought you are worried that the error output
does not end up in the test .out

i can undef NDEBUG before assert.h

>>> This test will fail on large SMP machines due to timeouts because thread creation is rather expensive there.  A
>>> less resource-intense way of writing this test would be to iterate multiple times and try to hit the race each
>>> time, with a smaller number of threads created in parallel.
>>>
>>
>> i can try, but iterating is not easy: there
>> is dynamic linker internal state that cannot
>> be reset (generation counter, max dtv idx,
>> allocated slotininfo entries) so i'd need to
>> start a new process for every iteration.
> 
> Can you use fork before each round?
> 

ok, i'll try that.

Re: [PATCH] Test case for BZ 19329

[Joseph Myers]

On Thu, 24 Nov 2016, Szabolcs Nagy wrote:

> Test concurrent dlopen and pthread_create when the loaded
> modules have TLS.  This triggers dl-tls assertion failures
> more reliably than the tst-stack4 test.
> 
> The dlopened module has 100 DT_NEEDED dependencies and
> for me about 4000 concurrent thread creations are needed
> to see failures on x86_64.

I'd be concerned about 4000 threads exceeding task or memory limits, 
possibly interfering with other things the same user is doing that also 
try to create tasks.

(I rountinely see tst-eintr1 failing with "pthread_create failed: Resource 
temporarily unavailable", sometimes terminating the test run because make 
fails to fork to run evaluate-test.sh, apparently not all threads having 
properly terminated by the time the test does.  I don't know how many 
threads that is creating.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Test case for BZ 19329

[Florian Weimer]

On 11/24/2016 02:41 PM, Joseph Myers wrote:
> On Thu, 24 Nov 2016, Szabolcs Nagy wrote:
>
>> Test concurrent dlopen and pthread_create when the loaded
>> modules have TLS.  This triggers dl-tls assertion failures
>> more reliably than the tst-stack4 test.
>>
>> The dlopened module has 100 DT_NEEDED dependencies and
>> for me about 4000 concurrent thread creations are needed
>> to see failures on x86_64.
>
> I'd be concerned about 4000 threads exceeding task or memory limits,
> possibly interfering with other things the same user is doing that also
> try to create tasks.

Yes, reducing the number of threads which are created in parallel is 
strongly recommended.

> (I rountinely see tst-eintr1 failing with "pthread_create failed: Resource
> temporarily unavailable", sometimes terminating the test run because make
> fails to fork to run evaluate-test.sh, apparently not all threads having
> properly terminated by the time the test does.  I don't know how many
> threads that is creating.)

As far as I can tell, tst-eintr1 creates only twenty threads at most: 
ten outer threads, and each outer thread creates one inner thread.

It's likely you are running into a kernel bug, where task exit is 
reported before the kernel has deallocated the resources used by the task:

   https://bugzilla.kernel.org/show_bug.cgi?id=154011

Florian