the pthread_key_create_2-1 posix test invokes the debugger

[kaoutsis]

source file is: src/tests/system/libroot/posix/posixtestsuite/conformance/interfaces/pthread_key_create/2-1.c

[kaoutsis]

crashes inside in the pthread_getspecific():

[...]
int main()
{
	pthread_key_t key;
	void* rc;

	/* Verify that the value associated with "key" in a new thread is NULL */
	rc = pthread_getspecific(key);
	if(rc != NULL)
	{
		printf("pthread_key_create_2-1 Test FAILED\n");
		return PTS_FAIL;
	}
[...]

[kaoutsis]

the problem may be that the program didn't call the pthread_setspecific(), or there is any call to pthread_create() so the thread local store is uninitialized ?

[korli]

pthread_getspecific() should return NULL for unknown keys. And tls_get() should do the same. It seems tls_get() doesn't return NULL for unknown keys. Could you check on BeOS ? The bug seems to be in tls_get() which doesn't check the key.

[kaoutsis]

Replying to korli:

pthread_getspecific() should return NULL for unknown keys. And tls_get() should do the same. It seems tls_get() doesn't return NULL for unknown keys. Could you check on BeOS ? The bug seems to be in tls_get() which doesn't check the key.

On beos tls_get() returns NULL for unknown keys, i will attach the tls-test.cpp that ran on beos: $ g++ tls-test.cpp -o tls-test -lroot $ ./tls-test tls_get returns null $

[kaoutsis]

a test to see what tls_get() returns for an unknown key on beos

[bonefish]

tls_get()/tls_set() must not be called with an invalid key (they are implemented inline in BeOS, so we can't really do much about this). If the pthreads implementation relies on tls_get() behaving a certain way, it is broken.

That aside, we don't clear the TLS slots upon thread construction, which we probably should do. The related code is in src/system/kernel/arch/x86/arch_thread.c:arch_thread_init_tls(), if anyone wants to do that.

[axeld]

We shouldn't have to clear the TLS slots since they are put into a clean area (as part of the thread's stack). If they are dirty, some code uses this part early on, and that shouldn't happen.

[bonefish]

Replying to axeld:

We shouldn't have to clear the TLS slots since they are put into a clean area (as part of the thread's stack). If they are dirty, some code uses this part early on, and that shouldn't happen.

Please have a look at arch_thread_init_tls() -- the TLS data are copied from a mostly uninitialized array on the stack. A memset() there will do the trick, but the pthreads issue should probably be looked into first, before it is hidden.

[korli]

The test pass successfully with this change. Could you check ?

Index: src/tests/system/libroot/posix/posixtestsuite/conformance/interfaces/pthread_key_create/1-2.c =================================================================== --- src/tests/system/libroot/posix/posixtestsuite/conformance/interfaces/pthread_key_create/1-2.c (révision 23000) +++ src/tests/system/libroot/posix/posixtestsuite/conformance/interfaces/pthread_key_create/1-2.c (copie de travail) @@ -46,7 +46,7 @@

int main() {

pthread_t new_th;

• void *value_ptr;

+ void *value_ptr = NULL;

/* Create a key */ for(i = 0;i<NUM_OF_THREADS;i++)

[kaoutsis]

Replying to korli:

The test pass successfully with this change. Could you check ?

Index: src/tests/system/libroot/posix/posixtestsuite/conformance/interfaces/pthread_key_create/1-2.c =================================================================== --- src/tests/system/libroot/posix/posixtestsuite/conformance/interfaces/pthread_key_create/1-2.c (révision 23000) +++ src/tests/system/libroot/posix/posixtestsuite/conformance/interfaces/pthread_key_create/1-2.c (copie de travail) @@ -46,7 +46,7 @@

int main() {

pthread_t new_th;

• void *value_ptr;

+ void *value_ptr = NULL;

/* Create a key */ for(i = 0;i<NUM_OF_THREADS;i++)

i checked and it passed, but to be frank, i don't understand what the author wants to achieve. Note that the above fragment of code is from 1-2.c which is the case of #1644 (this is ticket #1646 for the file 2-1.c)

[bonefish]

Replying to korli:

@@ -46,7 +46,7 @@

int main() {

pthread_t new_th;

• void *value_ptr;

+ void *value_ptr = NULL;

/* Create a key */ for(i = 0;i<NUM_OF_THREADS;i++)

If this changes the behavior, our wait_for_thread() is obviously broken, as it should overwrite value_ptr in each iteration before being checked, so that the initial value should be irrelevant.

[korli]

Replying to bonefish:

If this changes the behavior, our wait_for_thread() is obviously broken, as it should overwrite value_ptr in each iteration before being checked, so that the initial value should be irrelevant.

Yes, it seems wait_for_thread() doesn't find the thread and doesn't overwrite value_ptr. BTW shouldn't it find the thread death entry ? Or is it too late and it's gone with the thread clean process ?

[axeld]

Replying to bonefish:

Please have a look at arch_thread_init_tls() -- the TLS data are copied from a mostly uninitialized array on the stack. A memset() there will do the trick, but the pthreads issue should probably be looked into first, before it is hidden.

That array is 8 bytes large (2 TLS entries), and is completely initialized and then copied to the TLS array; a memset() wouldn't change anything.

[kaoutsis]

Replying to kaoutsis:

Replying to korli:

pthread_getspecific() should return NULL for unknown keys. And tls_get() should do the same. It seems tls_get() doesn't return NULL for unknown keys. Could you check on BeOS ? The bug seems to be in tls_get() which doesn't check the key.

On beos tls_get() returns NULL for unknown keys, i will attach the tls-test.cpp that ran on beos: $ g++ tls-test.cpp -o tls-test -lroot $ ./tls-test tls_get returns null $

i don't know if this helps: i ran the little test on haiku and returns null also; if this little test is valid to check the returned value of tls_get() for unknown keys?

[korli]

Replying to kaoutsis:

i don't know if this helps: i ran the little test on haiku and returns null also; if this little test is valid to check the returned value of tls_get() for unknown keys?

Maybe try with a value bigger than 64 (TLS_MAX_KEYS).

[bonefish]

Replying to axeld:

Replying to bonefish:

Please have a look at arch_thread_init_tls() -- the TLS data are copied from a mostly uninitialized array on the stack. A memset() there will do the trick, but the pthreads issue should probably be looked into first, before it is hidden.

That array is 8 bytes large (2 TLS entries), and is completely initialized and then copied to the TLS array; a memset() wouldn't change anything.

My mistake, you're right of course. I wonder why this has been brought up, anyway. Apparently Haiku returns NULL for undefined keys, and the test program doesn't even use undefined keys in the first place.

Indeed wait_for_thread() is broken (and was even before my changes for job control support). When invoked for a thread of the same team that has already gone, it always fails. I suppose we need to introduce a per team list of dead threads. Currently there's only a list for child processes. I'll look into that.

[kaoutsis]

Replying to korli:

Replying to kaoutsis:

i don't know if this helps: i ran the little test on haiku and returns null also; if this little test is valid to check the returned value of tls_get() for unknown keys?

Maybe try with a value bigger than 64 (TLS_MAX_KEYS).

ok, i will attach tls-test2.cpp, and tls-test2-beos-results.txt; briefly: on beos when the key is negative, test crashes, when key is zero, tls_get doesn't return null, and when key is > 0 until value 990 tls_get returns null. Later i will post the haiku results.

[kaoutsis]

the range of key values are from 0 to 69 to this test

[kaoutsis]

the haiku results: test-app crashes for values bigger than 63

[kaoutsis]

Replying to kaoutsis:

Replying to korli:

Replying to kaoutsis:

i don't know if this helps: i ran the little test on haiku and returns null also; if this little test is valid to check the returned value of tls_get() for unknown keys?

Maybe try with a value bigger than 64 (TLS_MAX_KEYS).

ok, i will attach tls-test2.cpp, and tls-test2-beos-results.txt; briefly: on beos when the key is negative, test crashes, when key is zero, tls_get doesn't return null, and when key is > 0 until value 990 tls_get returns null. Later i will post the haiku results.

i changed the test-app a bit, the range of key values are now from 0 to 69: so for negative values both hrev5 and haiku crashes, for values 0 and 1 both tls_get doesn't return null, for values 2 to 63 on both systems tls_get return null, and for values 64 and up haiku crashes, in the hrev5 instead tls_get() returns null

[bonefish]

The test program is broken. The behavior of a pthread_getspecific() called with an invalid key is undefined as per specification. Commented the bad code block in hrev23010.