nptl: Add test nptl/tst-pthread-perthread-inherit
Commit Message
This test checks the inheritance behavior of the per-thread flags.
It is also a regular (non-xtest) for pthread_attr_setperthreadids_np
and pthread_attr_getperthreadids_np.
2019-06-28 Florian Weimer <fweimer@redhat.com>
* nptl/Makefile (tests): Add tst-pthread-perthread-inherit.
* nptl/tst-pthread-perthread-inherit.c: New file.
Comments
On 6/28/19 4:05 PM, Florian Weimer wrote:
> This test checks the inheritance behavior of the per-thread flags.
>
> It is also a regular (non-xtest) for pthread_attr_setperthreadids_np
> and pthread_attr_getperthreadids_np.
>
> 2019-06-28 Florian Weimer <fweimer@redhat.com>
>
> * nptl/Makefile (tests): Add tst-pthread-perthread-inherit.
> * nptl/tst-pthread-perthread-inherit.c: New file.
>
Great test overall! I have only one qualm about the boolean vs scope
comparison (see below).
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 52913cc2f0..56f1578860 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -324,7 +324,8 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
> tst-rwlock-pwn \
> tst-rwlock-tryrdlock-stall tst-rwlock-trywrlock-stall \
> tst-unwind-thread \
> - tst-pthread-perthreadfs tst-pthread-perthreadfs-chroot
> + tst-pthread-perthreadfs tst-pthread-perthreadfs-chroot \
> + tst-pthread-perthread-inherit
OK.
>
> tests-internal := tst-rwlock19 tst-rwlock20 \
> tst-sem11 tst-sem12 tst-sem13 \
> diff --git a/nptl/tst-pthread-perthread-inherit.c b/nptl/tst-pthread-perthread-inherit.c
> new file mode 100644
> index 0000000000..913deffdcc
> --- /dev/null
> +++ b/nptl/tst-pthread-perthread-inherit.c
> @@ -0,0 +1,203 @@
> +/* Test inheritance of per-thread attributes.
> + Copyright (C) 2019 Free Software Foundation, Inc.
OK.
> + 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 <errno.h>
> +#include <stdbool.h>
> +#include <stdlib.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/xthread.h>
> +
> +/* Controls how an attribute is applied. */
> +enum kind { null_attribute, default_attribute, attribute_per_process,
> + attribute_per_thread };
> +
> +/* Determine the combined per-thread/per-process status from the two
> + requested attributes. */
> +static int
> +combine_kind (enum kind a, enum kind b)
> +{
> + if (a == attribute_per_thread || b == attribute_per_thread)
> + return PTHREAD_PER_THREAD_NP;
> + else
> + return PTHREAD_PER_PROCESS_NP;
> +}
> +
> +/* Convert one kind to the corresponding PTHREAD_PER_PROCESS_NP,
> + PTHREAD_PER_THREAD_NP flag. */
> +static int
> +convert_kind (enum kind k)
> +{
> + if (k == attribute_per_thread)
> + return PTHREAD_PER_THREAD_NP;
> + else
> + return PTHREAD_PER_PROCESS_NP;
> +}
> +
> +/* Return true if the thread has per-thread IDs. */
> +static bool
> +perthread_flag (pthread_t thr,
> + int (*getter) (const pthread_attr_t *, int *))
> +{
> + pthread_attr_t attr;
> + int ret = pthread_getattr_np (thr, &attr);
> + if (ret != 0)
> + {
> + errno = ret;
> + FAIL_EXIT1 ("pthread_getattr_np: %m");
> + }
> + int flag = -1;
> + TEST_COMPARE (getter (&attr, &flag), 0);
> + if (flag != PTHREAD_PER_THREAD_NP)
> + TEST_COMPARE (flag, PTHREAD_PER_PROCESS_NP);
> + xpthread_attr_destroy (&attr);
> + return flag == PTHREAD_PER_THREAD_NP;
> +}
OK.
> +
> +/* The loop in do_test iterates through the Cartesian product of all
> + possible values. */
> +static enum kind outer_perthreadfs;
> +static enum kind inner_perthreadfs;
> +static enum kind outer_perthreadids;
> +static enum kind inner_perthreadids;
> +
> +static void *
> +inner_thread (void *closure)
> +{
> + TEST_COMPARE (perthread_flag (pthread_self (),
> + pthread_attr_getperthreadfs_np),
> + combine_kind (outer_perthreadfs, inner_perthreadfs));
> + TEST_COMPARE (perthread_flag (pthread_self (),
> + pthread_attr_getperthreadids_np),
> + combine_kind (outer_perthreadids, inner_perthreadids));
I don't like that this compares a boolean to a scope value, it's
not explicit enough about what we're comparing and so is confusing.
Please have perthread_flag return a proper scope value to compare,
or refactor to make them both compare boolean?
> + return NULL;
> +}
> +
> +static void *
> +outer_thread (void *attr)
> +{
> + TEST_COMPARE (perthread_flag (pthread_self (),
> + pthread_attr_getperthreadfs_np),
> + convert_kind (outer_perthreadfs));
> + TEST_COMPARE (perthread_flag (pthread_self (),
> + pthread_attr_getperthreadids_np),
> + convert_kind (outer_perthreadids));
> + pthread_t thr = xpthread_create (attr, inner_thread, NULL);
> + TEST_COMPARE (perthread_flag (thr, pthread_attr_getperthreadfs_np),
> + combine_kind (outer_perthreadfs, inner_perthreadfs));
> + TEST_COMPARE (perthread_flag (thr, pthread_attr_getperthreadids_np),
> + combine_kind (outer_perthreadids, inner_perthreadids));
> + return xpthread_join (thr);
OK.
> +}
> +
> +/* Apply the attribute KIND according to SETTER to ATTR. */
> +static void
> +attribute_apply (pthread_attr_t *attr, enum kind kind,
> + int (*setter) (pthread_attr_t *attr, int))
> +{
> + switch (kind)
> + {
> + case default_attribute:
> + return;
> +
> + case attribute_per_process:
> + case attribute_per_thread:
> + TEST_COMPARE (setter (attr, convert_kind (kind)), 0);
OK.
> + return;
> +
> + case null_attribute:
> + /* Report failure below. */
> + ;
> + }
> +
> + FAIL_EXIT1 ("invalid kind: %d", (int) kind);
OK.
> +}
> +
> +/* Create a new attribute according to both kinds. */
> +static pthread_attr_t *
> +make_attribute (enum kind perthreadfs, enum kind perthreadids)
> +{
> + if (perthreadfs == null_attribute)
> + {
> + TEST_COMPARE (perthreadids, null_attribute);
> + return NULL;
> + }
> + pthread_attr_t *result = xmalloc (sizeof (*result));
> + xpthread_attr_init (result);
> + attribute_apply (result, perthreadfs, pthread_attr_setperthreadfs_np);
> + attribute_apply (result, perthreadids, pthread_attr_setperthreadids_np);
> + return result;
> +}
> +
> +/* Deallocate the attribute pointer returned from make_attribute. */
> +static void
> +free_attribute (pthread_attr_t *attr)
> +{
> + if (attr != NULL)
> + {
> + xpthread_attr_destroy (attr);
> + free (attr);
> + }
> +}
> +
> +static int
> +do_test (void)
> +{
> + for (outer_perthreadfs = null_attribute;
> + outer_perthreadfs <= attribute_per_thread;
> + ++outer_perthreadfs)
> + for (inner_perthreadfs = null_attribute;
> + inner_perthreadfs <= attribute_per_thread;
> + ++inner_perthreadfs)
> + for (outer_perthreadids = null_attribute;
> + outer_perthreadids <= attribute_per_thread;
> + ++outer_perthreadids)
> + for (inner_perthreadids = null_attribute;
> + inner_perthreadids <= attribute_per_thread;
> + ++inner_perthreadids)
OK. Yay! :-) All combinations!
> + {
> + /* Some combinations are impossibe. If we must pass a
s/impossibe/impossible/g
> + NULL attribute at one level, then that is true for both
> + kinds of properties. */
> + if ((outer_perthreadfs == null_attribute)
> + != (outer_perthreadids == null_attribute))
> + continue;
> + if ((inner_perthreadfs == null_attribute)
> + != (inner_perthreadids == null_attribute))
> + continue;
Do you *have* to exclude these? Could we let the extra tests run?
Sure they don't make sense, but why do you say they are "impossible?"
> +
> + pthread_attr_t *outer_attr
> + = make_attribute (outer_perthreadfs, outer_perthreadids);
> + pthread_attr_t *inner_attr
> + = make_attribute (inner_perthreadfs, inner_perthreadids);
> + pthread_t thr = xpthread_create (outer_attr,
> + outer_thread, inner_attr);
> + TEST_COMPARE (perthread_flag (thr,
> + pthread_attr_getperthreadfs_np),
> + convert_kind (outer_perthreadfs));
> + TEST_COMPARE (perthread_flag (thr,
> + pthread_attr_getperthreadids_np),
> + convert_kind (outer_perthreadids));
> + xpthread_join (thr);
> + free_attribute (inner_attr);
> + free_attribute (outer_attr);
> + }
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
>
* Carlos O'Donell:
>> + NULL attribute at one level, then that is true for both
>> + kinds of properties. */
>> + if ((outer_perthreadfs == null_attribute)
>> + != (outer_perthreadids == null_attribute))
>> + continue;
>> + if ((inner_perthreadfs == null_attribute)
>> + != (inner_perthreadids == null_attribute))
>> + continue;
>
> Do you *have* to exclude these? Could we let the extra tests run?
>
> Sure they don't make sense, but why do you say they are "impossible?"
If threadfs requires a NULL attribute, and threadids requres
PTHREAD_PER_THREAD_NP, then we cannot run the subtest because the NULL
value does not carry any attributes.
Thanks,
Florian
On 6/30/19 5:10 AM, Florian Weimer wrote:
> * Carlos O'Donell:
>
>>> + NULL attribute at one level, then that is true for both
>>> + kinds of properties. */
>>> + if ((outer_perthreadfs == null_attribute)
>>> + != (outer_perthreadids == null_attribute))
>>> + continue;
>>> + if ((inner_perthreadfs == null_attribute)
>>> + != (inner_perthreadids == null_attribute))
>>> + continue;
>>
>> Do you *have* to exclude these? Could we let the extra tests run?
>>
>> Sure they don't make sense, but why do you say they are "impossible?"
>
> If threadfs requires a NULL attribute, and threadids requres
> PTHREAD_PER_THREAD_NP, then we cannot run the subtest because the NULL
> value does not carry any attributes.
I don't follow.
You have two functions, pthread_attr_setperthreadfs_np, and
pthread_attr_setperthreadids_np.
You want to call them with the permutation of their options
and check the results.
At an API level the arguments used to call them are unrelated.
I don't understand what you what to express in the last paragraph.
* Carlos O'Donell:
> On 6/30/19 5:10 AM, Florian Weimer wrote:
>> * Carlos O'Donell:
>>
>>>> + NULL attribute at one level, then that is true for both
>>>> + kinds of properties. */
>>>> + if ((outer_perthreadfs == null_attribute)
>>>> + != (outer_perthreadids == null_attribute))
>>>> + continue;
>>>> + if ((inner_perthreadfs == null_attribute)
>>>> + != (inner_perthreadids == null_attribute))
>>>> + continue;
>>>
>>> Do you *have* to exclude these? Could we let the extra tests run?
>>>
>>> Sure they don't make sense, but why do you say they are "impossible?"
>>
>> If threadfs requires a NULL attribute, and threadids requres
>> PTHREAD_PER_THREAD_NP, then we cannot run the subtest because the NULL
>> value does not carry any attributes.
>
> I don't follow.
>
> You have two functions, pthread_attr_setperthreadfs_np, and
> pthread_attr_setperthreadids_np.
>
> You want to call them with the permutation of their options
> and check the results.
>
> At an API level the arguments used to call them are unrelated.
>
> I don't understand what you what to express in the last paragraph.
pthread_create has only one attribute argument. If it is NULL, it
means that both attributes are absent.
On 7/1/19 12:47 AM, Florian Weimer wrote:
> * Carlos O'Donell:
>
>> On 6/30/19 5:10 AM, Florian Weimer wrote:
>>> * Carlos O'Donell:
>>>
>>>>> + NULL attribute at one level, then that is true for both
>>>>> + kinds of properties. */
>>>>> + if ((outer_perthreadfs == null_attribute)
>>>>> + != (outer_perthreadids == null_attribute))
>>>>> + continue;
>>>>> + if ((inner_perthreadfs == null_attribute)
>>>>> + != (inner_perthreadids == null_attribute))
>>>>> + continue;
>>>>
>>>> Do you *have* to exclude these? Could we let the extra tests run?
>>>>
>>>> Sure they don't make sense, but why do you say they are "impossible?"
>>>
>>> If threadfs requires a NULL attribute, and threadids requres
>>> PTHREAD_PER_THREAD_NP, then we cannot run the subtest because the NULL
>>> value does not carry any attributes.
>>
>> I don't follow.
>>
>> You have two functions, pthread_attr_setperthreadfs_np, and
>> pthread_attr_setperthreadids_np.
>>
>> You want to call them with the permutation of their options
>> and check the results.
>>
>> At an API level the arguments used to call them are unrelated.
>>
>> I don't understand what you what to express in the last paragraph.
>
> pthread_create has only one attribute argument. If it is NULL, it
> means that both attributes are absent.
Ah, In my mind I was just "promoting" the NULL attribute to a non-NULL
when an attribute is needed i.e. the merger of both requirements ends
up being a non-NULL attribute.
In your opinion this becomes a violation of the test constraint.
I see your point. I'm OK with the code as-is then.
@@ -324,7 +324,8 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
tst-rwlock-pwn \
tst-rwlock-tryrdlock-stall tst-rwlock-trywrlock-stall \
tst-unwind-thread \
- tst-pthread-perthreadfs tst-pthread-perthreadfs-chroot
+ tst-pthread-perthreadfs tst-pthread-perthreadfs-chroot \
+ tst-pthread-perthread-inherit
tests-internal := tst-rwlock19 tst-rwlock20 \
tst-sem11 tst-sem12 tst-sem13 \
new file mode 100644
@@ -0,0 +1,203 @@
+/* Test inheritance of per-thread attributes.
+ Copyright (C) 2019 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 <errno.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/xthread.h>
+
+/* Controls how an attribute is applied. */
+enum kind { null_attribute, default_attribute, attribute_per_process,
+ attribute_per_thread };
+
+/* Determine the combined per-thread/per-process status from the two
+ requested attributes. */
+static int
+combine_kind (enum kind a, enum kind b)
+{
+ if (a == attribute_per_thread || b == attribute_per_thread)
+ return PTHREAD_PER_THREAD_NP;
+ else
+ return PTHREAD_PER_PROCESS_NP;
+}
+
+/* Convert one kind to the corresponding PTHREAD_PER_PROCESS_NP,
+ PTHREAD_PER_THREAD_NP flag. */
+static int
+convert_kind (enum kind k)
+{
+ if (k == attribute_per_thread)
+ return PTHREAD_PER_THREAD_NP;
+ else
+ return PTHREAD_PER_PROCESS_NP;
+}
+
+/* Return true if the thread has per-thread IDs. */
+static bool
+perthread_flag (pthread_t thr,
+ int (*getter) (const pthread_attr_t *, int *))
+{
+ pthread_attr_t attr;
+ int ret = pthread_getattr_np (thr, &attr);
+ if (ret != 0)
+ {
+ errno = ret;
+ FAIL_EXIT1 ("pthread_getattr_np: %m");
+ }
+ int flag = -1;
+ TEST_COMPARE (getter (&attr, &flag), 0);
+ if (flag != PTHREAD_PER_THREAD_NP)
+ TEST_COMPARE (flag, PTHREAD_PER_PROCESS_NP);
+ xpthread_attr_destroy (&attr);
+ return flag == PTHREAD_PER_THREAD_NP;
+}
+
+/* The loop in do_test iterates through the Cartesian product of all
+ possible values. */
+static enum kind outer_perthreadfs;
+static enum kind inner_perthreadfs;
+static enum kind outer_perthreadids;
+static enum kind inner_perthreadids;
+
+static void *
+inner_thread (void *closure)
+{
+ TEST_COMPARE (perthread_flag (pthread_self (),
+ pthread_attr_getperthreadfs_np),
+ combine_kind (outer_perthreadfs, inner_perthreadfs));
+ TEST_COMPARE (perthread_flag (pthread_self (),
+ pthread_attr_getperthreadids_np),
+ combine_kind (outer_perthreadids, inner_perthreadids));
+ return NULL;
+}
+
+static void *
+outer_thread (void *attr)
+{
+ TEST_COMPARE (perthread_flag (pthread_self (),
+ pthread_attr_getperthreadfs_np),
+ convert_kind (outer_perthreadfs));
+ TEST_COMPARE (perthread_flag (pthread_self (),
+ pthread_attr_getperthreadids_np),
+ convert_kind (outer_perthreadids));
+ pthread_t thr = xpthread_create (attr, inner_thread, NULL);
+ TEST_COMPARE (perthread_flag (thr, pthread_attr_getperthreadfs_np),
+ combine_kind (outer_perthreadfs, inner_perthreadfs));
+ TEST_COMPARE (perthread_flag (thr, pthread_attr_getperthreadids_np),
+ combine_kind (outer_perthreadids, inner_perthreadids));
+ return xpthread_join (thr);
+}
+
+/* Apply the attribute KIND according to SETTER to ATTR. */
+static void
+attribute_apply (pthread_attr_t *attr, enum kind kind,
+ int (*setter) (pthread_attr_t *attr, int))
+{
+ switch (kind)
+ {
+ case default_attribute:
+ return;
+
+ case attribute_per_process:
+ case attribute_per_thread:
+ TEST_COMPARE (setter (attr, convert_kind (kind)), 0);
+ return;
+
+ case null_attribute:
+ /* Report failure below. */
+ ;
+ }
+
+ FAIL_EXIT1 ("invalid kind: %d", (int) kind);
+}
+
+/* Create a new attribute according to both kinds. */
+static pthread_attr_t *
+make_attribute (enum kind perthreadfs, enum kind perthreadids)
+{
+ if (perthreadfs == null_attribute)
+ {
+ TEST_COMPARE (perthreadids, null_attribute);
+ return NULL;
+ }
+ pthread_attr_t *result = xmalloc (sizeof (*result));
+ xpthread_attr_init (result);
+ attribute_apply (result, perthreadfs, pthread_attr_setperthreadfs_np);
+ attribute_apply (result, perthreadids, pthread_attr_setperthreadids_np);
+ return result;
+}
+
+/* Deallocate the attribute pointer returned from make_attribute. */
+static void
+free_attribute (pthread_attr_t *attr)
+{
+ if (attr != NULL)
+ {
+ xpthread_attr_destroy (attr);
+ free (attr);
+ }
+}
+
+static int
+do_test (void)
+{
+ for (outer_perthreadfs = null_attribute;
+ outer_perthreadfs <= attribute_per_thread;
+ ++outer_perthreadfs)
+ for (inner_perthreadfs = null_attribute;
+ inner_perthreadfs <= attribute_per_thread;
+ ++inner_perthreadfs)
+ for (outer_perthreadids = null_attribute;
+ outer_perthreadids <= attribute_per_thread;
+ ++outer_perthreadids)
+ for (inner_perthreadids = null_attribute;
+ inner_perthreadids <= attribute_per_thread;
+ ++inner_perthreadids)
+ {
+ /* Some combinations are impossibe. If we must pass a
+ NULL attribute at one level, then that is true for both
+ kinds of properties. */
+ if ((outer_perthreadfs == null_attribute)
+ != (outer_perthreadids == null_attribute))
+ continue;
+ if ((inner_perthreadfs == null_attribute)
+ != (inner_perthreadids == null_attribute))
+ continue;
+
+ pthread_attr_t *outer_attr
+ = make_attribute (outer_perthreadfs, outer_perthreadids);
+ pthread_attr_t *inner_attr
+ = make_attribute (inner_perthreadfs, inner_perthreadids);
+ pthread_t thr = xpthread_create (outer_attr,
+ outer_thread, inner_attr);
+ TEST_COMPARE (perthread_flag (thr,
+ pthread_attr_getperthreadfs_np),
+ convert_kind (outer_perthreadfs));
+ TEST_COMPARE (perthread_flag (thr,
+ pthread_attr_getperthreadids_np),
+ convert_kind (outer_perthreadids));
+ xpthread_join (thr);
+ free_attribute (inner_attr);
+ free_attribute (outer_attr);
+ }
+ return 0;
+}
+
+#include <support/test-driver.c>