Resending as plaint text so the lists don't reject it ...
On Thu, 9 May 2019, 11:37 Alexandre Oliva wrote:
> Other classes that have native_handle/typesizes.cc tests have
> native_type_handle defined as a pointer type, and the pointed-to type is
> native_handle, the type of the single data member of the class (*). It
> thus makes sense to check that the single data member and the enclosing
> class type have the same size and alignment: a pointer to the enclosing
> type should also be a valid pointer to its single member.
>
> (*) this single data member is actually inherited or enclosed in another
> class, but let's not get distracted by this irrelevant detail.
>
> std::thread, however, does not follow this pattern. Not because the
> single data member is defined as a direct data member of a nested class,
> rather than inherited from another class. The key difference is that it
> does not use native_type_handle's pointed-to type as the single data
> member, it rather uses native_type_handle directly as the type of the
> single data member.
>
> On GNU/Linux, and presumably on most platforms, native_handle_type =
> __gthread_t is not even a pointer type. This key difference would have
> been obvious if remove_pointer<T> rejected non-pointer types, but that's
> not the case. When given __gthread_t -> pthread_t -> unsigned long int,
> remove_pointer<T>::type is T, so we get unsigned long int back. The
> test works because there's no pointer type to strip off.
>
Which is by design. The test is written to work for the cases where
native_handle() returns &m_handle and also the std::thread case where it
returns m_handle directly.
The problem is this:
> However, on a platform that defines __gthread_t as a pointer type, we
> use that pointer type as native_type_handle and as the type for the
> single data member of std::thread. But then, the test compares size and
> alignment of that type with those of the type obtained by removing one
> indirection level. We're comparing properties of different, unrelated
> types. Unless the pointed-to type happens to have, by chance, the size
> and alignment of a pointer, the test will fail.
>
>
Good catch.
>
> IOW, the test is no good: it's not testing what it should, and wherever
> it passes, it's by accident.
>
It's not by accident on GNU/Linux, or other targets where pthread_t is not
a pointer.
> In order to test what it should, we'd need to use an alternate test
> function that does not strip off one indirection level from
> native_handle_type, if the test is to remain.
>
Or just adapt the current test to work for the std::thread case too, by
only removing the pointer when we know we need to remove it, as in the
attached patch. Does this work on targets using a pointer type for
pthread_t?
Should I introduce such an alternate test function and adjust the test,
> or just remove the broken test?
>
> Or should we change std::thread::native_handle_type to __gthread_t*,
> while keeping unchanged the type of the single data member
> std::thread::id::_M_thread?
>
We could do that, but it would break any user code using the native handle.
I'd prefer not to do that just because we have a buggy testcase.
diff --git a/libstdc++-v3/testsuite/util/thread/all.h
b/libstdc++-v3/testsuite/util/thread/all.h
index e5794fa4a97..ec24822a8ce 100644
--- a/libstdc++-v3/testsuite/util/thread/all.h
+++ b/libstdc++-v3/testsuite/util/thread/all.h
@@ -39,7 +39,12 @@ namespace __gnu_test
// Remove possible pointer type.
typedef typename test_type::native_handle_type native_handle;
- typedef typename std::remove_pointer<native_handle>::type native_type;
+ // For std::thread native_handle_type is the type of its data member,
+ // for other types it's a pointer to the type of the data member.
+ typedef typename std::conditional<
+ std::is_same<test_type, std::thread>::value,
+ ::native_handle,
+ typename std::remove_pointer<native_handle>::type>::type native_type;
int st = sizeof(test_type);
int snt = sizeof(native_type);
