Re: [PATCH 1/3] nptl: Add tests for internal pthread_mutex_t offsets

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Oct 20, 2017 at 9:41 AM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
> On 20/10/2017 12:43, H.J. Lu wrote:
>> On Fri, Oct 20, 2017 at 7:08 AM, Adhemerval Zanella
>> <adhemerval.zanella@linaro.org> wrote:
>>>
>>>
>>> On 20/10/2017 11:57, H.J. Lu wrote:
>>>> On Fri, Oct 20, 2017 at 6:17 AM, Adhemerval Zanella
>>>> <adhemerval.zanella@linaro.org> wrote:
>>>>>
>>>>>
>>>>> On 19/10/2017 20:39, H.J. Lu wrote:
>>>>>> On Thu, Oct 19, 2017 at 2:54 PM, Adhemerval Zanella
>>>>>> <adhemerval.zanella@linaro.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 19/10/2017 19:12, H.J. Lu wrote:
>>>>>>>> On Thu, Oct 19, 2017 at 2:00 PM, Adhemerval Zanella
>>>>>>>>> diff --git a/sysdeps/x86_64/nptl/pthread-offsets.h b/sysdeps/x86_64/nptl/pthread-offsets.h
>>>>>>>>> new file mode 100644
>>>>>>>>> index 0000000..16c6b0d
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/sysdeps/x86_64/nptl/pthread-offsets.h
>>>>>>>>> @@ -0,0 +1,5 @@
>>>>>>>>> +#define __PTHREAD_MUTEX_NUSERS_OFFSET   12
>>>>>>>>> +#define __PTHREAD_MUTEX_KIND_OFFSET     16
>>>>>>>>> +#define __PTHREAD_MUTEX_SPINS_OFFSET    20
>>>>>>>>> +#define __PTHREAD_MUTEX_ELISION_OFFSET  22
>>>>>>>>> +#define __PTHREAD_MUTEX_LIST_OFFSET     24
>>>>>>>>> --
>>>>>>>>
>>>>>>>> These are wrong for x32.  You need to add
>>>>>>>>
>>>>>>>> #if __WORDSIZE == 64
>>>>>>>> ...
>>>>>>>
>>>>>>> Right, I used the values which are the one expected to avoid
>>>>>>> ABI breakage.  This indeed will break x32 tests build, but the
>>>>>>> idea is to have at 1/3 and 3/3 patch pushed together (which
>>>>>>> I should have made explicit in patch submission).
>>>>>>
>>>>>> No.  1/3 should be backported to 2.26/2.25 branches unchanged.
>>>>>
>>>>> So your suggestion is to have __WORDSIZE == 64 with the wrong
>>>>> values and get them right on the subsequent patch?
>>>>
>>>> The 1/3 patch should have the CORRECT offsets for x32, which
>>>> should pass on 2.25 branch and fail on 2.26/2.27 until the 3/3 patch
>>>> is applied.
>>>
>>> I am not following then because I cherry pick the patch on 2.25 branch
>>> it is works as is for x32.  And it also fails as expected on 2.26 with
>>> make nptl/tests.
>>
>> This doesn't sound right. Can I try your git branch?
>>
>
> I pushed both a release/2.25/master [1] and a release/2.26/master [2] with
> my test 1/3 patch applied on top of it.  The only change for 2.25 was to
> use #ifdef insteaf of #if to check for __elision existence (since it is
> not defined on all architectures).
>
> For 2.25 I saw no issue while on 2.26 I see:
>
> tst-offsets.c: In function ‘do_test’:
> tst-offsets.c:32:3: error: static assertion failed: "offset of __data.__nusers field of pthread_mutex_t != 12"
>    _Static_assert (offsetof (type, member) == offset, \
>    ^
> tst-offsets.c:38:3: note: in expansion of macro ‘TEST_OFFSET’
>    TEST_OFFSET (pthread_mutex_t, __data.__nusers,
>    ^~~~~~~~~~~
> tst-offsets.c:32:3: error: static assertion failed: "offset of __data.__kind field of pthread_mutex_t != 16"
>    _Static_assert (offsetof (type, member) == offset, \
>    ^
> tst-offsets.c:40:3: note: in expansion of macro ‘TEST_OFFSET’
>    TEST_OFFSET (pthread_mutex_t, __data.__kind,
>    ^~~~~~~~~~~
> tst-offsets.c:32:3: error: static assertion failed: "offset of __data.__list field of pthread_mutex_t != 24"
>    _Static_assert (offsetof (type, member) == offset, \
>    ^
> tst-offsets.c:48:3: note: in expansion of macro ‘TEST_OFFSET’
>    TEST_OFFSET (pthread_mutex_t, __data.__list,
>    ^~~~~~~~~~~
>
> [1] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/bz22298-2.25
> [2] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/bz22298-2.26

You are right.  By pure luck, x32 and x86-64 have the same offsets
in struct __pthread_mutex_s.  Your 1/3 patch is correct for x32.
Sorry for the confusion.

Thanks.

-- 
H.J.